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

Add --mounter-path flag to kubelet that will allow overriding the mount command used by kubelet #34994

Merged
merged 1 commit into from
Oct 21, 2016
Merged

Add --mounter-path flag to kubelet that will allow overriding the mount command used by kubelet #34994

merged 1 commit into from
Oct 21, 2016

Conversation

jingxu97
Copy link
Contributor

@jingxu97 jingxu97 commented Oct 17, 2016 •
edited by k8s-oncall
Loading

In order to be able to use new mounter library, this PR adds the
mounterPath flag to kubelet which passes the flag to the mount
interface. If flag is empty, mount uses default mount path.

This change is Reviewable

@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Oct 17, 2016
@jingxu97 jingxu97 force-pushed the Oct/mounterPathFlag-10-17 branch from 86534b5 to 28f6a2f Compare October 18, 2016 18:02
@jingxu97 jingxu97 assigned vishh and mtaufen and unassigned lavalamp Oct 18, 2016
@mtaufen
Copy link
Contributor

mtaufen commented Oct 19, 2016

Probably worth adding a release note to the main comment to note the new Kubelet flag.

@mtaufen
Copy link
Contributor

mtaufen commented Oct 19, 2016

I think I'm pretty close to getting my changes to the node test suite to work with this. Still having some small problems, but they are related to my changes and not this PR.

vishh
vishh approved these changes Oct 19, 2016
View reviewed changes
Copy link
Contributor

@vishh vishh left a comment

Choose a reason for hiding this comment

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

Just one nit. Otherwise LGTM

pkg/apis/componentconfig/v1alpha1/types.go
@@ -358,6 +358,9 @@ type KubeletConfiguration struct {
// rktPath is the path of rkt binary. Leave empty to use the first rkt in
// $PATH.
RktPath string `json:"rktPath"`
// mounterPath is the path of mounter binary. If not set, kubelet will user default mount,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: "path to mounter binary. kubelet will attempt to use mount binary that is available via $PATH.

@vishh vishh added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 19, 2016
@vishh vishh added this to the v1.4 milestone Oct 19, 2016
@vishh vishh added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Oct 19, 2016
@vishh vishh changed the title Add mounterPath to mounter interface Add --mounter-path flag to kubelet that will allow overriding the mount command used by kubelet Oct 19, 2016
@mtaufen
Copy link
Contributor

mtaufen commented Oct 19, 2016

You still need to add the mounter-path flag to hack/verify-flags/known-flags.txt and probably need to run hack/update-codecgen.sh to get the Jenkins verification to pass.

@vishh vishh added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Oct 19, 2016
@jingxu97 jingxu97 force-pushed the Oct/mounterPathFlag-10-17 branch from 28f6a2f to e44e71c Compare October 20, 2016 19:56
@k8s-github-robot k8s-github-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Oct 20, 2016
@k8s-ci-robot
Copy link
Contributor

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

@jingxu97
Add mounterPath to mounter interface
34ef93a 
In order to be able to use new mounter library, this PR adds the
mounterPath flag to kubelet which passes the flag to the mount
interface. If flag is empty, mount uses default mount path.
@jingxu97 jingxu97 force-pushed the Oct/mounterPathFlag-10-17 branch from e44e71c to 34ef93a Compare October 20, 2016 21:15
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2016
@saad-ali
Copy link
Member

LGTM

@saad-ali saad-ali added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 20, 2016
@k8s-github-robot
Copy link

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

@jessfraz
Copy link
Contributor

this is in the v1.4 milestone is this for 1.4

@k8s-ci-robot
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit 34ef93a. 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 34ef93a. 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.

@saad-ali
Copy link
Member

@jessfraz There will need to be a 1.4.x release that this PR + a few others will all go into--together they will enable GCI NFS and GlusterFS binaries. We'll sync up with you once all the changes are in HEAD and have a bit of time to soak before we begin cherry picking them and plan the point release (ETA about a week or so).

@saad-ali
Copy link
Member

@k8s-bot cvm gke e2e test this

@saad-ali
Copy link
Member

@k8s-bot gci gke e2e test this

@jessfraz
Copy link
Contributor

ok cool

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 230fe1a into kubernetes:master Oct 21, 2016
@saad-ali saad-ali mentioned this pull request Oct 21, 2016
Merged
@mtaufen
Copy link
Contributor

mtaufen commented Oct 21, 2016

Woohoo!

@jessfraz jessfraz added cherrypick-candidate cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. labels Oct 28, 2016
@jessfraz jessfraz mentioned this pull request Oct 28, 2016
Closed
@jessfraz jessfraz removed cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. cherrypick-candidate labels Oct 28, 2016
dims pushed a commit to dims/kubernetes that referenced this pull request Feb 8, 2018
Merge pull request kubernetes#34994 from jingxu97/Oct/mounterPathFlag…
9e14dce 
…-10-17

Automatic merge from submit-queue

Add `--mounter-path` flag to kubelet that will allow overriding the `mount` command used by kubelet

In order to be able to use new mounter library, this PR adds the
mounterPath flag to kubelet which passes the flag to the mount
interface. If flag is empty, mount uses default mount path.
@mengjiao-liu mengjiao-liu mentioned this pull request Jul 21, 2022
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vishh vishh vishh approved these changes

Assignees

@mtaufen mtaufen

@vishh vishh

Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. 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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.4
Development

Successfully merging this pull request may close these issues.
9 participants
@jingxu97 @mtaufen @k8s-ci-robot @saad-ali @k8s-github-robot @jessfraz @vishh @lavalamp @googlebot