-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[master] Add option to change CNI network default #2123
Conversation
Hi @haircommander. Thanks for your PR. I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
Adding WIP tag because: this patch currently doesn't actually work. Further, there should be some discussion on the default config name. Lastly, the documentation has to be added if this change is breaking. I just put it here to run against CI to see if any of my changes thus far break anything. |
d1074cb
to
043a896
Compare
3cac136
to
a31c67c
Compare
/test e2e_features_rhel |
a31c67c
to
42727aa
Compare
42727aa
to
e1a6e87
Compare
/test kata-containers |
/retest |
@haircommander: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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. |
/ok-to-test |
e1a6e87
to
58a9dbd
Compare
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: haircommander, rhatdan, sboeuf 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 |
1 similar comment
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: haircommander, rhatdan, sboeuf 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 |
/retest |
36c8fbe
to
a501214
Compare
Before, CRI-O would get confused when there were other CNI configuration files in the same plugin_dir. Fix this by updating CNI and ocicni to latest version, which allow for tools to ignore config files with different names. Now, users can run CRI-O and change the cni_default_network name, which allows them to have multiple tools to occupy the same directory. The default value for this is crio. Update documentation to reflect this change. Signed-off-by: Peter Hunt <pehunt@redhat.com>
a501214
to
61eba8a
Compare
/retest |
@haircommander: PR needs rebase. 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. |
@haircommander Is this still something you are working on? |
@rhatdan it was backlogged but I'd like to continue working on it. |
@haircommander this needs a rebase :) |
@haircommander: The following tests failed, say
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. |
@haircommander Needs a rebase. |
on my todo, which has gotten much longer with all of these pings 😄 |
@giuseppe Do you have a chance to take this one over? |
If @giuseppe has no time I'm keen to take this one as well. |
@saschagrunert Take it. I am sure @giuseppe Has plenty of other things to do? I always have pet projects for him. :^) (User namespace...) |
closing in favor of #3452 |
- What I did
Began work on changing the network configurations from using every config file in the config dir to only ones specified.
- How I did it
Vendored in changes to ocicni and cni to allow for adding default network config to InitCNI
- Description for the changelog