StatefulSet: Don't try to update immutable hostname/subdomain fields. #51044
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
kow3ns
added
the
release-note-none
k8s-github-robot
added
approved
| func initIdentity(set *apps.StatefulSet, pod *v1.Pod) { | ||
| updateIdentity(set, pod) | ||
| // Set these immutable fields only on initial Pod creation, not updates. | ||
| // Set hostname/subdomain in preparation for removal of the annotations. |
There was a problem hiding this comment.
This line of the comment is not applicable on this branch, since the annotations were already removed.
|
@kow3ns Random crazy idea: What if we continue to check whether the hostname/subdomain (fields) match our expectation, and if not, we delete the Pod? Basically a StatefulSet Pod in this state is broken anyway, and the controller's job is to repair them when they're broken. |
|
@enisoc Possible to do, but probably unnecessary as the fields are immutable, and, if you came to 1.8.x by way of 1.7.x, you are already using the fields. |
…me or v1.PodSpec.Subdomain
kow3ns
force-pushed
the
sts-immutable-network-id
branch
from
6fb66ef to
a6473d8
Compare
|
Yes it's unnecessary on master. I was thinking about for the cherrypick into 1.7. The hostname/subdomain fields are part of the desired state that the controller wants to maintain. Should we give up on the Pod if those fields are immutable, or should we delete the Pod in order to make progress towards the desired state? |
|
It requires some thought, you upgrade form 1.6 to 1.7, and all of a sudden you get an automatic rolling update across all of your stateful applications (assuming we do the safe thing and orchestrate the deletion trying to preserve availability where possible). If we warn users that the update has to be manually, prior to the upgrade, they can update applications individually, and safely, prior to performing the 1.6.x to 1.7.x upgrade. |
|
Even if we do this, I would still recommend that users do it themselves before upgrading. The scenario I'm hoping to mitigate is if the user fails follow the recommendation. Their StatefulSet will get into a broken state that the controller can choose to either try to fix (by deleting Pods) or do nothing (because deleting Pods could make things worse). This may be moot though, because making a change that starts to delete Pods is probably too scary to do in a patch release. That's why I prefaced it by saying it's a crazy idea. :) |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enisoc, kow3ns Associated issue: 51043 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
Automatic merge from submit-queue (batch tested with PRs 51102, 50712, 51037, 51044, 51059) |
enisoc
changed the title
wojtek-t
added
cherry-pick-approved
Automatic merge from submit-queue Manual cherrypick of #51043 StatefulSet controller no longer attempts to mutate v1.PodSpec.Hostname or v1.PodSpec.Subdomain fixes: #51043 Partial fix for #48327 Manual cherrypick of #51044 ```release-note StatefulSet: Fix "forbidden pod updates" error on Pods created prior to upgrading to 1.7. (#48327) ```
|
Commit found in the "release-1.7" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked. |
Automatic merge from submit-queue. Allow Hostname and Subdomain to be set if empty **What this PR does / why we need it**: This PR allows the Hostname and Subdomain field of v1.PodSpec to be set when empty, and modifies the StatefulSet controller to set them when empty. For #48327: We have merged #50942 to ensure that the Hostname and Subdomain fields are set when a new Pod is created. Users should upgrade to 1.6.9 and perform a rolling restart of all Pods in their StatefulSets to ensure that these fields are set prior to an upgrade to 1.7.5. We have merged #51149 and #51044 to rollback the attempted mutation introduced in #44137. This PR allows the Hostname and Subdomain field to be set exactly once, so that when users fail to read the notes, and encounter this issue, their clusters should self heal (even though they will experience a temporary network disruption for Pods in their StatefulSets.) ```release-note StatefulSet will now fill the `hostname` and `subdomain` fields if they're empty on existing Pods it owns. This allows it to self-correct the issue where StatefulSet Pod DNS entries disappear after upgrading to v1.7.x (#48327). ```
What this PR does / why we need it: The StatefulSet controller no longer attempts to mutate "hostname" or "subdomain" fields of the "pod.spec" to enforce the network identity of Pods in a StatefeulSet. Since these fields are set upon creation and immutable thereafter setting the annotations is no longer necessary.
fixes: #51043