Stop using node selector as ipfailover label#10388
Stop using node selector as ipfailover label#10388openshift-bot merged 1 commit intoopenshift:masterfrom marun:bz/1365176
Conversation
The ipfailover dc and its template were having the label(s) provided for the node selector set as their label(s). If, as in the documentation, the router service was selecting on that label, the service would end up targeting the keepalived pods. This change ensures the dc and template define a new label rather than the one provided as the node selector.
|
[test] |
|
re-[test] one-off yum mirror timeout on dind image build |
|
re-[test] #10393 |
|
Evaluated for origin test up to af00ff0 |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/7882/) |
|
LGTM, do we need to update the documentation so that we describe the label we are putting on the ipfailover containers? |
| } | ||
|
|
||
| labels := map[string]string{ | ||
| "ipfailover": name, |
There was a problem hiding this comment.
Can we just use a label name instead of ipfailover ala: "name": name - makes it a more generic selector?
Edited
There was a problem hiding this comment.
What would the value of being more generic? Name is already a metadata property, and I think it would make more sense to be more specific rather than less.
There was a problem hiding this comment.
fwiw I was following the convention of how the router dc/template is labeled:
https://github.com/openshift/origin/blob/master/pkg/cmd/admin/router/router.go#L499
I'm not picky about what convention we choose, but I'd like it to be reflected in the code.
There was a problem hiding this comment.
Yeah, guess ipfailover is better - ignore me comment!!
|
LGTM just a minor comment re: a more generic label selector |
|
@knobunc I don't think a doc change is required. I'm not sure it's a good idea to apply the same label on the router pods as the nodes targeted to host them, but it doesn't break anything. |
|
[merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/7882/) (Image: devenv-rhel7_4831) |
|
Evaluated for origin merge up to af00ff0 |
5 participants
The ipfailover dc and its template were having the label(s) provided
for the node selector set as their label(s). If, as in the
documentation, the router service was selecting on that label, the
service would end up targeting the keepalived pods. This change
ensures the dc and template define a new label rather than the one
provided as the node selector.
ref: https://bugzilla.redhat.com/show_bug.cgi?id=1365176
cc: @openshift/networking