Identify statefulsets by labels rather than error prone name parsing #309
Conversation
wallrj
changed the title
|
In the initial commit I used
|
| glog.Errorf("statefulset name %q did not contain cluster name %q", ssName, cluster.Name) | ||
| } | ||
| for _, ss := range sets { | ||
| npName := ss.Labels[util.NodePoolNameLabelKey] |
There was a problem hiding this comment.
What if the label isn't set?
Eventually I think this sort of thing should be moved into some kind of 'meta accessor' helper... Not sure how that's best to be structured. We should dig into the upstream codebase. Doesn't need to be for this PR though.
There was a problem hiding this comment.
Yeah, I don't know where to draw the line in handling cases where an outside program or person has meddled with the labels. If there isn't a label there'll be a CassandraCluster.Status.NodePools[""] NodePoolStatus entry which should give a clue to what's gone wrong.
And if we do some day change the statefulset name format we may and up with multiple sets with the same nodepool label.
I'll leave it like this for now.
|
/retest |
|
/lgtm cancel |
|
/lgtm |
|
@wallrj: you cannot LGTM your own PR. 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kragniz, munnerz, wallrj 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 |
|
/retest |
1 similar comment
|
/retest |
Fixes: #304
Release note: