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

kubeadm: always mount a flex volume path for the controller-manager #84468

Merged
merged 1 commit into from
Oct 28, 2019

Conversation

neolit123
Copy link
Member

What this PR does / why we need it:

Checking if the path exists before creating the volume is
problematic because the path will be created regardless
after the initial call to "kubeadm init" and once the CM Pod
is running.

Then on subsequent calls to "kubeadm init" or the "control-plane"
phase the manifest for the CM will be different.

Always mount this path, but also consider the user provided
flag override from ClusterConfiguration.

Which issue(s) this PR fixes:

Fixes kubernetes/kubeadm#1858

Special notes for your reviewer:
NONE

Does this PR introduce a user-facing change?:

kubeadm: always mount the kube-controller-manager hostPath volume that is given by the --flex-volume-plugin-dir flag.

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


/assign @ereslibre @yastij
/kind bug
/priority backlog

Checking if the path exists before creating the volume is
problematic because the path will be created regardless
after the initial call to "kubeadm init" and once the CM Pod
is running.

Then on subsequent calls to "kubeadm init" or the "control-plane"
phase the manifest for the CM will be different.

Always mount this path, but also consider the user provided
flag override from ClusterConfiguration.
@k8s-ci-robot k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Oct 28, 2019
@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. priority/backlog Higher priority than priority/awaiting-more-evidence. 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. labels Oct 28, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: neolit123

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 Oct 28, 2019
@k8s-ci-robot k8s-ci-robot added area/kubeadm sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Oct 28, 2019
@neolit123 neolit123 changed the title kubeadm: always add a flex volume path for the controller-manager kubeadm: always mount a flex volume path for the controller-manager Oct 28, 2019
@ereslibre
Copy link
Contributor

Thank you @neolit123!

/lgtm
/hold

Other reviewers, feel free to unhold.

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Oct 28, 2019
@neolit123
Copy link
Member Author

/retest

Copy link
Member

@yastij yastij left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 28, 2019
@k8s-ci-robot k8s-ci-robot merged commit b6c8f49 into kubernetes:master Oct 28, 2019
@k8s-ci-robot k8s-ci-robot added this to the v1.17 milestone Oct 28, 2019
saschagrunert added a commit to saschagrunert/kubernetes that referenced this pull request Jan 13, 2020
In kubernetes#84468 we added a host path mount for a flex volume which is needed
by the controller manager. We should pre-check if the path is write-able
at all because the default path might be read-only. To do this, we now
do an `os.MkdirAll` and propagate the error up to to the caller.

Signed-off-by: Sascha Grunert <sgrunert@suse.com>
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. area/kubeadm cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. 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. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible flex volume implementation issue
4 participants