🌱 E2E tests for self-hosted upgrades

[tcordeu]

What this PR does / why we need it:
Add E2E tests:

• Upgrade a single control plane, self-hosted cluster.
• Upgrade an HA control plane, self-hosted cluster.

Which issue(s) this PR fixes
Fixes #3728

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign neolit123 after the PR has been reviewed.
You can assign the PR to them by writing /assign @neolit123 in a comment when ready.

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

Needs approval from an approver in each of these files:

• OWNERS

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]

Hi @tcordeu. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[tcordeu]

Even tough the tests seem to be running fine I am seeing logs like this when testing the upgrades:

INFO: Waiting for etcd to have the upgraded image tag
INFO: Got error while streaming logs for pod capi-kubeadm-bootstrap-system/capi-kubeadm-bootstrap-controller-manager-b46d954bc-dhqj4, container manager: http2: server sent GOAWAY and closed the connection; LastStreamID=24623, ErrCode=NO_ERROR, debug=""
INFO: Got error while streaming logs for pod capd-system/capd-controller-manager-845bbfdf7d-wpdnb, container manager: http2: server sent GOAWAY and closed the connection; LastStreamID=24623, ErrCode=NO_ERROR, debug=""
INFO: Got error while streaming logs for pod capd-system/capd-controller-manager-845bbfdf7d-wpdnb, container kube-rbac-proxy: http2: server sent GOAWAY and closed the connection; LastStreamID=24623, ErrCode=NO_ERROR, debug=""
INFO: Got error while streaming logs for pod capi-kubeadm-control-plane-system/capi-kubeadm-control-plane-controller-manager-6b54c76ff9-rt7l8, container kube-rbac-proxy: http2: server sent GOAWAY and closed the connection; LastStreamID=24623, ErrCode=NO_ERROR, debug=""
INFO: Got error while streaming logs for pod capi-system/capi-controller-manager-7c6fc9fcbf-4tp7c, container kube-rbac-proxy: http2: server sent GOAWAY and closed the connection; LastStreamID=24623, ErrCode=NO_ERROR, debug=""
INFO: Got error while streaming logs for pod capi-system/capi-controller-manager-7c6fc9fcbf-4tp7c, container manager: http2: server sent GOAWAY and closed the connection; LastStreamID=24623, ErrCode=NO_ERROR, debug=""
INFO: Got error while streaming logs for pod capi-kubeadm-bootstrap-system/capi-kubeadm-bootstrap-controller-manager-b46d954bc-dhqj4, container kube-rbac-proxy: http2: server sent GOAWAY and closed the connection; LastStreamID=24623, ErrCode=NO_ERROR, debug=""
STEP: PASSED!

@fabriziopandini, do you have any idea as to why this is happening?

[fabriziopandini]

@tcordeu changes looks fine, however, having in mind #4672, I'm wondering if we should change a little bit approach and have two test suites instead of having two test specs inside the same suite.

In practice, we need to change test/e2e/self_hosted_test.go in order to have

var _ = Describe("When testing Cluster API working on single control plane self-hosted clusters", func() {
	SelfHostedSpec(context.TODO(), func() SelfHostedSpecInput {
		return SelfHostedSpecInput{
			...
			ControlPlaneMachineCount: 1,
		}
	})
})

var _ = Describe("When testing Cluster API working on HA self-hosted clusters", func() {
	SelfHostedSpec(context.TODO(), func() SelfHostedSpecInput {
		return SelfHostedSpecInput{
			...
			ControlPlaneMachineCount: 3,
		}
	})
})

And make the new field SelfHostedSpecInput.ControlPlaneMachineCount to flow down to ApplyClusterTemplateAndWait (Last but not least, we should revert back to having a single It statement inside SelfHostedSpec)
wdyt?

[fabriziopandini]

INFO: Got error while streaming logs for pod capi-kubeadm-bootstrap-system/capi-kubeadm-bootstrap-controller-manager-b46d954bc-dhqj4, container manager: http2: server sent GOAWAY and closed the connection; LastStreamID=24623, ErrCode=NO_ERROR, debug=""

@fabriziopandini, do you have any idea as to why this is happening?

I assume the error happens because when we install providers, we start a go routine that streams logs from the pods hosting our controllers; however, if those pods gets destroyed/re-created to another machine (like during an upgrade), the streaming code in the go routine fails.
I don't have a clear idea on how to fix it, I will open an issue to track this

[tcordeu]

@tcordeu changes looks fine, however, having in mind #4672, I'm wondering if we should change a little bit approach and have two test suites instead of having two test specs inside the same suite.

In practice, we need to change test/e2e/self_hosted_test.go in order to have

var _ = Describe("When testing Cluster API working on single control plane self-hosted clusters", func() {
	SelfHostedSpec(context.TODO(), func() SelfHostedSpecInput {
		return SelfHostedSpecInput{
			...
			ControlPlaneMachineCount: 1,
		}
	})
})

var _ = Describe("When testing Cluster API working on HA self-hosted clusters", func() {
	SelfHostedSpec(context.TODO(), func() SelfHostedSpecInput {
		return SelfHostedSpecInput{
			...
			ControlPlaneMachineCount: 3,
		}
	})
})

And make the new field SelfHostedSpecInput.ControlPlaneMachineCount to flow down to ApplyClusterTemplateAndWait (Last but not least, we should revert back to having a single It statement inside SelfHostedSpec)
wdyt?

Seems like a good idea considering #4672, I will change it!

[tcordeu]

@fabriziopandini I implemented the changes you proposed, please give it a look when you're free!

[fabriziopandini]

/ok-to-test

[fabriziopandini]

/lgtm
/test pull-cluster-api-e2e-full-main
To force the test to run

[fabriziopandini]

@tcordeu the test is failing
FYI we just updated instructions on how to run/debug E2E tests locally, see https://master.cluster-api.sigs.k8s.io/developer/testing.html#running-the-end-to-end-tests-locally

[tcordeu]

Upon further review KubernetesVersion and KubernetesVersionUpgradeTo have the same value (v1.19.11), so the update was not doing much... (even tough the original error seems to be etcd taking too long to update).
@fabriziopandini I have two ideas:

• Using KubernetesVersionUpgradeFrom (v1.18.19) and testing v1.18 to v1.19. AFAIK, this would require using an older version of clusterctl (see https://github.com/kubernetes-sigs/cluster-api/blob/master/test/e2e/clusterctl_upgrade.go)
• Using a more recent version to upgrade, maybe v1.19.11 to v1.20.

What do you think?

[fabriziopandini]

@tcordeu god catch.
Being this now an upgrade test, it should use KUBERNETES_VERSION_UPGRADE_FROM (instead of KUBERNETES_VERSION) and KUBERNETES_VERSION_UPGRADE_TO

[CecileRobertMichon]

/lgtm cancel

[k8s-ci-robot]

@tcordeu: PR needs rebase.

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.

[fabriziopandini]

Lets use the global contest introduced by #4706 instead of context.TODO

[fabriziopandini]

@tcordeu do you have time to rebase this PR?

[k8s-ci-robot]

@tcordeu: 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-full-main	5010a23	link	/test pull-cluster-api-e2e-full-main
pull-cluster-api-test-main-mink8s	5010a23	link	/test pull-cluster-api-test-main-mink8s
pull-cluster-api-verify-main	5010a23	link	true	/test pull-cluster-api-verify-main
pull-cluster-api-test-mink8s-main	5010a23	link	true	/test pull-cluster-api-test-mink8s-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]

/close
due to inactivity

[k8s-ci-robot]

@fabriziopandini: Closed this PR.

In response to this:

/close
due to inactivity

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.