Skip to content
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

Removing v1alpha3 & v1alpha4 apiVersions #8038

Closed
17 of 18 tasks
sbueringer opened this issue Feb 1, 2023 · 47 comments · Fixed by #9939
Closed
17 of 18 tasks

Removing v1alpha3 & v1alpha4 apiVersions #8038

sbueringer opened this issue Feb 1, 2023 · 47 comments · Fixed by #9939
Assignees
Labels
area/api Issues or PRs related to the APIs kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/release-blocking Issues or PRs that need to be closed before the next CAPI release triage/accepted Indicates an issue or PR is ready to be actively worked on.
Milestone

Comments

@sbueringer
Copy link
Member

sbueringer commented Feb 1, 2023

Motivation

The last releases using v1alpha3 (v0.3.x) and v1alpha4 (v0.4.x) apiVersions have been both EOL since ~ April 2022 (~ a year). I think now it is time to have a plan to remove v1alpha3 & v1alpha4.

Proposal

I would propose the following timeline:

  • Cluster API release v1.4: Announcement of imminent apiVersions removal in v1.5 & v1.6
  • Cluster API release v1.5:
    • v1alpha3: Kubernetes API server will stop serving v1alpha3
      • We will set served to false for the apiVersions in all our CRDs.
      • Then users can’t read or write with the old apiVersions anymore.
      • The API server will still be able to read and convert the old apiVersions.
      • If necessary, users can easily enable the apiVersions again by reverting served back to true on the CRD.
    • v1alpha4: no change
  • Cluster API release v1.6:
    • v1alpha3: Remove v1alpha3 apiVersions
      • Drop all code related to v1alpha3 (API types, conversions, tests, …)
      • If we get feedback that folks need more time to migrate we can discuss delaying this stage by another minor release to give folks more time.
    • v1alpha4: Kubernetes API server will stop serving v1alpha4
      • Same as above
  • Cluster API release v1.7:
    • v1alpha4: Remove v1alpha4 apiVersions
      • Same as above

Tasks

Prerequisites: (have to be done for v1.4.0)

v1.4:

  • Mark v1alpha3 & v1alpha4 Go API types as deprecated: 🌱 Deprecate v1alpha3 & v1alpha4 #8071
    • Note: The apiVersions were already deprecated before, we just forgot to mark the Go types accordingly.
  • Release notes should highlight the imminent removal of the apiVersions in v1.5 & v1.6

v1.5: (@killianmuldoon)

  • v1alpha3: Stop serving via kubebuilder:unservedversion ⚠️ Stop serving v1alpha3 API types #8549

  • Ensure clusterctl upgrade warns users if the upgrade would break them (e.g. v0.3 => v1.x (version without v1alpha3 apiVersion))

    • This should already be covered by our CRD migration code, but let's verify it.
    • This might only become relevant with v1.6 when we actually remove the apiVersion from the CRDs
  • Document the requirement to restart the kube-controller-manager after updating to a CAPI version which no longer serves v1alpha3 📖 Add note about v1alpha3 removal to book #8740

  • Release notes should reflect the changes and make a note of the controller-manager restart requirement.

v1.6: (@killianmuldoon)

v1.7: (@killianmuldoon)

Misc:

/kind cleanup
/area api

@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. area/api Issues or PRs related to the APIs needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 1, 2023
@furkatgofurov7
Copy link
Member

/cc

@sbueringer sbueringer self-assigned this Feb 1, 2023
@sbueringer
Copy link
Member Author

cc @kubernetes-sigs/cluster-api-release-team

@lentzi90
Copy link
Contributor

lentzi90 commented Feb 1, 2023

I see no mention of how to ensure that old objects are "upgraded"?
IIRC this is automatically done when the old version is no longer in the storedVersions AND the object is written to the API. However, there is no guarantee that users will update their objects between releases. I.e. someone could have created a Cluster using v1alpha4, then clusterctl upgrade to a release that no longer supports v1alpha4 and get stuck because the cluster was still stored using v1alpha4.

One way to solve this is the storage version migrator (also linked from ref below). Another option would be to ensure that clusterctl takes care of it as part of clusterctl upgrade. I know cert-manager used their own CLI to handle this when they dropped old API versions.

Ref: https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#previous-storage-versions

