Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] PUT http request not replacing/updating the entity #55

Open
Masber opened this issue Dec 15, 2024 · 13 comments · May be fixed by #57
Open

[BUG] PUT http request not replacing/updating the entity #55

Masber opened this issue Dec 15, 2024 · 13 comments · May be fixed by #57
Assignees
Labels
bug Something isn't working

Comments

@Masber
Copy link

Masber commented Dec 15, 2024

Describe the bug
According to the API documentation, BSS bootparametes accepts PUT http requests to update entities but it fails when I try with the following error

{"type":"about:blank","title":"Bad Request","detail":"No data","status":400}

To Reproduce
Steps to reproduce the behavior:

  1. start OCHAMI
  2. Create new BSS bootparameter entity
curl --cacert cacert.pem -H "Authorization: Bearer $ACCESS_TOKEN" https://foobar.openchami.cluster:8443/boot/v1/bootparameters \
-d '{"hosts": ["x1007c0s0b0n1"], "macs": ["00:40:a6:82:f6:c5"], "nids": [1], "params": "test", "kernel": "test", "initrd": "test"}'
  1. Try to update the entity
curl -XPUT --cacert cacert.pem -H "Authorization: Bearer $ACCESS_TOKEN" https://foobar.openchami.cluster:8443/boot/v1/bootparameters \
-d '{ "hosts": ["x1007c0s0b0n1"], "params": "test param=test", "kernel": "s3://boot-images/1dbb777c-2527-449b-bd6d-fb4d1cb79e88/kernel", "initrd": "s3://boot-images/1dbb777c-2527-449b-bd6d-fb4d1cb79e88/initrd" }'

Expected behavior
According to the documentation, the PUT request should return a 200 code and update the bss bootparameter entity

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: Ubuntu 24.04.1 LTS
  • Browser NA
  • Version "bss-version": "1.31.3"

Smartphone (please complete the following information):

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS8.1]
  • Browser [e.g. stock browser, safari]
  • Version [e.g. 22]

Additional context
Add any other context about the problem here.

@Masber Masber added the bug Something isn't working label Dec 15, 2024
@Masber
Copy link
Author

Masber commented Dec 15, 2024

PATCH works

curl -XPATCH --cacert cacert.pem -H "Authorization: Bearer $ACCESS_TOKEN" https://foobar.openchami.cluster:8443/boot/v1/bootparameters \
-d '{ "hosts": ["x1007c0s0b0n1"], "params": "test param=test", "kernel": "test", "initrd": "test" }'

@alexlovelltroy
Copy link
Member

@shunr-hpe Is this true in CSM as well?

@synackd
Copy link
Collaborator

synackd commented Dec 16, 2024

@Masber Which storage backend are you running BSS with? Postgres or Etcd?

@Masber
Copy link
Author

Masber commented Dec 17, 2024

Hi, I am deploying using deployment recipes quickstart (postgres)

@alexlovelltroy
Copy link
Member

We think this is probably a Postgres-specific bug and will prioritize getting it looked at. Feel free to submit a PR if you think you know where this is happening, @Masber

@synackd
Copy link
Collaborator

synackd commented Dec 18, 2024

I am able to reproduce this. It looks like the postgres.Add method is being called when BSS receives a PUT at /bootparameters when using the PostgreSQL backend. I will dig into seeing why this is the case.

@shunr-hpe
Copy link

Those commands worked with the CSM flavor of BSS

curl -ks -X PUT ${BSS_URL}/boot/v1/bootparameters \
-d '{"hosts": ["x1007c0s0b0n1"], "macs": ["00:40:a6:82:f6:c5"], "nids": [1], "params": "test", "kernel": "test", "initrd": "test"}'

curl -ks -X GET "${BSS_URL}/boot/v1/bootparameters?name=x1007c0s0b0n1" | jq .[0] > v1.json 

curl -ks -X PUT ${BSS_URL}/boot/v1/bootparameters \
-d '{ "hosts": ["x1007c0s0b0n1"], "params": "test param=test", "kernel": "s3://boot-images/1dbb777c-2527-449b-bd6d-fb4d1cb79e88/kernel", "initrd": "s3://boot-images/1dbb777c-2527-449b-bd6d-fb4d1cb79e88/initrd" }'

curl -ks -X GET 'http://localhost:27778/boot/v1/bootparameters?name=x1007c0s0b0n1' | jq .[0] > v2.json

