-
Notifications
You must be signed in to change notification settings - Fork 4.8k
cmd/openshift-sdn: Allow user-specified CNI configuration files #22898
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
Conversation
|
/hold wip |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: squeed The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold |
This lets us specify the exact CNI configuration file we want, but still only writes it when the network is ready.
|
Bah, it failed tests because the file path is the readiness probe. Can't change that. |
|
Test failures seem like a rebase issue. Race detector triggered on some upstream code. |
|
/retest |
|
/retest |
|
Weird, ovs failed to come up in the last upgrade run. That shouldn't have anything to do with this PR, but it's unsettling... |
|
/retest |
|
|
||
| cniConfDir := "/etc/cni/net.d" | ||
| if val, ok := sdn.NodeConfig.KubeletArguments["cni-conf-dir"]; ok && len(val) == 1 { | ||
| cniConfDir = val[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@squeed so we're dropping the enforced same CNI configdir path between kubelet and the SDN and just making sure we manage that manually through the daemonset YAML via host mounts now? Which is fine, just want to be sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
basically, yes - given that NodeConfig doesn't exist anyways.
|
@squeed: PR needs rebase. DetailsInstructions 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. |
This lets us specify the exact CNI configuration file we want, but still only writes it when the network is ready.