Skip to content

Conversation

@shunr-hpe
Copy link
Collaborator

@shunr-hpe shunr-hpe commented Aug 29, 2025

SMD leaks memory if the request is not fully read and closed. The DrainAndCloseRequestBody function was created to do this. This change adds a call to this in doGroupMembersPut.

I tested this using the quickstart guide, and made calls like the following

curl -s -X POST -H "Authorization: Bearer $ACCESS_TOKEN" -d '{"label": "fish", "description": "a fish", "members": {"IDs": ["x1000c1s7b0", "x1000c1s7b1"]}}' http://smd:27779/hsm/v2/groups | jq

curl -s -X GET -H "Authorization: Bearer $ACCESS_TOKEN" http://smd:27779/hsm/v2/groups/fish | jq
curl -s -X GET -H "Authorization: Bearer $ACCESS_TOKEN" http://smd:27779/hsm/v2/groups/fish/members | jq

curl -s -X PUT -H "Authorization: Bearer $ACCESS_TOKEN" -d '{"Group": "fish", "IDs": ["x1000c1s7b1", "x1000c1s7b2"]}' http://smd:27779/hsm/v2/groups/fish/members | jq

SMD leaks memory if the request is not fully read and closed. The
DrainAndCloseRequestBody function was created to do this. This change
adds a call to this in doGroupMembersPut.

Signed-off-by: Shane Unruh <shane.unruh@hpe.com>
Copy link
Collaborator

@bmcdonald3 bmcdonald3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@synackd
Copy link
Contributor

synackd commented Aug 29, 2025

@shunr-hpe I'm currently testing this with the ochami tool emulating your curl requests above.

Can you comment as to what the difference in output I should be seeing is?

1. Creation of Group

Creation of group with members:

$ ochami smd group add fish -D 'a fish' -m x3000c0s0b0n0,x3000c0s0b1n0 --log-level debug --log-format basic
DEBUG | lib.go:177 > logging has been initialized
DEBUG | lib.go:309 > using base URI from default cluster redondo
DEBUG | lib.go:405 > authentication enabled for cluster redondo, reading and checking token
DEBUG | lib.go:437 > Determining token from environment variable based on cluster in config file
DEBUG | lib.go:443 > --cluster not specified, using default-cluster: redondo
DEBUG | lib.go:454 > Reading token from environment variable: REDONDO_ACCESS_TOKEN
DEBUG | lib.go:456 > Token found from environment variable: REDONDO_ACCESS_TOKEN=ey...
DEBUG | client.go:245 > POST: https://redondo.usrc:8443/hsm/v2/groups
DEBUG | client.go:266 > Request headers:
DEBUG | client.go:268 >   User-Agent: [ochami/0.5.1]
DEBUG | client.go:268 >   Authorization: [Bearer ey...]
DEBUG | client.go:274 > Request body:
DEBUG | client.go:275 > {"label":"fish","description":"a fish","members":{"ids":["x3000c0s0b0n0","x3000c0s0b1n0"]}}
DEBUG | client.go:288 > Response status: 201 Created
DEBUG | client.go:290 > Response headers:
DEBUG | client.go:292 >   Content-Type: [application/json]
DEBUG | client.go:292 >   Location: [/hsm/v2/groups/fish]
DEBUG | client.go:292 >   Date: [Fri, 29 Aug 2025 18:02:54 GMT]
DEBUG | client.go:292 >   Content-Length: [32]
DEBUG | client.go:305 > Response body:
DEBUG | client.go:306 > [{"URI":"/hsm/v2/groups/fish"}]

INFO | http.go:124 > Response status: HTTP/2.0 201 Created

Logs:

smd[3266131]: 172.16.0.254 - - [29/Aug/2025:18:02:54 +0000] "POST /hsm/v2/groups HTTP/1.1" 201 32 "" "ochami/0.5.1"
smd[3266131]: 6:02PM INF Request bytes_in=91 bytes_out=32 duration=194.988844 method=POST remote_addr=172.16.0.254 request_id=smd/Wa7VWeRz2q-000046 request_uri=/hsm/v2/groups status=Created status_code=201 user_agent=ochami/0.5.1
smd[3266131]: 2025/08/29 18:02:54 [smd/Wa7VWeRz2q-000046] "POST http://redondo.usrc:8443/hsm/v2/groups HTTP/1.1" from 172.16.0.254 - 201 32B in 195.054675ms

2. Checking Creation

Group:

$ ochami smd group get --name fish -F json-pretty
[
  {
    "description": "a fish",
    "label": "fish",
    "members": {
      "ids": [
        "x3000c0s0b0n0",
        "x3000c0s0b1n0"
      ]
    }
  }
]

