Skip to content

Baremetal virtual media Get API - #3303

Merged
mandre merged 3 commits into
gophercloud:mainfrom
hroyrh:vmedia_get
Mar 11, 2025
Merged

Baremetal virtual media Get API#3303
mandre merged 3 commits into
gophercloud:mainfrom
hroyrh:vmedia_get

Conversation

@hroyrh

@hroyrh hroyrh commented Feb 17, 2025

Copy link
Copy Markdown
Contributor

Fixes #3302

@github-actions github-actions Bot added the edit:baremetal This PR updates baremetal code label Feb 17, 2025
github-actions[bot]
github-actions Bot reviewed Feb 17, 2025
View reviewed changes

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you for submitting your first PR! Be sure that we will be looking at it but keep in mind
this sometimes takes a while.
Please let the maintainers know if your PR has not got enough attention after a few days.
If any doubt, please consult our PR tutorial.

@github-actions github-actions Bot added the semver:minor Backwards-compatible change label Feb 17, 2025
@coveralls

coveralls commented Feb 17, 2025 •
edited
Loading

Copy link
Copy Markdown

Coverage Status

coverage: 78.686% (+0.007%) from 78.679%
when pulling 6751a7a on hroyrh:vmedia_get
into 5afeaf8 on gophercloud:main.

@Sharpz7

Sharpz7 commented Feb 19, 2025

Copy link
Copy Markdown
Contributor

This looks like a microversion change that is needed. Let me know if you want help implementing that fix

Sharpz7
Sharpz7 reviewed Feb 19, 2025
View reviewed changes
Comment thread internal/acceptance/openstack/baremetal/v1/nodes_test.go
client, err := clients.NewBareMetalV1Client()
th.AssertNoErr(t, err)
client.Microversion = "1.89"
client.Microversion = "1.93"

@Sharpz7 Sharpz7 Feb 19, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Although, it does look like you have updated it, so my confidence on that is low.

@hroyrh hroyrh Feb 21, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the issue is driver related, the test is using ipmi but the feature only supports redfish. I'm looking into fixing it.

@iurygregory

iurygregory commented Mar 3, 2025

Copy link
Copy Markdown
Contributor

the tests are failing, but they are legit failures. get api will only work on redfish based.. so you can remove the change in the acceptance, I will be trying to move things to redfish in another PR and later we can update I would say.

I've manually tested this with ironic directly and things should work, I'm just wondering what we expect as the output when doing

Via curl we would get something like:

[
  {
    "media_types": [
      "Floppy",
      "USBStick"
    ],
    "inserted": false,
    "image": ""
  },
  {
    "media_types": [
      "CD",
      "DVD"
    ],
    "inserted": true,
    "image": "http://<address>/redfish/boot-12345678-aaaa-5856-bfb6-6aaaacd3dd11.iso"
  }
]

With gophercloud doing

result := nodes.GetVirtualMedia(ctx, client, myID)

returns the following:

{{[map[image: inserted:false media_types:[Floppy USBStick]] map[image:http://<address>/redfish/boot-12345678-aaaa-5856-bfb6-6aaaacd3dd11.iso inserted:true media_types:[CD DVD]]] 0 map[Content-Length:[212] Content-Type:[application/json] Date:[Fri, 28 Feb 2025 01:26:50 GMT] Openstack-Request-Id:[req-...] X-Openstack-Ironic-Api-Maximum-Version:[1.95] X-Openstack-Ironic-Api-Minimum-Version:[1.1] X-Openstack-Ironic-Api-Version:[1.93]] <nil>}}

using Result

result := nodes.GetVirtualMedia(ctx, client, myID).Result

{[map[image: inserted:false media_types:[Floppy USBStick]] map[image:http://<address>/redfish/boot-12345678-aaaa-5856-bfb6-6aaaacd3dd11.iso inserted:true media_types:[CD DVD]]] 0 map[Content-Length:[212] Content-Type:[application/json] Date:[Fri, 28 Feb 2025 01:31:18 GMT] Openstack-Request-Id:[req-34c36302-b93f-4492-89a8-c1e27f3a8cbb] X-Openstack-Ironic-Api-Maximum-Version:[1.95] X-Openstack-Ironic-Api-Minimum-Version:[1.1] X-Openstack-Ironic-Api-Version:[1.93]] <nil>}

using Err

result := nodes.GetVirtualMedia(ctx, client, myID).Err

<nil>

using Body

result := nodes.GetVirtualMedia(ctx, client, myID).Body

[map[image: inserted:false media_types:[Floppy USBStick]] map[image:http://<address>/redfish/boot-12345678-aaaa-5856-bfb6-6aaaacd3dd11.iso inserted:true media_types:[CD DVD]]]

@github-actions github-actions Bot removed the semver:minor Backwards-compatible change label Mar 7, 2025
@github-actions

github-actions Bot commented Mar 7, 2025

Copy link
Copy Markdown

Failed to assess the semver bump. See logs for details.

@github-actions github-actions Bot added the semver:minor Backwards-compatible change label Mar 7, 2025
@iurygregory

iurygregory commented Mar 7, 2025

Copy link
Copy Markdown
Contributor

LGTM

@iurygregory

iurygregory commented Mar 7, 2025

Copy link
Copy Markdown
Contributor

@EmilienM when you have time can you check this PR? Thanks!

@mandre
mandre merged commit 1031a87 into gophercloud:main Mar 11, 2025
@hroyrh hroyrh mentioned this pull request Mar 21, 2025
Merged
@pierreprinetti

pierreprinetti commented Mar 23, 2025

Copy link
Copy Markdown
Member

@mandre Why was this not backported to v2?

@mandre

mandre commented Mar 23, 2025

Copy link
Copy Markdown
Contributor

@mandre Why was this not backported to v2?

It was an oversight.

@mandre mandre added the backport-v2 This PR will be backported to v2 label Mar 23, 2025
@github-actions

github-actions Bot commented Mar 23, 2025

Copy link
Copy Markdown

Failed to backport PR to v2 branch. See logs for details.

@mandre

mandre commented Mar 23, 2025

Copy link
Copy Markdown
Contributor

The backport to v2 depends on #3334.

@mandre mandre added backport-v2 This PR will be backported to v2 and removed backport-v2 This PR will be backported to v2 labels Mar 23, 2025
@gophercloud-backport-bot gophercloud-backport-bot Bot mentioned this pull request Mar 23, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments
@Sharpz7 Sharpz7 Sharpz7 left review comments
@mandre mandre mandre approved these changes

Assignees

No one assigned

Labels

backport-v2 This PR will be backported to v2 edit:baremetal This PR updates baremetal code semver:minor Backwards-compatible change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Baremetal: add support for new virtual media GET API

6 participants

@hroyrh @coveralls @Sharpz7 @iurygregory @pierreprinetti @mandre