Modified subpath configmap mount fails when container restarts #89629

fatedier · 2020-03-29T10:57:56Z

What type of PR is this?
/kind bug

What this PR does / why we need it:
When a container uses a configmap which is mounted with the subPath option, the configmap is changed and then the container (but not the pod) restarts the mounting of the configmap fails.

Since #82784 is not active and not rebased, i submit this PR to continue this work or someone can provide another solution?

Which issue(s) this PR fixes:
Fixes #68211

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Fix an issue with container restarts using a modified configmap or secret subpath volume mount.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2020-03-29T10:58:05Z

Hi @fatedier. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

fatedier · 2020-03-30T06:16:40Z

/assign @saad-ali

msau42 · 2020-06-16T00:22:19Z

/ok-to-test
/assign @msau42 @jsafrane

fatedier · 2020-06-16T11:49:41Z

/retest

fatedier · 2020-06-16T14:31:37Z

The test case Variable Expansion should not change the subpath mount on a container restart if the environment variable changes failed with this change.

It seems that SubPathExpr rely on this to ensure subpath mount not changed on a container restart. I'm not sure if it's appropriate.

msau42 · 2020-06-16T16:52:51Z

It seems that SubPathExpr rely on this to ensure subpath mount not changed on a container restart. I'm not sure if it's appropriate.

I'm open to changing that behavior to be consistent with configmap/secret update behavior. cc @jsafrane @kevtaylor for thoughts. I think the only consideration is that it's a conformance test so we would have to intentionally break it. There's probably a process for it.

kevtaylor · 2020-06-16T17:14:47Z

So the idea was always to preserve the original file mount when the container might fail and not restart on a new host location. If someone did alter the environment value in between you could consider this is quite an edge case and potentially intentional change, so relaxing it doesn't seem drastic.

msau42 · 2020-06-16T18:29:44Z

/assign @johnbelamaric
for conformance test question. Basically, this test case was enforcing a bit of a corner case to begin with. I can't think of a strong reason why we would want this current behavior, especially as we see this behavior as problematic for other volume types (not part of conformance atm)

msau42 · 2020-07-07T00:52:12Z

/lgtm
/approve

Thank you! Can you also add a release note, that this fixes an issue with container restarts using a modified configmap or secret subpath volume mount?

msau42 · 2020-07-07T00:52:37Z

/assign @johnbelamaric
for removal of conformance test

johnbelamaric · 2020-07-08T16:01:08Z

/approve

k8s-ci-robot · 2020-07-08T16:01:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fatedier, johnbelamaric, msau42

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:

~~pkg/volume/util/OWNERS~~ [msau42]
~~test/conformance/testdata/OWNERS~~ [johnbelamaric]
~~test/e2e/common/OWNERS~~ [msau42]
~~test/e2e/storage/OWNERS~~ [msau42]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

msau42 · 2020-07-08T16:16:05Z

/priority important-soon

msau42 · 2020-07-08T19:17:26Z

/retest

msau42 · 2020-07-09T01:35:34Z

/retest

fatedier · 2020-07-09T09:40:45Z

/retest

fatedier · 2020-07-09T13:04:57Z

/test pull-kubernetes-e2e-kind

…-upstream-release-1.18 Automated cherry pick of #89629: fix 68211: modified subpath configmap mount fails when

…d subpath configmap mount fails when"

…ick-of-#89629-upstream-release-1.18 Revert "Automated cherry pick of #89629: fix 68211: modified subpath configmap mount fails when"

verult · 2021-04-03T00:52:03Z

The e2e test started to flake: #100194

silenceper · 2021-07-21T11:29:46Z

pkg/volume/util/subpath/subpath_linux.go

