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.

Sign up for GitHub

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

🌱 Add timeout to check if KCP object exists #5889

Merged
merged 1 commit into from
Jan 3, 2022
Merged

🌱 Add timeout to check if KCP object exists #5889

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions test/framework/controlplane_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,12 +228,15 @@ func DiscoveryAndWaitForControlPlaneInitialized(ctx context.Context, input Disco
Expect(input.Lister).ToNot(BeNil(), "Invalid argument. input.Lister can't be nil when calling DiscoveryAndWaitForControlPlaneInitialized")
Expect(input.Cluster).ToNot(BeNil(), "Invalid argument. input.Cluster can't be nil when calling DiscoveryAndWaitForControlPlaneInitialized")

controlPlane := GetKubeadmControlPlaneByCluster(ctx, GetKubeadmControlPlaneByClusterInput{
Copy link
Member

Choose a reason for hiding this comment

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

after applying the template of KCP, if the KCP object doesn't exist yet, then the test will fail. It can be a bit problematic when the test runs in a system with high latency, and KCP object needs more time to be created.

If I got this right, this is just a problem of a stale cache/slow API server.
In that case, I would just fix it by adding an eventually inside GetKubeadmControlPlaneByCluster waiting for 2-3 sec; also, worth noticing that running tests in environments with resource limits impacting APIserver/etcd performances from one side it is insightful, from the other introduces a lot of variants that makes test automation complex.

Copy link
Member

@sbueringer sbueringer Jan 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@fabriziopandini I think it's not necessarily the stale client-side Client used in the e2e test.

As afaik the ClusterLabel is set by the cluster controller async it could be that the controller hasn't reconciled the control plane yet and thus GetKubeadmControlPlaneByCluster isn't able to get KCP based on the cluster label.

(I'm aware that this PR is already merged and it's fine as is for me too, was just looking at it because of the justification for the cherry-pick PRs :))

Lister: input.Lister,
ClusterName: input.Cluster.Name,
Namespace: input.Cluster.Namespace,
})
Expect(controlPlane).ToNot(BeNil())
var controlPlane *controlplanev1.KubeadmControlPlane
Eventually(func(g Gomega) {
controlPlane = GetKubeadmControlPlaneByCluster(ctx, GetKubeadmControlPlaneByClusterInput{
Lister: input.Lister,
ClusterName: input.Cluster.Name,
Namespace: input.Cluster.Namespace,
})
g.Expect(controlPlane).ToNot(BeNil())
}, "10s", "1s").Should(Succeed())

log.Logf("Waiting for the first control plane machine managed by %s/%s to be provisioned", controlPlane.Namespace, controlPlane.Name)
WaitForOneKubeadmControlPlaneMachineToExist(ctx, WaitForOneKubeadmControlPlaneMachineToExistInput{
Expand Down