Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wait for first pod to termiante in PV test #73900

Merged
merged 1 commit into from
Feb 13, 2019

Conversation

cofyc
Copy link
Member

@cofyc cofyc commented Feb 11, 2019

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:

Local plugin skips setting fsGroup if volume is mounted by other pods. In "should be mountable" test, we must wait for first pod to terminate.

Example logs of a failing test:

# Starting MountVolume "local-r9bh9" for first pod (injector)
I0210 14:28:38.910272    1599 reconciler.go:237] Starting operationExecutor.MountVolume for volume "local-r9bh9" (UniqueName: "kubernetes.io/local-volume/local-r9bh9") pod "in-tree-injector-qqsh" (UID: "282ff226-2d40-11e9-abc3-42010a8a0002")
# Starting MountVolume "local-r9bh9" for second pod (in-tree-client)
I0210 14:28:43.403945    1599 reconciler.go:237] Starting operationExecutor.MountVolume for volume "local-r9bh9" (UniqueName: "kubernetes.io/local-volume/local-r9bh9") pod "in-tree-client" (UID: "2abd5da5-2d40-11e9-abc3-42010a8a0002")
# Because volume is used by injector, local plugin skips setting fsGroup 
I0210 14:28:43.408536    1599 server.go:463] Event(v1.ObjectReference{Kind:"Pod", Namespace:"volumes-2021", Name:"in-tree-client", UID:"2abd5da5-2d40-11e9-abc3-42010a8a0002", APIVersion:"v1", ResourceVersion:"34697", FieldPath:""}): type: 'Warning' reason: 'AlreadyMountedVolume' The requested fsGroup is 1234, but the volume local-r9bh9 has GID 0. The volume may not be shareable.
# Begin to umount volume "local-r9bh9" for first pod (injector)
I0210 14:28:43.808597    1599 reconciler.go:181] operationExecutor.UnmountVolume started for volume "in-tree-volume-trd4" (UniqueName: "kubernetes.io/local-volume/local-r9bh9") pod "282ff226-2d40-11e9-abc3-42010a8a0002" (UID: "282ff226-2d40-11e9-abc3-42010a8a0002")

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/flake Categorizes issue or PR as related to a flaky test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Feb 11, 2019
@cofyc
Copy link
Member Author

cofyc commented Feb 11, 2019

/sig storage
/priority important-soon
/assign @msau42

@k8s-ci-robot k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Feb 11, 2019
@cofyc cofyc changed the title Wait for first pod to termiante in local pv test Wait for first pod to termiante in PV test Feb 11, 2019
@cofyc cofyc force-pushed the fix-local-test branch 3 times, most recently from dad4514 to e392dfb Compare February 11, 2019 04:14
@cofyc
Copy link
Member Author

cofyc commented Feb 11, 2019

/retest

@oomichi
Copy link
Member

oomichi commented Feb 11, 2019

/cc @oomichi

@msau42
Copy link
Member

msau42 commented Feb 11, 2019

/approve

I contemplated if it would be better to just make the two pods use the same fsgroup, but I think it would be good to test that it can be changed.

local plugin will skip setting fsGroup if volume is mounted by other pods
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 12, 2019
@msau42
Copy link
Member

msau42 commented Feb 12, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 12, 2019
@cofyc
Copy link
Member Author

cofyc commented Feb 12, 2019

/test pull-kubernetes-verify

@oomichi
Copy link
Member

oomichi commented Feb 12, 2019

Thanks for updating,

/lgtm

@cofyc
Copy link
Member Author

cofyc commented Feb 13, 2019

@msau42 Could you approve again? prow didn't add approved label here.

@msau42
Copy link
Member

msau42 commented Feb 13, 2019

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cofyc, 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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 13, 2019
@k8s-ci-robot k8s-ci-robot merged commit 5442980 into kubernetes:master Feb 13, 2019
@cofyc cofyc deleted the fix-local-test branch May 4, 2019 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/flake Categorizes issue or PR as related to a flaky test. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants