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

stop removing cni directories as they aren't installed by kubeadm #83950

Merged
merged 1 commit into from
Oct 17, 2019

Conversation

yastij
Copy link
Member

@yastij yastij commented Oct 15, 2019

Signed-off-by: Yassine TIJANI [email protected]

What type of PR is this?

/kind bug
/priority backlog

What this PR does / why we need it: This PR stops removing /etc/cni/net.d as its removal might break tools that install configuration under this directory. Also, this isn't created by kubeadm. Added a message when using reset like we do for iptables/ipvs

Which issue(s) this PR fixes: Fixes kubernetes/kubeadm#1822

Special notes for your reviewer:

/assign @neolit123

Does this PR introduce a user-facing change?:

kubeadm no longer removes /etc/cni/net.d as it does not install it. Users should remove files from it manually or rely on the component that created them

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


@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. size/XS Denotes a PR that changes 0-9 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 15, 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 15, 2019
@ereslibre
Copy link
Contributor

Thank you @yastij!

/approve
/lgtm

@neolit123
Copy link
Member

thanks @yastij

kubeadm no longer removes /etc/cni/net.d as it does not install it. Users should remove it manually or rely on the component that created it

i would re-word to:

Users should remove files from it manually or rely on the component that created them

i'm 50/50 whether this needs action required:, not that it's a breaking change but for better visiblity.
it will definitely cause issues for users that are just trying different CNIs, if multiple networks are in there from my tests CoreDNS just fails.

would you mind sending a PR for this page as well:
https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/create-cluster-kubeadm/#pod-network

we need to have a Caution note that users should clean the files of a certain CNI plugin manually before installing a different CNI.

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

/cc @ereslibre

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ereslibre, yastij

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 15, 2019
@ereslibre
Copy link
Contributor

/hold

Missed @neolit123 review, feel free to unhold when feedback is addressed.

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

yastij commented Oct 16, 2019

@neolit123 @ereslibre - updated
/hold cancel

@yastij
Copy link
Member Author

yastij commented Oct 16, 2019

/retest

@neolit123
Copy link
Member

@yastij
as suggested here we probably should apply this minor clarification to the release note:
#83950 (comment)

@neolit123
Copy link
Member

/retest

@neolit123
Copy link
Member

/lgtm

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

yastij commented Oct 16, 2019

/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 16, 2019
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

1 similar comment
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@yastij
Copy link
Member Author

yastij commented Oct 17, 2019

/retest

@k8s-ci-robot k8s-ci-robot merged commit 4b58ef0 into kubernetes:master Oct 17, 2019
@k8s-ci-robot k8s-ci-robot added this to the v1.17 milestone Oct 17, 2019
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/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.

kubeadm reset breaks CNI for others
5 participants