Edit: Ref for cert-manager: https://cert-manager.io/docs/installation/upgrading/remove-deprecated-apis/#upgrading-existing-cert-manager-resources

@sbueringer
Copy link
Member Author

sbueringer commented Feb 1, 2023

@lentzi90 Thx for bringing this up. This is a very good point. But I think we have this covered as we ~ implemented what cert-manager has in clusterctl: #6749 + #6691 (comment)

P.S. I think we also have test-coverage for this with v1.6 as the clusterctl upgrade tests should cover this (added a sub-task to verify that)

@lentzi90
Copy link
Contributor

lentzi90 commented Feb 1, 2023

Amazing! Thank you very much for this! Looks like it will even work automagically for the provider CRDs! 🎉

@chrischdi
Copy link
Member

Due to the discussions at the CAPI SIG Meeting:

If v1.6 drops the code (which includes migration won't work anymore):
It could also be considered as a reason to name it v2.0 instead to highlight the breaking change.

Note: from quickly reading the docs (especially https://main.cluster-api.sigs.k8s.io/contributing), I was not able to find a wording about this special case or that we should name bump the major version number.

@CecileRobertMichon
Copy link
Contributor

I was not able to find a wording about this special case or that we should name bump the major version number.

AFAIK we follow semver so I think https://semver.org/ is enough justification

+1 for bumping to v2.0

@sbueringer
Copy link
Member Author

Do we actually follow semver or are we following a Kubernetes-style versioning scheme? (they also treat their API versions totally independent of the Kubernetes version number)

@chrischdi
Copy link
Member

⚠ The project does not follow Go Modules guidelines for compatibility requirements for 1.x semver releases.

Cluster API follows upstream Kubernetes semantic versioning. With the v1 release of our codebase, we guarantee the following:
...
[0]

@CecileRobertMichon
Copy link
Contributor

CecileRobertMichon commented Feb 2, 2023

That's fair, I guess as long as we deprecate the APIs first it's fair game to remove them as a breaking change in a later minor version.

One suggestion: maybe instead of doing both v1alpha3 and v1alpha4 together we should offset them by one minor release, chances of users out there still using v1alpha3 are probably extremely low (and much lower than v1alpha4 which itself is probably low too), so maybe we can do v1alpha3 as a first "safer" step so we can learn from anything unexpected when we do v1alpha4?

@fabriziopandini
Copy link
Member

maybe instead of doing both v1alpha3 and v1alpha4 together we should offset them by one minor release

this seems a good idea to me, with probably a little bit more work, but it seems a good compromise IMO

@sbueringer
Copy link
Member Author

sbueringer commented Feb 2, 2023

So v1alpha3 v1.5 + v1.6 (+ potential delay depending on feedback) and then v1alpha4 with v1.7 + v1.8 (+ potential delay depending on feedback)?

Will probably take us until at least mid 2024 to get there :/

@CecileRobertMichon
Copy link
Contributor

How about:

  • Cluster API release v1.3: Announcement of imminent apiVersions deprecation/removal (this is right now, this issue)
  • Cluster API release v1.4: Kubernetes API server will stop serving v1alpha3 + v1alpha4 is marked as "deprecated" (if it's not already the case)
    • We will set served to false for v1alpha3 in all our CRDs.
    • Then users can’t read or write with v1alpha3 anymore.
    • The API server will still be able to read and convert v1alpha3.
    • If necessary, users can easily enable v1alpha3 again by reverting served back to true on the CRD.
  • Cluster API release v1.5: Kubernetes API server will stop serving v1alpha4
    • Same provisions as above
    • Remove v1alpha3
      • Drop all code related to v1alpha3 (API types, conversions, tests, …)
  • Cluster API release v1.6: v1alpha4 apiVersions
    • Drop all code related to v1alpha4 (API types, conversions, tests, …)
      If we get feedback that folks need more time to migrate after v1.5 we can discuss delaying this stage by another minor release to give folks more time.

This way it's still the same timeline (everything done by v1.6) but we give ourselves more of a safe rollout instead of removing everything at once. Another option is to delay everything by one release (so stop serving v1alpha3 in v1.5 as planned) so everything is done by release v1.7 but I don't think we really need an extra 4 months to announce removal of v1alpha3, it's been deprecated for 2 years (!) https://github.com/kubernetes-sigs/cluster-api/blob/main/CONTRIBUTING.md#support-and-guarantees, it's more of a matter if we think we can get the work needed in time for v1.4 (by end of March).

@sbueringer
Copy link
Member Author

Sounds good to me!

@cpanato
Copy link
Member

cpanato commented Feb 3, 2023

+1

@sbueringer
Copy link
Member Author

Ah no we need the fixes I mentioned above in v1.4.0 first and can start removing in v1.5

@killianmuldoon
Copy link
Contributor

killianmuldoon commented Feb 3, 2023

kubernetes-sigs/cluster-api-provider-kubemark#63

Yeah - We'd need to delay the process by one release, but that sounds alright to me. It could all be done by 1.7 in this case?

@elmiko
Copy link
Contributor

elmiko commented Feb 6, 2023

i'm +1 for splitting the action to remove v1alpha3, then v1alpha4 in the next release. in general the plan here lgtm

@sbueringer
Copy link
Member Author

Thx for the comments. I updated the issue description accordingly.

Please take another look, thx :)

@fabriziopandini
Copy link
Member

+1 for me!

@sbueringer
Copy link
Member Author

I've opened a PR to document the removal plan: #8117

I will also ask for a lazy consensus for the PR in the office hours today to "formalize" the approval of the plan

@killianmuldoon
Copy link
Contributor

Added an item to the task list around communicating the requirement to restart the controller manager in order to get deletion working after updating to a version which doesn't serve v1alpha3.

@sbueringer
Copy link
Member Author

/unassign

@sbueringer
Copy link
Member Author

@killianmuldoon I think the last step should be unblocked now. IIRC you wanted to take over that one once you're back, right? (No rush!!)

@sbueringer sbueringer added this to the v1.7 milestone Dec 8, 2023
@sbueringer sbueringer added the kind/release-blocking Issues or PRs that need to be closed before the next CAPI release label Dec 8, 2023
@killianmuldoon
Copy link
Contributor

Yup - I'll do a PR for this this week!

@sbueringer
Copy link
Member Author

sbueringer commented Jan 29, 2024

/reopen

Not 100% sure how we want to handle the release note thing (i.e. if we want to keep the issue open or if there is a way to get this done now)

@k8s-ci-robot k8s-ci-robot reopened this Jan 29, 2024
@k8s-ci-robot
Copy link
Contributor

@sbueringer: Reopened this issue.

In response to this:

/reopen

Not 100% sure how we want to handle the release note thing (i.e. if we want to keep the issue or if there is a way to get this done now)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@killianmuldoon
Copy link
Contributor

I think it's fine to close the issue - the release note should already be there in a basic form - i.e. with the breaking change. I think it's just important for the release team to be generally aware of it to add the extra information as was done in previous releases.

This issue is probably too complicated to track that, but I'm not sure if the release team generally tracks information for future release notes somewhere.

@sbueringer
Copy link
Member Author

@cahillsf ^^

Sounds reasonable

/close

@k8s-ci-robot
Copy link
Contributor

@sbueringer: Closing this issue.

In response to this:

@cahillsf ^^

Sounds reasonable

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@sbueringer
Copy link
Member Author

Looks like we have to fix a link somehwere: https://github.com/sbueringer/cluster-api/actions/runs/7696026837/job/20970116673

@killianmuldoon
Copy link
Contributor

I'll pick that up

@cahillsf
Copy link
Member

I think it's fine to close the issue - the release note should already be there in a basic form - i.e. with the breaking change. I think it's just important for the release team to be generally aware of it to add the extra information as was done in previous releases.

cool -- i will poke around at how this was communicated the last time we deprecated an API version. please let me know if there was anything aside from clearly indicating this change in the upcoming patch release that the release team needs to do

@killianmuldoon
Copy link
Contributor

I think something like the deprecation notice in https://github.com/kubernetes-sigs/cluster-api/releases/tag/v1.6.0 would be fine i.e. The API version v1alpha4 has been completely removed in this release.

Note: this would be for the minor release v1.7.0, not for an upcoming patch release.

@cahillsf
Copy link
Member

Ah right right, cool sounds good thanks Killian

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Issues or PRs related to the APIs kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/release-blocking Issues or PRs that need to be closed before the next CAPI release triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.