UPSTREAM: 49285: do not mutate statefulset on update #15328
Conversation
|
[test] |
| @@ -450,7 +450,11 @@ func (ssc *StatefulSetController) sync(key string) error { | |||
| // syncStatefulSet syncs a tuple of (statefulset, []*v1.Pod). | |||
| func (ssc *StatefulSetController) syncStatefulSet(set *apps.StatefulSet, pods []*v1.Pod) error { | |||
| glog.V(4).Infof("Syncing StatefulSet %v/%v with %d pods", set.Namespace, set.Name, len(pods)) | |||
| if err := ssc.control.UpdateStatefulSet(set, pods); err != nil { | |||
| setCopy, err := api.Scheme.DeepCopy(set) | |||
There was a problem hiding this comment.
this should probably go deeper into UpdateStatefulSet when we know the mutation will happen, will tweak this if this confirms the tests pass.
There was a problem hiding this comment.
this should probably go deeper into UpdateStatefulSet when we know the mutation will happen, will tweak this if this confirms the tests pass.
It should go deeper, but if this works it's ok to bring out the big hammer to make progress as long as you still chase it down.
There was a problem hiding this comment.
I'm really surprised the mutation detector didn't catch this upstream...
There was a problem hiding this comment.
I also didn't see anything in the call tree below this that mutated without copying first... I might be missing something, but the fact that the diff was only on the kind and apiVersion fields is odd to me
There was a problem hiding this comment.
I also didn't see anything in the call tree below this that mutated without copying first... I might be missing something, but the fact that the diff was only on the kind and apiVersion fields is odd to me
Suggests that it's getting converted, encoded, or something. It wasn't obvious to me either.
There was a problem hiding this comment.
yeah seems like decoded to internal version, I will open upstream pull and we can take it from there
There was a problem hiding this comment.
|
flake: #15329 [test] again |
|
@mfojtik open upstream to match please. |
|
@deads2k will give it one more [test] to be sure... do we have time to investigate what actually mutate or we are fine with this and follow up issue? |
I'd like to see an upstream PR opened, so that this title can be updated,so the future rebaser can figure out how to handle this. Otherwise, no objection. |
|
flake: #15359 [test] |
|
@deads2k i ran test on this 4 times and haven't seen the mutation shutdown |
mfojtik
changed the title
|
Evaluated for origin test up to df1437e |
|
[merge] |
|
continuous-integration/openshift-jenkins/merge Waiting: You are in the build queue at position: 5 |
|
Evaluated for origin merge up to df1437e |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/3320/) (Base Commit: b4eb3b9) (PR Branch Commit: df1437e) |
|
Isolated change to fix a flake. Merging on green test. |
No description provided.