Skip to content

Migrate storage version of networking APIs to v1 #3498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 2, 2025

Conversation

kfaseela
Copy link
Member

Signed-off-by: Faseela K <faseela.k@est.tech>
@kfaseela kfaseela requested a review from a team as a code owner April 28, 2025 21:04
@istio-policy-bot
Copy link

🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test?

Courtesy of your friendly test nag.

@istio-policy-bot
Copy link

😊 Welcome @kfaseela! This is either your first contribution to the Istio api repo, or it's been
a while since you've been here.

You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

@istio-testing istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 28, 2025
@kfaseela kfaseela added the release-notes-none Indicates a PR that does not require release notes. label Apr 28, 2025
@kfaseela
Copy link
Member Author

@keithmattix : wondering why make gen did not set storageVersion: true for these CRDs in crd.gen.yaml

Signed-off-by: Faseela K <faseela.k@est.tech>
@istio-testing istio-testing added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 28, 2025
@keithmattix
Copy link
Contributor

Hmm looks like this doesn't work the way I remember it working. Will take a look

@keithmattix
Copy link
Contributor

Ah, I remember now @kfaseela. We need to manually edit the versions that we need to make the storage version. So e.g. nothing under v1alpha3 should have storage version; remove that and manually add the comment to the v1 copies

Signed-off-by: Faseela K <faseela.k@est.tech>
Signed-off-by: Faseela K <faseela.k@est.tech>
@kfaseela kfaseela added the do-not-merge Block automatic merging of a PR. label Apr 28, 2025
Copy link
Contributor

@keithmattix keithmattix left a comment

Choose a reason for hiding this comment

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

This looks right; let's try and get this merged soon so automator will pull it into istio/istio and we can confirm that the tests work

@keithmattix
Copy link
Contributor

@dhawton do we need to cherrypick the api so this is available for 1.26?

@dhawton
Copy link
Member

dhawton commented Apr 29, 2025

@dhawton do we need to cherrypick the api so this is available for 1.26?

Yes. We're outside of the window for merge so it'd need TOC approval if it were to be brought into 1.26. So if you/TOC desires it, then it will need the label to be in there.

@keithmattix
Copy link
Contributor

Polling @istio/technical-oversight-committee

@keithmattix
Copy link
Contributor

We're really close to 1.26 release and I'd prefer not to throw a wrench in it. Let's target 1.27 with this

@kfaseela kfaseela removed the do-not-merge Block automatic merging of a PR. label May 2, 2025
@kfaseela
Copy link
Member Author

kfaseela commented May 2, 2025

1.27 is fine!

@istio-testing istio-testing merged commit 6944c75 into istio:master May 2, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes-none Indicates a PR that does not require release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate storage version for the Istio CRDs to v1
6 participants