🐛 Restore v1alpha3/v1alpha4 conversion to fix SSA issue & add e2e test coverage

[sbueringer]

What this PR does / why we need it:

This PR:

• Fixes the issue with old apiVersions in managedFields + SSA by restoring conversion of v1alpha3 & v1alpha4
• Adds corresponding e2e test coverage to verify the fix works and avoid regressions

More details on the issue soon.

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

[sbueringer]

/test pull-cluster-api-e2e-main

[sbueringer]

/test ?

[k8s-ci-robot]

@sbueringer: The following commands are available to trigger required jobs:

• /test pull-cluster-api-build-main
• /test pull-cluster-api-e2e-blocking-main
• /test pull-cluster-api-e2e-conformance-ci-latest-main
• /test pull-cluster-api-e2e-conformance-main
• /test pull-cluster-api-e2e-dualstack-and-ipv6-main
• /test pull-cluster-api-e2e-main
• /test pull-cluster-api-e2e-mink8s-main
• /test pull-cluster-api-e2e-upgrade-1-29-1-30-main
• /test pull-cluster-api-test-main
• /test pull-cluster-api-test-mink8s-main
• /test pull-cluster-api-verify-main

The following commands are available to trigger optional jobs:

• /test pull-cluster-api-apidiff-main

Use /test all to run the following jobs that were automatically triggered:

• pull-cluster-api-apidiff-main
• pull-cluster-api-build-main
• pull-cluster-api-e2e-blocking-main
• pull-cluster-api-test-main
• pull-cluster-api-verify-main

In response to this:

/test ?

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]

/test pull-cluster-api-e2e-conformance-ci-latest-main
/test pull-cluster-api-e2e-upgrade-1-29-1-30-main

[sbueringer]

/test pull-cluster-api-e2e-main

[k8s-ci-robot]

@sbueringer: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-e2e-conformance-ci-latest-main	d30cb88	link	true	/test pull-cluster-api-e2e-conformance-ci-latest-main
pull-cluster-api-e2e-upgrade-1-29-1-30-main	d30cb88	link	true	/test pull-cluster-api-e2e-upgrade-1-29-1-30-main

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

[fabriziopandini]

Awesome!
This is a big PR and a lot of work in such a short time and I would like to praise a few things make this PR to stand out

• Restoring APIs in internal packages really helps in keeping the public API surfaces as minimal and possible
• Adding E2e test validating a double clusterctl upgrade allows us to make this change with confidence (and we will benefit from this as well down the line when working to a more sophisticated solution)
• The trick to force controller manager restart is a real piece of art

just a few nits, nothing really blocking

[sbueringer]

/test pull-cluster-api-e2e-main

[fabriziopandini]

/lgtm
/approve
Feel free to unwip/unhold when the test are completed

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: fd27c37fb7323e2fc2188bf2875f79388daa56fe

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

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 [fabriziopandini]

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

[killianmuldoon]

I haven't done a full review of this -but I'm trying to understand the change and the root cause.

Do we need to keep apiVersions around pretty much indefinitely - but with served: false in order to keep the conversion webhooks working?

At least until we can be sure that all apiVersions in the managed fields have been updated?

[sbueringer]

I haven't done a full review of this -but I'm trying to understand the change and the root cause.

Do we need to keep apiVersions around pretty much indefinitely - but with served: false in order to keep the conversion webhooks working?

At least until we can be sure that all apiVersions in the managed fields have been updated?

Yup. That's exactly it on a high-level. Fabrizio will share more details in the office hours (please check the agenda the information is already there, we'll update the issue later accordingly as well).

This PR and its cherry-pick are only meant as a temporary solution until we figure out the right way to handle this problem. The idea was to only restore the minimal amount necessary to get conversion working again (which is the reason why we restored v1alpha3/v1alpha4 under internal).

[sbueringer]

P.S. Happy to follow-up if you have further findings

[sbueringer]

P.S.2 Sorry for the short time frame to review. Trying to get this fixed up before I'm going on PTO on Friday and in time so we have some time in CI before the 1.6 patch release.