Skip to content

⚠️ Standardize flavor in OpenStackMachine#3087

Merged
k8s-ci-robot merged 1 commit intokubernetes-sigs:mainfrom
nikParasyr:v1beta2_flavor
Apr 17, 2026
Merged

⚠️ Standardize flavor in OpenStackMachine#3087
k8s-ci-robot merged 1 commit intokubernetes-sigs:mainfrom
nikParasyr:v1beta2_flavor

Conversation

@nikParasyr
Copy link
Copy Markdown
Contributor

@nikParasyr nikParasyr commented Mar 30, 2026

What this PR does / why we need it:
Update v1beta2 to standardize flavor into a struct of ID and Filter, similar to other fields.

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 #2896

Special notes for your reviewer:

  1. There is a weird interaction between OpenStackMachine and OpenStackServer as the latter retains the old Flavor and FlavorID

TODOs:

  • squashed commits
  • if necessary:
    • includes documentation
    • adds unit tests

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 30, 2026
@netlify
Copy link
Copy Markdown

netlify bot commented Mar 30, 2026

Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

Name Link
🔨 Latest commit 3e049fc
🔍 Latest deploy log https://app.netlify.com/projects/kubernetes-sigs-cluster-api-openstack/deploys/69e1d2d8fe611800083794b8
😎 Deploy Preview https://deploy-preview-3087--kubernetes-sigs-cluster-api-openstack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 30, 2026
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 30, 2026
@nikParasyr nikParasyr force-pushed the v1beta2_flavor branch 10 times, most recently from 050c69d to 30a045e Compare April 3, 2026 10:10
@nikParasyr
Copy link
Copy Markdown
Contributor Author

/test pull-cluster-api-provider-openstack-e2e-full-test

Copy link
Copy Markdown
Contributor

@bnallapeta bnallapeta left a comment

Choose a reason for hiding this comment

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

Mostly looks good. Minor comments.

Comment thread api/v1beta2/types.go
Comment thread api/v1beta2/types.go
return f.Name == nil && len(f.Tags) == 0
}

// FlavorParam describes a nova flavor. It can be specified by ID or filter
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.

perhaps add tests for this one?

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.

I increased the test coverage, can you elaborate on what tests you are missing?

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.

Thanks. I see the new tests.

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 7, 2026
@nikParasyr nikParasyr force-pushed the v1beta2_flavor branch 2 times, most recently from e802d5d to 6d48213 Compare April 14, 2026 06:48
@nikParasyr
Copy link
Copy Markdown
Contributor Author

/test pull-cluster-api-provider-openstack-e2e-full-test

Comment thread api/v1beta2/types.go
Comment thread api/v1beta1/conversion.go
Comment thread api/v1beta2/types.go
return f.Name == nil && len(f.Tags) == 0
}

// FlavorParam describes a nova flavor. It can be specified by ID or filter
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.

I increased the test coverage, can you elaborate on what tests you are missing?

@nikParasyr nikParasyr force-pushed the v1beta2_flavor branch 3 times, most recently from 37d3ea7 to 982a050 Compare April 14, 2026 07:53
@nikParasyr
Copy link
Copy Markdown
Contributor Author

/test pull-cluster-api-provider-openstack-e2e-full-test

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 14, 2026
Copy link
Copy Markdown
Contributor

@lentzi90 lentzi90 left a comment

Choose a reason for hiding this comment

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

Overall really nice!
I have a couple of nits for the unit tests and then a question on what to do with the new filter options that are not fully implemented. See below.

Comment thread pkg/webhooks/openstackmachine_webhook_test.go Outdated
Comment thread pkg/webhooks/openstackmachine_webhook_test.go
Comment thread controllers/openstackmachine_controller.go
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 15, 2026
@nikParasyr
Copy link
Copy Markdown
Contributor Author

/test pull-cluster-api-provider-openstack-e2e-full-test

@nikParasyr
Copy link
Copy Markdown
Contributor Author

@lentzi90 this is ready for a re-check, i think ive addressed all your remarks

@nikParasyr nikParasyr requested a review from lentzi90 April 15, 2026 12:11
@bnallapeta
Copy link
Copy Markdown
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 16, 2026
Copy link
Copy Markdown
Contributor

@lentzi90 lentzi90 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!
This looks ready to merge.
/approve

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lentzi90

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 16, 2026
@lentzi90
Copy link
Copy Markdown
Contributor

/retest

@lentzi90
Copy link
Copy Markdown
Contributor

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 17, 2026
Update v1beta2 to standardize flavor into a struct of ID
and Filter, similar to other fields.
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 17, 2026
@bnallapeta
Copy link
Copy Markdown
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 17, 2026
@k8s-ci-robot k8s-ci-robot merged commit 0f0b378 into kubernetes-sigs:main Apr 17, 2026
18 checks passed
@github-project-automation github-project-automation bot moved this from Inbox to Done in CAPO Roadmap Apr 17, 2026
@nikParasyr nikParasyr deleted the v1beta2_flavor branch April 17, 2026 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

✨ v1beta2: Standardize flavor in OpenStackMachine

4 participants