+		if err != nil {
+			return false, "", fmt.Errorf("error calling findMountInfo for %s: %s", bindPathTarget, err)
+		}
+		if mntInfo.Root != subpath.Path {


in my env, /proc/self/mountinfo like this ,

523 47 253:16 /kubelet/pods/d166411c-ea15-11eb-abc8-525400f80eae/volumes/kubernetes.io~configmap/extra-cfg/..2021_07_21_11_21_39.969349293/extra.ini /data/kubelet/pods/d166411c-ea15-11eb-abc8-525400f80eae/volume-subpaths/extra-cfg/test/0 rw,noatime shared:29 - xfs /dev/vdb rw,attr2,inode64,prjquota

mntInfo.Root is /kubelet/pods/d166411c-ea15-11eb-abc8-525400f80eae/volumes/kubernetes.io~configmap/extra-cfg/..2021_07_21_11_21_39.969349293/extra.ini

is always not equal subpath.Path because miss /data prefix.
kubelet root path in /data

/dev/vdb 1000G 11G 989G 2% /data

@fatedier
I tested it . If kubelet mounts an extra data disk, it will cause the missing prefix of mntInfo.Root

k8s-ci-robot requested review from jingxu97 and msau42 March 29, 2020 10:58

k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 29, 2020

k8s-ci-robot assigned saad-ali Mar 30, 2020

k8s-ci-robot assigned jsafrane Jun 16, 2020

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jun 16, 2020

k8s-ci-robot assigned msau42 Jun 16, 2020

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 16, 2020

fatedier added 2 commits June 16, 2020 11:37

fix 68211: modified subpath configmap mount fails when container restart

6c2562a

autogen files update

3089411

fatedier force-pushed the subpath branch from 323d4ca to 3089411 Compare June 16, 2020 04:03

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 16, 2020

k8s-ci-robot assigned johnbelamaric Jun 16, 2020

fatedier requested a review from msau42 July 6, 2020 11:48

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 7, 2020

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jul 7, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 8, 2020

k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jul 8, 2020

k8s-ci-robot merged commit d9b084a into kubernetes:master Jul 9, 2020

k8s-ci-robot added this to the v1.19 milestone Jul 9, 2020

github-actions bot mentioned this pull request Jul 14, 2020

Week Ending July 12, 2020 dev-obs/actus#193

Open

jsturtevant mentioned this pull request Jul 15, 2020

Windows: Fixes subpath volume persistance on container restart #92869

Closed

craiglpeters mentioned this pull request Jul 16, 2020

Windows: subpath mount changes on a container restart if the environment variable changes #90336

Closed

msau42 mentioned this pull request Jul 21, 2020

Automated cherry pick of #89629: fix 68211: modified subpath configmap mount fails when #93272

Merged

k3a mentioned this pull request Jul 31, 2020

modified subpath configmap mount fails when container restarts #68211

Closed

k8s-ci-robot added a commit that referenced this pull request Aug 2, 2020

Merge pull request #93272 from msau42/automated-cherry-pick-of-#89629…

0f13560

…-upstream-release-1.18 Automated cherry pick of #89629: fix 68211: modified subpath configmap mount fails when

msau42 added a commit to msau42/kubernetes that referenced this pull request Aug 4, 2020

Revert "Automated cherry pick of kubernetes#89629: fix 68211: modifie…

039e6a0

…d subpath configmap mount fails when"

k8s-ci-robot added a commit that referenced this pull request Aug 12, 2020

Merge pull request #93695 from msau42/revert-93272-automated-cherry-p…

0f413e5

…ick-of-#89629-upstream-release-1.18 Revert "Automated cherry pick of #89629: fix 68211: modified subpath configmap mount fails when"

fzu-huang mentioned this pull request May 31, 2021

Support diagnoses and recover to BUG[68211] kubediag/kubediag#59

Closed

silenceper reviewed Jul 21, 2021

View reviewed changes

silenceper mentioned this pull request Jul 22, 2021

subpath configmap always remount, when container restarts #103836

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modified subpath configmap mount fails when container restarts #89629

Modified subpath configmap mount fails when container restarts #89629

fatedier commented Mar 29, 2020 •

edited

k8s-ci-robot commented Mar 29, 2020

fatedier commented Mar 30, 2020

msau42 commented Jun 16, 2020

fatedier commented Jun 16, 2020

fatedier commented Jun 16, 2020

msau42 commented Jun 16, 2020

kevtaylor commented Jun 16, 2020

msau42 commented Jun 16, 2020

msau42 commented Jul 7, 2020

msau42 commented Jul 7, 2020

johnbelamaric commented Jul 8, 2020

k8s-ci-robot commented Jul 8, 2020

msau42 commented Jul 8, 2020

msau42 commented Jul 8, 2020

msau42 commented Jul 9, 2020

fatedier commented Jul 9, 2020

fatedier commented Jul 9, 2020

verult commented Apr 3, 2021

silenceper Jul 21, 2021 •

edited

silenceper Jul 21, 2021 •

edited

Modified subpath configmap mount fails when container restarts #89629

Modified subpath configmap mount fails when container restarts #89629

Conversation

fatedier commented Mar 29, 2020 • edited

k8s-ci-robot commented Mar 29, 2020

fatedier commented Mar 30, 2020

msau42 commented Jun 16, 2020

fatedier commented Jun 16, 2020

fatedier commented Jun 16, 2020

msau42 commented Jun 16, 2020

kevtaylor commented Jun 16, 2020

msau42 commented Jun 16, 2020

msau42 commented Jul 7, 2020

msau42 commented Jul 7, 2020

johnbelamaric commented Jul 8, 2020

k8s-ci-robot commented Jul 8, 2020

msau42 commented Jul 8, 2020

msau42 commented Jul 8, 2020

msau42 commented Jul 9, 2020

fatedier commented Jul 9, 2020

fatedier commented Jul 9, 2020

verult commented Apr 3, 2021

silenceper Jul 21, 2021 • edited

Choose a reason for hiding this comment

silenceper Jul 21, 2021 • edited

Choose a reason for hiding this comment

fatedier commented Mar 29, 2020 •

edited

silenceper Jul 21, 2021 •

edited

silenceper Jul 21, 2021 •

edited