Add NodeWrapper for testing #3689
Conversation
k8s-ci-robot
added
release-note-none
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: kaisoz The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
size/XL
✅ Deploy Preview for kubernetes-sigs-kueue ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
/cc @mbobrovskyi @PBundyra |
|
Looks good! Please use wrapper in the other parts of code as well, including:
|
| @@ -0,0 +1,69 @@ | |||
| /* | |||
| Copyright 2023 The Kubernetes Authors. | |||
There was a problem hiding this comment.
| Copyright 2023 The Kubernetes Authors. | |
| Copyright 2024 The Kubernetes Authors. |
|
|
||
| // StatusConditions updates status conditions of the Node. | ||
| func (n *NodeWrapper) StatusConditions(conditions ...corev1.NodeCondition) *NodeWrapper { | ||
| n.Status.Conditions = conditions |
There was a problem hiding this comment.
Maybe append instead of overriding.
| StatusConditions(corev1.NodeCondition{ | ||
| Type: corev1.NodeReady, | ||
| Status: corev1.ConditionTrue, | ||
| }). |
There was a problem hiding this comment.
I think we could also have a helper Ready() for more brevity. It will append the condition calling StatusConditions (thus the other comment). We can still keep the StatusConditions function for other uses.
I'm also ok to address the other files in a follow up, to de-risk conflicts in the meanwhile, but as you prefer @kaisoz . |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Creating nodes for testing is quite verbose. This PR introduces the
NodeWrapperto make them their creation more compact.Which issue(s) this PR fixes:
Fixes #3661
Special notes for your reviewer:
This PR only refactors one test so that you can see how the usage
NodeWrapperlooks like. That's the reason why theNodeWrapperonly contains a few methods (the ones I needed for the refactor).I all is fine, I propose that I can refactor other tests in other PRs 😊
Does this PR introduce a user-facing change?