Fix test framework for eksa version #6278
Conversation
eks-distro-bot
added
the
size/S
Codecov Report
@@ Coverage Diff @@
## main #6278 +/- ##
==========================================
- Coverage 75.17% 75.11% -0.06%
==========================================
Files 448 447 -1
Lines 37592 37242 -350
==========================================
- Hits 28261 27976 -285
+ Misses 7743 7701 -42
+ Partials 1588 1565 -23
|
pkg/controller/clusters/status.go
Outdated
| @@ -22,6 +22,11 @@ func UpdateClusterStatusForControlPlane(ctx context.Context, client client.Clien | |||
| return errors.Wrapf(err, "getting kubeadmcontrolplane") | |||
| } | |||
|
|
|||
| if kcp == nil { | |||
There was a problem hiding this comment.
why this change? it's difficult to understand the changes without any context
There was a problem hiding this comment.
it caused a nil pointer dereference when deleting a workload cluster in TestVSphereUpgradeKubernetes123to124UbuntuWorkloadClusterAPI in updateControlPlaneReadyCondition(cluster, kcp)
There was a problem hiding this comment.
I'd vote to just return nil here instead of also setting the condition then. The ControlPlaneInitialized condition should not be updated again after being "True". This will update the value back to "False"
There was a problem hiding this comment.
is it easier to just add this check to updateControlPlaneReadyCondition?
There was a problem hiding this comment.
yeah we can do that instead
There was a problem hiding this comment.
yeah, i think that solution is better
tatlat
force-pushed
the
fix-test-framework
branch
from
c6925df
to
61c0943
Compare
eks-distro-bot
added
size/XS
eks-distro-bot
added
size/S
|
/lgtm |
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tatlat 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 |
|
/cherry-pick release-0.17 |
|
@tatlat: new pull request created: #6283 In 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/test-infra repository. |
* fix framework * fix nil pointer dereference in e2e * move nil check again * add kcp nil test
Issue #, if available:
Description of changes:
Testing (if applicable):
Documentation added/planned (if applicable):
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.