diff v1.json v2.json
5,7c5,7
<   "params": "test",
<   "kernel": "test",
<   "initrd": "test",
---
>   "params": "test param=test",
>   "kernel": "s3://boot-images/1dbb777c-2527-449b-bd6d-fb4d1cb79e88/kernel",
>   "initrd": "s3://boot-images/1dbb777c-2527-449b-bd6d-fb4d1cb79e88/initrd",

@synackd
Copy link
Collaborator

synackd commented Dec 19, 2024

The issue is that the Store() function that is called in the PUT handler calls postgres.Add(), which errs if the boot parameters exist. When I wrote this, it was in preparation for the SC23 demo and so I didn't get the time to implement a function for PUT. I can do that and open a PR for this issue.

@synackd
Copy link
Collaborator

synackd commented Dec 19, 2024

I also noticed that, when sending a PUT, BSS tries to PATCH instead. For example, when running:

curl -XPUT --cacert cacert.pem -H "Authorization: Bearer $ACCESS_TOKEN" https://foobar.openchami.cluster:8443/boot/v1/bootparameters \
-d '{ "hosts": ["x1007c0s0b0n1"], "params": "test param=test", "kernel": "s3://boot-images/1dbb777c-2527-449b-bd6d-fb4d1cb79e88/kernel", "initrd": "s3://boot-images/1dbb777c-2527-449b-bd6d-fb4d1cb79e88/initrd" }'

the following appears in the BSS logs:

2024/12/19 22:30:25 DEBUG: BootparametersPut(): Received request /boot/v1/bootparameters
[...]
2024/12/19 22:30:25 /bootparameters PATCH FAILED: postgres.Add: No nodes to add (possible duplicate(s)): {
  "hosts": [
    "x1007c0s0b0n1"
  ],
  "params": "test param=test",
  "kernel": "s3://boot-images/1dbb777c-2527-449b-bd6d-fb4d1cb79e88/kernel",
  "initrd": "s3://boot-images/1dbb777c-2527-449b-bd6d-fb4d1cb79e88/initrd",
  "cloud-init": {
    "meta-data": null,
    "user-data": null,
    "phone-home": {
      "pub_key_dsa": "",
      "pub_key_rsa": "",
      "pub_key_ecdsa": "",
      "instance_id": "",
      "hostname": "",
      "fqdn": ""
    }
  }
}

EDIT: Looks like that's just a copy-paste error. It's incorrect here but correct here.

@synackd synackd linked a pull request Dec 21, 2024 that will close this issue
@synackd synackd linked a pull request Dec 21, 2024 that will close this issue
@synackd
Copy link
Collaborator

synackd commented Dec 21, 2024

@Masber The linked PR above should fix the issue, and I tested it with your MWE above. Can you test to make sure it works on your end?

@Masber
Copy link
Author

Masber commented Dec 24, 2024

Hi @synackd ,

thank you for this, I tested the following curl command:

curl -XPUT --cacert cacert.pem -H "Authorization: Bearer $ACCESS_TOKEN" https://foobar.openchami.cluster:8443/boot/v1/bootparameters \
-d '{ "hosts": ["x1007c0s0b0n1", "x1001c1s5b0n1", "x1001c1s6b0n0", "x1001c1s6b0n1"], "params": "test param=test", "kernel": "test", "initrd": "test" }'

And it is now working

thank you

@Masber Masber closed this as completed Dec 24, 2024
@synackd
Copy link
Collaborator

synackd commented Dec 25, 2024

Hi @synackd ,

thank you for this, I tested the following curl command:

curl -XPUT --cacert cacert.pem -H "Authorization: Bearer $ACCESS_TOKEN" https://foobar.openchami.cluster:8443/boot/v1/bootparameters \
-d '{ "hosts": ["x1007c0s0b0n1", "x1001c1s5b0n1", "x1001c1s6b0n0", "x1001c1s6b0n1"], "params": "test param=test", "kernel": "test", "initrd": "test" }'

And it is now working

thank you

Sounds good. Will request a review on that PR.

@synackd
Copy link
Collaborator

synackd commented Dec 26, 2024

Hi @Masber, I'd like to keep this open until the linked PR is merged, just so we can consider this complete once the changes get merged into main.

@synackd synackd reopened this Dec 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants