🌱 Deprecate v1alpha3 & v1alpha4

[sbueringer]

Signed-off-by: Stefan Büringer buringerst@vmware.com

What this PR does / why we need it:

Note: The apiVersions were already deprecated before, we just forgot to mark the Go types accordingly.

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

[sbueringer]

cc @CecileRobertMichon @fabriziopandini

[CecileRobertMichon]

lgtm, just one question

[fabriziopandini]

/lgtm

I'm +1 to deprecate packages, it is simple and it is recognized by IDE and tools.
I'm +1 to deprecate top-level types, it reinforces the message.
I'm ok with not adding a deprecated notice on sub-level types and fields given 1 and 2
I don't think we should add noise to logs, unless we find a way to add a warning the appears once in the controllers bootstrap sequence.

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 3c8a316f5f4ba78d8b82679a9f2cf8f81e9e8b58

[killianmuldoon]

We should also think about some of the provider contracts which are only there to provide backward compatibility e.g. #8096.

Maybe I should open an issue to find and track these? WDYT @sbueringer @fabriziopandini @CecileRobertMichon ?

[sbueringer]

We should also think about some of the provider contracts which are only there to provide backward compatibility e.g. #8096.

Maybe I should open an issue to find and track these? WDYT @sbueringer @fabriziopandini @CecileRobertMichon ?

Should be fine to open an issue to track things like this. But how is this related to dropping v1alpha3 and v1alpha4?

[killianmuldoon]

Should be fine to open an issue to track things like this. But how is this related to dropping v1alpha3 and v1alpha4?

AFAIU this field - providerMachine .spec.failureDomain is part of the v1alpha3 contract, so there is code covering it for migration purposes. My assumption is we'll want to remove this code when we remove v1alpha3, but items that are in the old contract may not be obvious, so it might take some work to document and list everything.

[killianmuldoon]

This can probably be covered by the issue at #8038 - didn't realize I commented this on the PR - will continue the discussion there.

[sbueringer]

@CecileRobertMichon @jackfrancis any objections to merging this as is?

I'm open to follow-up issues or PRs if someone wants to do more

[CecileRobertMichon]

/lgtm
/approve
/hold

feel free to remove the hold

Jack is oof on paternity leave so he might not get back to you for a while

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [CecileRobertMichon]

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

[sbueringer]

Alright, thx!

/hold cancel

[sbueringer]

AFAIU this field - providerMachine .spec.failureDomain is part of the v1alpha3 contract, so there is code covering it for migration purposes. My assumption is we'll want to remove this code when we remove v1alpha3, but items that are in the old contract may not be obvious, so it might take some work to document and list everything.

My plan was to remove the v1alpha3 & v1alpha4 API types and packages in core CAPI. I wasn't planning to touch the contract stuff.

I think we should have a separate discussion & issue about leftovers of previous contracts. In my mind the contract covers how core Cluster API interacts with providers (specifically bootstrap, control plane, infra resources). As far as I can tell a specific Cluster API version only is supposed to support one contract "version" (e.g. v1beta1) at a time. If we still have code in Cluster API today that covers "previous contract versions" I think we now have to consider this as part of the v1beta1 contract. We should figure out what code that exactly is and then I think we should deprecate this behavior ("with the v1beta1 contract") and can eventually get rid of it once we bump the contract to v1beta2.

[sbueringer]

^^ @killianmuldoon