Allow configurable CNI bin dir in openshift SDN

[pravisankar]

No description provided.

[pravisankar]

@openshift/sig-networking @dcbw PTAL

[pravisankar]

/retest

[pravisankar]

/retest

[dcbw]

/lgtm
/approve

But we could also perhaps grab this cni-bin-dir and set it in podManager and use that instead of /opt/cni/bin, if for some reason the CNI binaries do get retargeted.

[smarterclayton]

Why are we adding this? Who would set this wrong? Adding this check is only valuable if we expect this to be hit most of the time. This entire code path is going away very soon, so there's not much value in adding it.

[pravisankar]

kubelet supports allowing users to specify path to cni binaries with 'cni-bin-dir' option.
OpenShift sdn ignores 'cni-bin-dir' and always looks at default /opt/cni/bin as we expect the rpms to place the binaries in the default path.
For some reason, if they change the cni binaries and use this flag, sadly we don't even bailout and pod creation fails with some other error.
This change doesn't allow user to set cni-bin-dir to non-default path in case of openshift-sdn. Other option is we could also change openshift sdn to honor this flag.
But yes, this is a rare corner case. Do we want to handle this? Is there a compelling reason to move openshift-sdn cni binary to non-default location? (may be in case of third party cni plugins that use different path)
@smarterclayton @dcbw ^^

[dcbw]

@pravisankar @smarterclayton /opt is forbidden in some environments, and especially on Fedora/RHEL /opt is not really an allowed place to put stuff. Fedora/RHEL would rather the CNI plugins live in /usr/libexec and in fact the containernetworking-plugins RPM in Fedora puts them there, not /opt. There have also been upstream PRs like kubernetes/kubernetes#46609 that want to put plugins elsewhere.

We're probably going to need to support /usr/libexec for CNI plugins when we do the ovn-kubernetes RPM as well.

[pravisankar]

/hold

[pravisankar]

@dcbw @smarterclayton updated based on the feedback, PTAL

[dcbw]

/lgtm

At least the SDN parts are useful for the future; I know the kube flags/args thing is going away since we'll be doing a daemonset. But @smarterclayton good point, we need to figure out how the CNI side of things is done when the SDN is a daemonset; I guess cni-bin-dir and cni-conf-dir is going to come from a ConfigMap? Perhaps cni-bin-dir/cni-conf-dir ConfigMap options could be automatically generated based on the kubelet options/config?

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dcbw, pravisankar

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:

• pkg/cmd/server/kubernetes/network/OWNERS [dcbw]
• pkg/network/OWNERS [dcbw,pravisankar]

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

[pravisankar]

/retest

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.

[openshift-merge-robot]

/test all [submit-queue is verifying that this PR is safe to merge]

[openshift-ci-robot]

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

Test name	Commit	Details	Rerun command
ci/openshift-jenkins/extended_networking_minimal	65abbab	link	/test extended_networking_minimal

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.

[openshift-merge-robot]

Automatic merge from submit-queue.