🌱 add checker to validate conditions for v1beta2#12111
🌱 add checker to validate conditions for v1beta2#12111k8s-ci-robot merged 5 commits intokubernetes-sigs:mainfrom
Conversation
e107322 to
6dc0ee3
Compare
6dc0ee3 to
0abf799
Compare
|
/retest |
0abf799 to
aac7c75
Compare
aac7c75 to
ab8f1eb
Compare
|
/close |
|
@sivchari: Closed this PR. DetailsIn response to this:
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-sigs/prow repository. |
|
/reopen |
|
@sivchari: Reopened this PR. DetailsIn response to this:
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-sigs/prow repository. |
ab8f1eb to
3e41252
Compare
test/e2e/clusterctl_upgrade.go
Outdated
| } | ||
|
|
||
| Byf("Verify v1beta2 Available and Ready conditions (if exist) to be true for Cluster and Machines") | ||
| verifyV1Beta2ConditionsTrueV1Beta1(ctx, input.BootstrapClusterProxy.GetClient(), workloadClusterName, workloadClusterNamespace, |
There was a problem hiding this comment.
| verifyV1Beta2ConditionsTrueV1Beta1(ctx, input.BootstrapClusterProxy.GetClient(), workloadClusterName, workloadClusterNamespace, | |
| verifyV1Beta2ConditionsTrueV1Beta1(ctx, managementClusterProxy, workloadClusterName, workloadClusterNamespace, |
Should this be managementClusterProxy?
(like in l.726)
test/e2e/scale.go
Outdated
| clusterNamesToDelete := []string{} | ||
| for _, result := range clusterCreateResults { | ||
| clusterNamesToDelete = append(clusterNamesToDelete, result.clusterName) | ||
| for _, cluster := range clusterCreateResults { |
There was a problem hiding this comment.
Not sure if I would change the scale test. Or at least it would not be the first one where I would add this validation.
- These conditions won't be true if
input.SkipWaitForCreationis true - It's not easy to verify if this test is still green at high scale
Can we maybe in this PR rather add this validation to our other e2e tests? I think that would be more valuable
There was a problem hiding this comment.
Currently, v1veta1 schema are only registered in 2 file I modified. So to add this validation to other e2e tests, do we enable v1beta1 scheme to whole tests ?
There was a problem hiding this comment.
Ah I see. Let's create a copy of the verify func that uses v1beta2 and use that in other places
clusterctl upgrade is a special case. Not sure why we have v1beta1 in the scale test :D
| return c.Get(ctx, key, cluster) | ||
| }, 3*time.Minute, 3*time.Second).Should(Succeed(), "Failed to get Cluster object %s", klog.KRef(clusterNamespace, clusterName)) | ||
|
|
||
| if cluster.Status.V1Beta2 != nil && len(cluster.Status.V1Beta2.Conditions) > 0 { |
There was a problem hiding this comment.
Looks like we need a rebase (cluster.Status.v1beta2 doesn't exist anymore)
f334735 to
58ff884
Compare
Signed-off-by: sivchari <shibuuuu5@gmail.com>
Signed-off-by: sivchari <shibuuuu5@gmail.com>
Signed-off-by: sivchari <shibuuuu5@gmail.com>
Signed-off-by: sivchari <shibuuuu5@gmail.com>
58ff884 to
671164e
Compare
|
lgtm pending fix to CI failures |
cf9bd3c to
12432c0
Compare
|
/lgtm |
|
LGTM label has been added. DetailsGit tree hash: e45ba3e949e8ff3c6a96d95fa74765596d6ac66d |
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/area ci
/area testing
/area e2e-testing
close #11963
What this PR does / why we need it:
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 #