kubelet: make --cni-bin-dir accept a comma-separated list of CNI plugin directories #58714

dcbw · 2018-01-23T20:30:55Z

Allow CNI-related network plugin drivers (kubenet, cni) to search a list of
directories for plugin binaries instead of just one. This allows using an
administrator-provided path and fallbacks to others (like the previous default
of /opt/cni/bin) for backwards compatibility.

kubelet's --cni-bin-dir option now accepts multiple comma-separated CNI binary directory paths, which are search for CNI plugins in the given order.

@kubernetes/rh-networking @kubernetes/sig-network-misc @freehan @pecameron @rajatchopra

dcbw · 2018-01-24T02:17:55Z

/test pull-kubernetes-bazel-test

dcbw · 2018-01-24T03:45:01Z

@yujuhong @vishh looking for an /approve for cmd/kubelet and pkg/kubelet changes, thanks!

feiskyer · 2018-01-24T04:35:59Z

cmd/kubelet/app/server.go

@@ -980,7 +981,7 @@ func RunDockershim(f *options.KubeletFlags, c *kubeletconfiginternal.KubeletConf
 		NonMasqueradeCIDR: f.NonMasqueradeCIDR,
 		PluginName:        r.NetworkPluginName,
 		PluginConfDir:     r.CNIConfDir,
-		PluginBinDir:      r.CNIBinDir,
+		PluginBinDirs:     strings.Split(r.CNIBinDir, ":"),


Use comma instead of colon? ":" is part of dir name on Windows

dcbw · 2018-02-05T17:53:13Z

@feiskyer how about now? Changed to comma-separated.

feiskyer

LGTM

dcbw · 2018-02-06T16:07:39Z

@derekwaynecarr or maybe a quick look from you too.. Thanks!

dcbw · 2018-02-21T16:03:57Z

@vishh or maybe you? Thanks!

dcbw · 2018-02-21T16:04:39Z

@squeed @bboreham review and an /lgtm from you might be nice too because CNI.

bboreham · 2018-02-21T16:11:47Z

LGTM

feiskyer

/lgtm

feiskyer · 2018-02-24T02:01:42Z

cc @derekwaynecarr for approval

yujuhong · 2018-02-28T21:26:01Z

pkg/kubelet/network/kubenet/kubenet_linux.go

-		return true
+	found := false
+	for _, dir := range plugin.binDirs {
+		found = found || plugin.checkCNIPluginInOneDir(dir)


Is there any reason not to break/return once the plugin is found?

yujuhong · 2018-02-28T21:29:50Z

pkg/kubelet/network/kubenet/kubenet_linux.go

-	// kubenet will search for cni binaries in DefaultCNIDir first, then continue to vendorDir.
-	vendorDir         string
+	// binDirs is passed by kubelet cni-bin-dir parameter.
+	// kubenet will search for CNI binaries in DefaultCNIDir first, then continue to binDirs.


Not related to the changes in this PR specifically, but why does kubenet always searches the default directory? The description of the flag --cni-bin-dir does not indicate that the list of directories are in addition to the default...

@yujuhong good question, filed #60656 to track it in any case.

pluginDir -> confDir DefaultNetDir -> DefaultConfDir DefaultCNIDir -> DefaultBinDir

It's only used for the test code and after talking with Rajat, the vendor stuff was never really used anyway. So convert the vendor code into a plain array of plugin binary search paths, which is all the vendor code was doing anyway.

…in directories Allow CNI-related network plugin drivers (kubenet, cni) to search a list of directories for plugin binaries instead of just one. This allows using an administrator-provided path and fallbacks to others (like the previous default of /opt/cni/bin) for backwards compatibility.

dcbw · 2018-03-01T17:01:03Z

@yujuhong PTAL thanks!

yujuhong · 2018-03-01T18:02:21Z

/lgtm

k8s-ci-robot · 2018-03-01T18:03:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dcbw, feiskyer, yujuhong

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:

~~cmd/kubelet/OWNERS~~ [yujuhong]
~~pkg/kubelet/OWNERS~~ [yujuhong]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

fejta-bot · 2018-03-10T11:39:33Z

/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 comment for consistent failures.

fejta-bot · 2018-03-10T16:12:32Z

/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 comment for consistent failures.

fejta-bot · 2018-03-14T23:26:56Z

/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 comment for consistent failures.

fejta-bot · 2018-03-15T02:35:44Z

/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 comment for consistent failures.

fejta-bot · 2018-03-15T05:23:45Z

/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 comment for consistent failures.

k8s-ci-robot · 2018-03-19T09:32:20Z

@dcbw: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-unit	aad9a59cf70d93c4f1f557d37dbf75ed5e9f4c30	link	`/test pull-kubernetes-unit`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

fejta-bot · 2018-03-19T13:41:52Z

/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 comment for consistent failures.

k8s-github-robot · 2018-03-20T01:43:48Z

[MILESTONENOTIFIER] Milestone Pull Request Labels Incomplete

@dcbw @derekwaynecarr @feiskyer @yujuhong

Action required: This pull request requires label changes. If the required changes are not made within 3 days, the pull request will be moved out of the v1.11 milestone.

kind: Must specify exactly one of kind/bug, kind/cleanup or kind/feature.
priority: Must specify exactly one of priority/critical-urgent, priority/important-longterm or priority/important-soon.

Help

k8s-github-robot · 2018-03-20T04:34:37Z

Automatic merge from submit-queue (batch tested with PRs 59740, 59728, 60080, 60086, 58714). If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-ci-robot requested review from dnardo, sjpotter and yifan-gu January 23, 2018 20:31

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jan 23, 2018

dcbw force-pushed the cni-plugin-dirs branch 4 times, most recently from b4c04ed to a731acb Compare January 23, 2018 23:48

dcbw force-pushed the cni-plugin-dirs branch from a731acb to d305d75 Compare January 24, 2018 02:20

feiskyer reviewed Jan 24, 2018

View reviewed changes

dcbw force-pushed the cni-plugin-dirs branch 3 times, most recently from 70a1f57 to df90d8b Compare February 5, 2018 17:52

dcbw force-pushed the cni-plugin-dirs branch from df90d8b to aad9a59 Compare February 5, 2018 23:29

dcbw changed the title ~~kubelet: make --cni-bin-dir accept a colon-separate list of CNI plugin directories~~ kubelet: make --cni-bin-dir accept a comma-separated list of CNI plugin directories Feb 5, 2018

feiskyer reviewed Feb 6, 2018

View reviewed changes

dcbw added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Feb 6, 2018

dcbw assigned derekwaynecarr Feb 14, 2018

feiskyer approved these changes Feb 24, 2018

View reviewed changes

k8s-ci-robot assigned feiskyer Feb 24, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 24, 2018

yujuhong reviewed Feb 28, 2018

View reviewed changes

dcbw added 4 commits March 1, 2018 10:43

cni: clarify bin/conf directory variable names

16eaaae

pluginDir -> confDir DefaultNetDir -> DefaultConfDir DefaultCNIDir -> DefaultBinDir

kubenet: accept a list of CNI binary plugin paths

ee2ea22

dcbw force-pushed the cni-plugin-dirs branch from aad9a59 to 8778e50 Compare March 1, 2018 17:00

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 1, 2018

k8s-ci-robot assigned yujuhong Mar 1, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 1, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 1, 2018

dcbw added this to the v1.11 milestone Mar 19, 2018

k8s-github-robot added the milestone/incomplete-labels label Mar 20, 2018

k8s-github-robot merged commit 7bd2263 into kubernetes:master Mar 20, 2018

feiskyer mentioned this pull request Apr 3, 2018

failed to find plugin "loopback" in path [] #62054

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kubelet: make --cni-bin-dir accept a comma-separated list of CNI plugin directories #58714

kubelet: make --cni-bin-dir accept a comma-separated list of CNI plugin directories #58714

dcbw commented Jan 23, 2018 •

edited

Loading

dcbw commented Jan 24, 2018

dcbw commented Jan 24, 2018

feiskyer Jan 24, 2018

dcbw commented Feb 5, 2018

feiskyer left a comment

dcbw commented Feb 6, 2018

dcbw commented Feb 21, 2018

dcbw commented Feb 21, 2018 •

edited

Loading

bboreham commented Feb 21, 2018

feiskyer left a comment

feiskyer commented Feb 24, 2018

yujuhong Feb 28, 2018

dcbw Mar 1, 2018

yujuhong Feb 28, 2018

dcbw Mar 1, 2018

dcbw commented Mar 1, 2018

yujuhong commented Mar 1, 2018

k8s-ci-robot commented Mar 1, 2018

fejta-bot commented Mar 10, 2018

fejta-bot commented Mar 10, 2018

fejta-bot commented Mar 14, 2018

fejta-bot commented Mar 15, 2018

fejta-bot commented Mar 15, 2018

k8s-ci-robot commented Mar 19, 2018 •

edited

Loading

fejta-bot commented Mar 19, 2018

k8s-github-robot commented Mar 20, 2018

k8s-github-robot commented Mar 20, 2018

kubelet: make --cni-bin-dir accept a comma-separated list of CNI plugin directories #58714

kubelet: make --cni-bin-dir accept a comma-separated list of CNI plugin directories #58714

Conversation

dcbw commented Jan 23, 2018 • edited Loading

dcbw commented Jan 24, 2018

dcbw commented Jan 24, 2018

feiskyer Jan 24, 2018

Choose a reason for hiding this comment

dcbw commented Feb 5, 2018

feiskyer left a comment

Choose a reason for hiding this comment

dcbw commented Feb 6, 2018

dcbw commented Feb 21, 2018

dcbw commented Feb 21, 2018 • edited Loading

bboreham commented Feb 21, 2018

feiskyer left a comment

Choose a reason for hiding this comment

feiskyer commented Feb 24, 2018

yujuhong Feb 28, 2018

Choose a reason for hiding this comment

dcbw Mar 1, 2018

Choose a reason for hiding this comment

yujuhong Feb 28, 2018

Choose a reason for hiding this comment

dcbw Mar 1, 2018

Choose a reason for hiding this comment

dcbw commented Mar 1, 2018

yujuhong commented Mar 1, 2018

k8s-ci-robot commented Mar 1, 2018

fejta-bot commented Mar 10, 2018

fejta-bot commented Mar 10, 2018

fejta-bot commented Mar 14, 2018

fejta-bot commented Mar 15, 2018

fejta-bot commented Mar 15, 2018

k8s-ci-robot commented Mar 19, 2018 • edited Loading

fejta-bot commented Mar 19, 2018

k8s-github-robot commented Mar 20, 2018

k8s-github-robot commented Mar 20, 2018

dcbw commented Jan 23, 2018 •

edited

Loading

dcbw commented Feb 21, 2018 •

edited

Loading

k8s-ci-robot commented Mar 19, 2018 •

edited

Loading