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

Enable containerized storage plugins mounter on GCI #35350

Merged
merged 2 commits into from
Oct 28, 2016
Merged

Enable containerized storage plugins mounter on GCI #35350

merged 2 commits into from
Oct 28, 2016

Conversation

vishh
Copy link
Contributor

@vishh vishh commented Oct 22, 2016 •
edited
Loading 

On GCI, kubelet uses an external mounter script (typically a special container running in a chroot) to perform mount operations

This change is Reviewable

@vishh vishh added priority/backlog Higher priority than priority/awaiting-more-evidence. release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Oct 22, 2016
@k8s-github-robot k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 22, 2016
@vishh vishh force-pushed the enable-gci-cmount branch from 9879884 to 672fb14 Compare October 22, 2016 00:57
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 25, 2016
@vishh vishh force-pushed the enable-gci-cmount branch from 672fb14 to f60d597 Compare October 27, 2016 05:43
@vishh vishh removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 27, 2016
@vishh
Copy link
Contributor Author

vishh commented Oct 27, 2016

@mtaufen @jingxu97 this PR can go in as-is. Kindly prioritize reviewing this.

@vishh vishh force-pushed the enable-gci-cmount branch from f60d597 to 7d36846 Compare October 27, 2016 05:57
@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 27, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit 7d36846. 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.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GCE e2e failed for commit 7d36846. 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.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE etcd3 e2e failed for commit 7d36846. 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 e2e failed for commit 7d36846. 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 Kubemark GCE e2e failed for commit 7d36846. 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 GKE smoke e2e failed for commit 7d36846. 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 7d36846. 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.

@mtaufen
Copy link
Contributor

mtaufen commented Oct 27, 2016

Verification failed with I1026 23:00:48.054] Boilerplate header is wrong for: /go/src/k8s.io/kubernetes/cluster/gce/gci/configure-helper.sh

mtaufen
mtaufen reviewed Oct 27, 2016
View reviewed changes
Copy link
Contributor

@mtaufen mtaufen left a comment

Choose a reason for hiding this comment

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

One quick fix and then LGTM

cluster/gce/gci/configure-helper.sh
@@ -1,3 +1,4 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this initial newline might be why the boilerplate header verification failed.
And the shebang ofc needs to be on the first line anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack. Fixing it.

vishh added 2 commits October 27, 2016 11:56
@vishh
run the gci containerized mounter to cache mounter container image
64c1fa3 
Signed-off-by: Vishnu kannan <vishnuk@google.com>
@vishh
Enable containerized storage mounter on GCI
aceddfa 
Signed-off-by: Vishnu kannan <vishnuk@google.com>
@vishh vishh force-pushed the enable-gci-cmount branch from 7d36846 to aceddfa Compare October 27, 2016 18:56
@vishh
Copy link
Contributor Author

vishh commented Oct 27, 2016

@mtaufen Fixed the PR and rebased it. Let's see if tests are happy now.

@mtaufen
Copy link
Contributor

mtaufen commented Oct 27, 2016

LGTM pending tests

@mtaufen mtaufen added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 27, 2016
@vishh vishh added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed priority/backlog Higher priority than priority/awaiting-more-evidence. labels Oct 27, 2016
@vishh
Copy link
Contributor Author

vishh commented Oct 27, 2016

Bumping the priority since we need to soak this PR before tomorrow.

@vishh vishh mentioned this pull request Oct 27, 2016
Merged
@vishh vishh added this to the v1.5 milestone Oct 27, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 52988de into kubernetes:master Oct 28, 2016
@gmarek
Copy link
Contributor

gmarek commented Oct 28, 2016

This PR broke kubemark - I'm going to revert this PR. It turned out that we weren't running GCI based kubemark presubmits. @wojtek-t is fixing that.

@gmarek gmarek mentioned this pull request Oct 28, 2016
Merged
@jessfraz jessfraz mentioned this pull request Nov 17, 2016
Closed
@chentao1596 chentao1596 mentioned this pull request Dec 5, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtaufen mtaufen mtaufen left review comments

Assignees

@mtaufen mtaufen

@jingxu97 jingxu97

Labels
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 Denotes a PR that will be considered when it comes time to generate release notes. 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
@vishh @k8s-ci-robot @mtaufen @k8s-github-robot @gmarek @googlebot @jingxu97