Skip to content

⚠️ Standardize flavor in OpenStackMachine#3087

Open
nikParasyr wants to merge 2 commits intokubernetes-sigs:mainfrom
nikParasyr:v1beta2_flavor
Open

⚠️ Standardize flavor in OpenStackMachine#3087
nikParasyr wants to merge 2 commits intokubernetes-sigs:mainfrom
nikParasyr:v1beta2_flavor

Conversation

@nikParasyr
Copy link
Copy Markdown
Contributor

@nikParasyr nikParasyr commented Mar 30, 2026 •
edited
Loading

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
  2. Docs are not updated yet and use v1beta1. Not sure if I should update them now or after all the api changes are done.

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 •
edited
Loading

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

Name Link
🔨 Latest commit e802d5d
🔍 Latest deploy log https://app.netlify.com/projects/kubernetes-sigs-cluster-api-openstack/deploys/69d4fb449e262100089f128d
😎 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. labels Mar 30, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Mar 30, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign stephenfin for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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 size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label 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 4 times, most recently from 6650135 to cf319fc Compare March 30, 2026 18:20
@nikParasyr
Copy link
Copy Markdown
Contributor Author

nikParasyr commented Mar 30, 2026

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

1 similar comment
@nikParasyr
Copy link
Copy Markdown
Contributor Author

nikParasyr commented Mar 31, 2026

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

@nikParasyr
Copy link
Copy Markdown
Contributor Author

nikParasyr commented Mar 31, 2026

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

@nikParasyr nikParasyr force-pushed the v1beta2_flavor branch 4 times, most recently from 6763918 to 050c69d Compare April 3, 2026 09:22
@nikParasyr
Standardize flavor in OpenStackMachine
30a045e 
Update v1beta2 to standardize flavor into a struct of ID
and Filter, similar to other fields.
@nikParasyr
Copy link
Copy Markdown
Contributor Author

nikParasyr commented Apr 3, 2026

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

bnallapeta
bnallapeta reviewed Apr 6, 2026
View reviewed changes
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.

api/v1beta2/types.go
type FlavorParam struct {
// ID is the uuid of the flavor. ID will not be validated before use.
// +optional
ID optional.String `json:"id,omitempty"`
Copy link
Copy Markdown
Contributor

@bnallapeta bnallapeta Apr 6, 2026

Choose a reason for hiding this comment

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

Q: If ImageParam.ID is required, then FlavourParam also should be required no?

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

@bnallapeta bnallapeta Apr 6, 2026

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?

@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
Copy link
Copy Markdown
Contributor Author

nikParasyr commented Apr 8, 2026

/retest

3 similar comments
@nikParasyr
Copy link
Copy Markdown
Contributor Author

nikParasyr commented Apr 8, 2026

/retest

@nikParasyr
Copy link
Copy Markdown
Contributor Author

nikParasyr commented Apr 8, 2026

/retest

@nikParasyr
Copy link
Copy Markdown
Contributor Author

nikParasyr commented Apr 9, 2026

/retest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stephenfin stephenfin Awaiting requested review from stephenfin

1 more reviewer

@bnallapeta bnallapeta bnallapeta left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

CAPO Roadmap
Status: Inbox

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

✨ v1beta2: Standardize flavor in OpenStackMachine

3 participants

@nikParasyr @k8s-ci-robot @bnallapeta