Fix DownwardAPI refresh race. #59873
Conversation
WaitForAttachAndMount should mark only pod in DesiredStateOfWorldPopulator (DSWP) and DSWP should mark the volume to be remounted only when the new pod has been processed. Otherwise DSWP and reconciler race who gets the new pod first. If it's reconciler, then DownwardAPI and Projected volumes of the pod are not refreshed with new content and they are updated after the next periodic sync (60-90 seconds).
k8s-ci-robot
added
release-note-none
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/retest |
|
/lgtm |
|
/assign @derekwaynecarr |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: derekwaynecarr, errordeveloper, jingxu97, jsafrane 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 |
k8s-ci-robot
added
the
approved
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue (batch tested with PRs 59873, 59933, 59923, 59944, 59953). If you want to cherry-pick this change to another branch, please follow the instructions here. |
Automatic merge from submit-queue. Picks for volume manager Thanks to @jsafrane for these fixes kubernetes/kubernetes#59873 kubernetes/kubernetes#59923 59923 modified from upstream because some logging levels where already higher in 1.9 xref https://bugzilla.redhat.com/show_bug.cgi?id=1538216 Fixes #17605 Fixes #17556 @derekwaynecarr
Automatic merge from submit-queue. Picks for volume manager Thanks to @jsafrane for these fixes kubernetes#59873 kubernetes#59923 59923 modified from upstream because some logging levels where already higher in 1.9 xref https://bugzilla.redhat.com/show_bug.cgi?id=1538216 Fixes openshift/origin#17605 Fixes openshift/origin#17556 @derekwaynecarr Origin-commit: e4f2115102c01124cc7f168b7f1ae4c65f190875
| @@ -302,6 +305,9 @@ func (dswp *desiredStateOfWorldPopulator) processPodVolumes(pod *v1.Pod) { | |||
| // some of the volume additions may have failed, should not mark this pod as fully processed | |||
| if allVolumesAdded { | |||
| dswp.markPodProcessed(uniquePodName) | |||
| // New pod has been synced. Re-mount all volumes that need it | |||
| // (e.g. DownwardAPI) | |||
| dswp.actualStateOfWorld.MarkRemountRequired(uniquePodName) | |||
There was a problem hiding this comment.
this commit showed up in a bisect of scalability test regression of pod startup time (#60589 (comment)), and this change looks odd... does this force double mount setup for all pods?
There was a problem hiding this comment.
The flow that leads to MarkRemountRequired call is:
-
kubelet.syncPodis called (on pod update or every 90s)volumeManager.WaitForAttachAndMountis called and marks the pod for reprocessing
-
desiredStateOfWorldPopulator.findAndAddNewPodsis called periodicallydesiredStateOfWorldPopulator.processPodVolumesis called (only when the pod was marked for reprocessing insyncPod)actualStateOfWorld.MarkRemountRequiredis called- reconciler re-mounts the volumes (updates secrets, downward API, ...). Update of DownwardAPI is the reason why
syncPodis involved.
- reconciler re-mounts the volumes (updates secrets, downward API, ...). Update of DownwardAPI is the reason why
Notice that before this PR, MarkRemountRequired was called directly by kubelet.syncPod, so the frequency of the calls did not change. With this PR, they are called only in the right order. I don't think it could affect pod startup time.
On the other hand, syncPod is called with every pod update and pod changes quite often during startup. It is possible that I missed something there.
WaitForAttachAndMount should mark only pod in DesiredStateOfWorldPopulator (DSWP) and DSWP should mark the volume to be remounted only when the new pod has been processed. Otherwise DSWP and reconciler race who gets the new pod first. If it's reconciler, then DownwardAPI and Projected volumes of the pod are not refreshed with new content and they are updated after the next periodic sync (60-90 seconds). RelatedTo: kubernetes#59873
WaitForAttachAndMount should mark only pod in DesiredStateOfWorldPopulator (DSWP) and DSWP should mark the volume to be remounted only when the new pod has been processed.
Otherwise DSWP and reconciler race who gets the new pod first. If it's reconciler, then DownwardAPI and Projected volumes of the pod are not refreshed with new content and they are updated after the next periodic sync (60-90 seconds).
Fixes #59813
/assign @jingxu97 @saad-ali
/sig storage
/sig node