Switching from comparison on Role to helper. - #18514
Conversation
Preperation for something like kubernetes#18495. Moving away from direct comparison (== or !=) on IG role. Using helper methods such as HasNode() or HasControlPlane(). Also added a hack test so we don't backtrack. Should help prepare for supporting more control plane roles.
|
/assign @justinsb |
kubernetes-prow
Bot
added
cncf-cla: yes
kubernetes-prow
Bot
added
area/nodeup
area/provider/aws
|
/assign @hakman |
|
/test pull-kops-e2e-azure-cni-cilium |
|
|
||
| errors=0 | ||
|
|
||
| # Find all .go files, excluding vendor, .build, and the file where roles are defined. |
There was a problem hiding this comment.
Oooh I like this tactic. We should think about how we do this in general e.g. do these get deleted after a while? Should they actually only be in the PR description (e.g. if we had a variable rename, then we could automatically verify that the commit just did a variable rename).
Anyway, I like this, it sets up some interesting questions, but we don't need to deal with them in this PR - let's explore the idea a bit more first before trying to systematize it!
kubernetes-prow
Bot
added
the
lgtm
|
Very nice - much cleaner, and the idea of including the "macros" is really interesting. I think the test failure is a flake: /test pull-kops-e2e-azure-cni-cilium /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb 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 |
kubernetes-prow
Bot
added
the
approved
|
/test all |
3 participants
Preperation for something like #18495.
Moving away from direct comparison (== or !=) on IG role. Using helper methods such as HasNode() or HasControlPlane(). Also added a hack test so we don't backtrack.
Should help prepare for supporting more control plane roles.