Membership:

$ ochami smd group member get fish -F json-pretty
{
  "ids": [
    "x3000c0s0b0n0",
    "x3000c0s0b1n0"
  ]
}

These seem to work.

3. Updating Membership

$ ochami smd group member set fish x3000c0s0b1n0 x3000c0s0b2n0 --log-level debug --log-format basic
DEBUG | lib.go:177 > logging has been initialized
DEBUG | lib.go:309 > using base URI from default cluster redondo
DEBUG | lib.go:405 > authentication enabled for cluster redondo, reading and checking token
DEBUG | lib.go:437 > Determining token from environment variable based on cluster in config file
DEBUG | lib.go:443 > --cluster not specified, using default-cluster: redondo
DEBUG | lib.go:454 > Reading token from environment variable: REDONDO_ACCESS_TOKEN
DEBUG | lib.go:456 > Token found from environment variable: REDONDO_ACCESS_TOKEN=ey...
DEBUG | client.go:245 > PUT: https://redondo.usrc:8443/hsm/v2/groups/fish/members
DEBUG | client.go:266 > Request headers:
DEBUG | client.go:268 >   User-Agent: [ochami/0.5.1]
DEBUG | client.go:268 >   Authorization: [Bearer ey...]
DEBUG | client.go:274 > Request body:
DEBUG | client.go:275 > {"label":"fish","ids":["x3000c0s0b1n0","x3000c0s0b2n0"]}
DEBUG | client.go:288 > Response status: 201 Created
DEBUG | client.go:290 > Response headers:
DEBUG | client.go:292 >   Date: [Fri, 29 Aug 2025 18:10:36 GMT]
DEBUG | client.go:292 >   Content-Length: [106]
DEBUG | client.go:292 >   Content-Type: [application/json]
DEBUG | client.go:292 >   Location: [/hsm/v2/groups]
DEBUG | client.go:305 > Response body:
DEBUG | client.go:306 > [{"URI":"/hsm/v2/groups/fish/members/x3000c0s0b1n0"},{"URI":"/hsm/v2/groups/fish/members/x3000c0s0b2n0"}]

INFO  | http.go:124 > Response status: HTTP/2.0 201 Created

Logs:

smd[3266131]: 172.16.0.254 - - [29/Aug/2025:18:09:07 +0000] "PUT /hsm/v2/groups/fish/members HTTP/1.1" 201 106 "" "ochami/0.5.1"
smd[3266131]: 6:09PM INF Request bytes_in=56 bytes_out=106 duration=10.978092 method=PUT remote_addr=172.16.0.254 request_id=smd/Wa7VWeRz2q-000106 request_uri=/hsm/v2/groups/fish/members status=Created status_code=201 user_agent=ochami/0.5.1
smd[3266131]: 2025/08/29 18:09:07 [smd/Wa7VWeRz2q-000106] "PUT http://redondo.usrc:8443/hsm/v2/groups/fish/members HTTP/1.1" from 172.16.0.254 - 201 106B in 11.057523ms

4. Checking Output Again

$ ochami smd group get --name fish -F json-pretty
[
  {
    "description": "a fish",
    "label": "fish",
    "members": {
      "ids": [
        "x3000c0s0b1n0",
        "x3000c0s0b2n0"
      ]
    }
  }
]
$ ochami smd group member get fish -F json-pretty
{
  "ids": [
    "x3000c0s0b1n0",
    "x3000c0s0b2n0"
  ]
}

@shunr-hpe
Copy link
Collaborator Author

@synackd Thanks for testing that with the ochami cli. I should have thought of testing with that.

Your results are all that there is to see and they worked as expected. The put call successfully changed the group members.

For the log output there are no expected additional logs. This PR adds a call to DrainAndCloseRequestBody which does not log anything
https://github.com/Cray-HPE/hms-base/blob/master/http.go#L239-L244

This call to DrainAndCloseRequestBody is like the other calls in smd-api.go. For example:
https://github.com/OpenCHAMI/smd/blob/main/cmd/smd/smd-api.go#L345-L356

The historical context is that the DrainAndCloseRequestBody was created in CSM after OpenCHAMI forked smd. And those drain changes were merged into OpenCHAMI after the doGroupMembersPut was added in OpenCHAMI SMD.

Copy link
Contributor

@synackd synackd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification. Since the testing passed then, I approve. LGTM.

@synackd synackd merged commit a52e4f1 into main Aug 29, 2025
7 checks passed
@shunr-hpe shunr-hpe deleted the add-drain-and-close-request-body branch August 29, 2025 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants