馃尡 Follow-up fixes: Add MachinePools to Runtime SDK and Rollout tests #9719
Conversation
k8s-ci-robot
added
area/testing
k8s-ci-robot
added
the
size/L
|
/test pull-cluster-api-e2e-full-main |
1 similar comment
|
/test pull-cluster-api-e2e-full-main |
willie-yao
force-pushed
the
fix-mp-cc-e2e
branch
2 times, most recently
from
ce8bf21
to
957c082
Compare
|
/test pull-cluster-api-e2e-full-main |
|
/retest |
|
/test pull-cluster-api-e2e-full-main |
|
/retest |
willie-yao
force-pushed
the
fix-mp-cc-e2e
branch
from
4b7a33e
to
faac687
Compare
|
Let's also run e2e-full |
|
/test pull-cluster-api-e2e-full-main |
|
/retest |
| @@ -186,7 +186,7 @@ func ClusterClassRolloutSpec(ctx context.Context, inputGetter func() ClusterClas | |||
| }, | |||
There was a problem hiding this comment.
Based on e2e-full we're checking for MD instead of MP somewhere in the new MP validations:
key "machinedeployment.clusters.x-k8s.io/revision" does not exist in map map[Cluster.topology.machinePool.annotation:Cluster.topology.machinePool.annotationValue ClusterClass.machinePool.annotation:ClusterClass.machinePool.annotationValue]
(I assume it's only surfaced on this PR, because now we're actually validating :))
There was a problem hiding this comment.
It looks like the error happens here, right after applying the cluster template. I think MachineDeployment should exist in this case so I'm wondering what happened here.
There was a problem hiding this comment.
Ah I think it's due to the lack of a revision annotation for MachinePools like it does for MachineDeployments. Removing this without() should fix it. https://github.com/willie-yao/cluster-api/blob/fix-mp-cc-e2e/test/e2e/clusterclass_rollout.go#L647
There was a problem hiding this comment.
Sounds good. Looks like we now hit another similar case
kubectl.kubernetes.io/last-applied-configuration
There was a problem hiding this comment.
I think both InfraMachinePool and BootstrapConfig of a MachinePool don't have the last-applied-configuration annotation. So we can drop l.695 & l.719
There was a problem hiding this comment.
@sbueringer Willie is out sick today so I pushed this change to the branch to speed up getting test signal in case we want to get this in before cutting the RC
There was a problem hiding this comment.
Thx!! I squashed & added co-authored-by
|
@willie-yao @CecileRobertMichon +/- getting e2e-full green lgtm from my side, if you want to merge before the weekend. |
willie-yao
force-pushed
the
fix-mp-cc-e2e
branch
from
8e08815
to
40680e8
Compare
|
/test pull-cluster-api-e2e-full-main |
2 similar comments
|
/test pull-cluster-api-e2e-full-main |
|
/test pull-cluster-api-e2e-full-main |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 8891fe8ddc2ad9275aa93139edbdd017482795ae
|
sbueringer
force-pushed
the
fix-mp-cc-e2e
branch
2 times, most recently
from
37f7fb7
to
c7eb3c3
Compare
k8s-ci-robot
removed
the
lgtm
Co-authored-by: Cecile Robert-Michon <cecilerobertm@gmail.com>
sbueringer
force-pushed
the
fix-mp-cc-e2e
branch
from
c7eb3c3
to
dbdf199
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 8b6a4fc42e2de899da1f20546c974c20710fde51
|
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
What this PR does / why we need it:
This PR follows up #9703 with fixes listed here: #9703 (review)
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 #
/area testing