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.

Sign up for GitHub

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

Jump to bottom

Temporarily disable GCI mounter in e2e node tests #35724

Merged
merged 1 commit into from
Nov 4, 2016
Merged

Temporarily disable GCI mounter in e2e node tests #35724

merged 1 commit into from
Nov 4, 2016
+7 −3

Conversation

mtaufen
Copy link
Contributor

@mtaufen mtaufen commented Oct 27, 2016 •
edited by k8s-oncall
Loading

This is just so we have an off-switch ready to go if we need it. Don't merge unless we need to disable this functionality in the e2e node tests.

This change is Reviewable

@mtaufen mtaufen added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. release-note-none Denotes a PR that doesn't merit a release note. area/node-e2e labels Oct 27, 2016
@vishh
Copy link
Contributor

vishh commented Oct 27, 2016

You might want to include a revert of #35350 as well in this PR!

@k8s-ci-robot
Copy link
Contributor

Jenkins unit/integration failed for commit 49cc2019086cd6bac7c3d75929c8ba6169b7a790. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-github-robot k8s-github-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 27, 2016
@vishh vishh mentioned this pull request Nov 2, 2016
Closed
@dchen1107
Copy link
Member

@mtaufen Can you rebase the pr so that we can get this one merged soon based on the discussion at #35935 (comment)

@dchen1107 dchen1107 added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Nov 2, 2016
@mtaufen mtaufen force-pushed the disable-cmount-for-e2e-node branch from 49cc201 to 5a61a3b Compare November 2, 2016 20:42
@mtaufen mtaufen removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 2, 2016
@mtaufen
Copy link
Contributor Author

mtaufen commented Nov 2, 2016

Rebased

@mtaufen mtaufen force-pushed the disable-cmount-for-e2e-node branch 2 times, most recently from 0eb0891 to f5ee924 Compare November 2, 2016 20:46
vishh
vishh reviewed Nov 2, 2016
View reviewed changes
test/e2e_node/remote/remote.go
@@ -276,7 +276,12 @@ func RunRemote(archive string, host string, cleanup bool, junitFilePrefix string
// Insert args at beginning of testArgs, so any values from command line take precedence
testArgs = fmt.Sprintf("--experimental-mounter-rootfs-path=%s ", mounterRootfsPath) + testArgs
testArgs = fmt.Sprintf("--experimental-mounter-path=%s ", mounterPath) + testArgs
glog.Infof("GCI node and GCI mounter both detected, setting --experimental-mounter-path=%q and --experimental-mounter-rootfs-path=%q accordingly", mounterPath, mounterRootfsPath)

glog.Warningf("Flag forwarding to Kubelet for --experimental-mounter-rootfs-path and --experimental-mounter-path has been disabled." +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This warning is only needed if node e2e diverges from regular behavior. That's not the case now. So let's remove this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

@mtaufen
Copy link
Contributor Author

mtaufen commented Nov 2, 2016

@k8s-bot gci gke e2e test this

@mtaufen mtaufen force-pushed the disable-cmount-for-e2e-node branch from f5ee924 to df47194 Compare November 2, 2016 22:03
@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 2, 2016
@mtaufen mtaufen force-pushed the disable-cmount-for-e2e-node branch from df47194 to f692e50 Compare November 3, 2016 15:34
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 3, 2016
@vishh
Copy link
Contributor

vishh commented Nov 3, 2016

Let's not merge this for now given that the scope for the custom mounter has been reduced to non tmpfs, ext* and bind mounts.

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 4, 2016
@mtaufen mtaufen assigned dchen1107 and unassigned vishh Nov 4, 2016
@dchen1107
Copy link
Member

Discussed with @vishh and @mtaufen: re: #35935

We should merge this as soon as possible since the pr () mentioned by @vishh at #35724 (comment) was reverted, and issue #35935 caused by gci-mounter failures cause the block of submit-queue. We should disable the usage of gci-mounter for now while @vishh and @jingxu97 are working on the fix for gci-mounter.

@mtaufen please rebase your pr and get this one in. Thanks!

@dchen1107 dchen1107 added kind/flake Categorizes issue or PR as related to a flaky test. and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Nov 4, 2016
@mtaufen mtaufen force-pushed the disable-cmount-for-e2e-node branch from f692e50 to 5330d55 Compare November 4, 2016 19:03
@mtaufen
Copy link
Contributor Author

mtaufen commented Nov 4, 2016

Re-rebased

@mtaufen mtaufen removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 4, 2016
@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 4, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit 5330d55c9f740be6c06e549808cc3c4ffd81869b. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GKE smoke e2e failed for commit 5330d55c9f740be6c06e549808cc3c4ffd81869b. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE etcd3 e2e failed for commit 5330d55c9f740be6c06e549808cc3c4ffd81869b. Full PR test history.

The magic incantation to run this job again is @k8s-bot gce etcd3 e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE Node e2e failed for commit 5330d55c9f740be6c06e549808cc3c4ffd81869b. Full PR test history.

The magic incantation to run this job again is @k8s-bot node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins Kubemark GCE e2e failed for commit 5330d55c9f740be6c06e549808cc3c4ffd81869b. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE e2e failed for commit 5330d55c9f740be6c06e549808cc3c4ffd81869b. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit 5330d55c9f740be6c06e549808cc3c4ffd81869b. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@mtaufen mtaufen force-pushed the disable-cmount-for-e2e-node branch from 5330d55 to c76c9c5 Compare November 4, 2016 19:43
@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GCE e2e failed for commit c76c9c5. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@mtaufen
Copy link
Contributor Author

mtaufen commented Nov 4, 2016

@k8s-bot gci gce e2e test this

@dchen1107
Copy link
Member

LGTM

@dchen1107 dchen1107 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 4, 2016
@dchen1107 dchen1107 added this to the v1.5 milestone Nov 4, 2016
yujuhong
yujuhong reviewed Nov 4, 2016
View reviewed changes
test/e2e_node/services/services.go
"--experimental-mounter-path", framework.TestContext.MounterPath,
"--experimental-mounter-rootfs-path", framework.TestContext.MounterRootfsPath,

// Temporarily disabled:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not blocking this PR, just a nit: it'd be nice to add a link to a github issue to explain the reasons and track when we can re-enable it.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit fbe29f4 into kubernetes:master Nov 4, 2016
@jessfraz jessfraz mentioned this pull request Nov 17, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yujuhong yujuhong yujuhong left review comments

@vishh vishh vishh left review comments

Assignees

@dchen1107 dchen1107

Labels
area/node-e2e 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/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.
7 participants
@mtaufen @vishh @k8s-ci-robot @dchen1107 @k8s-github-robot @yujuhong @googlebot