Skip to content

Conversation

@matofeder
Copy link
Member

@matofeder matofeder commented Jan 16, 2025

What this PR does / why we need it:

This PR contains a hot-fix that allows CreateOpts.properties definition via node-images.yaml as follows:

apiVersion: openstack.infrastructure.clusterstack.x-k8s.io/v1alpha1
openStackNodeImages:
  - createOpts:
      container_format: bare
      disk_format: qcow2
      name: ubuntu-capi-image-v1.29.9
      visibility: private
      properties:
        hw_disk_bus: "scsi"
        hw_rng_model: "virtio"
        hw_scsi_model: "virtio-scsi"
    url: ...

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #223

Special notes for your reviewer:

TODOs:

  • squash commits
  • include documentation
  • add unit tests

@matofeder matofeder force-pushed the fix_image_properties branch from 115c18c to afeee9f Compare January 16, 2025 21:42
@matofeder matofeder changed the title Fix image properties propagation 🐛 Fix image properties propagation Jan 16, 2025
Signed-off-by: Matej Feder <[email protected]>
@matofeder matofeder force-pushed the fix_image_properties branch from 43c746c to 3f53487 Compare January 16, 2025 23:03
Copy link
Member

@chess-knight chess-knight left a comment

Choose a reason for hiding this comment

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

LGTM

@jschoone jschoone requested a review from Nils98Ar January 17, 2025 10:33
Copy link
Member

@Nils98Ar Nils98Ar left a comment

Choose a reason for hiding this comment

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

Thank you, looks good!

Is there a reason that hw_disk_bus the only property that you set in the unit tests?

@matofeder
Copy link
Member Author

Is there a reason that hw_disk_bus the only property that you set in the unit tests?

There is no reason, I just arbitrarily picked some.

Copy link
Contributor

@jschoone jschoone 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 fixing it that fast @matofeder.
I can also confirm it works:

openstack image show ubuntu-capi-image-v1.31.4 -c properties -f json
{
  "properties": {
    ...
    "hw_disk_bus": "scsi",
    "hw_rng_model": "virtio",
    "hw_scsi_model": "virtio-scsi"
  }
}

@jschoone
Copy link
Contributor

@matofeder can you create a release with this fix?

@matofeder
Copy link
Member Author

@matofeder can you create a release with this fix?

sure, I will do that asap

@matofeder matofeder merged commit 56aa0f4 into main Jan 17, 2025
11 checks passed
@matofeder matofeder deleted the fix_image_properties branch January 17, 2025 15:30
@Nils98Ar
Copy link
Member

Thanks @matofeder! I didn't expect this to be solved so quickly :)

@matofeder
Copy link
Member Author

I tagged the latest main with v0.1.0-alpha.6, I believe that the github pipeline does the rest (release)..

@matofeder
Copy link
Member Author

matofeder commented Jan 17, 2025

OK, I just had to click on the button and publish the release produced by the pipeline.

https://github.com/SovereignCloudStack/cluster-stack-provider-openstack/releases/tag/v0.1.0-alpha.6

let me know if you encounter any issues

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.

Images miss SCS metadata

5 participants