-
Notifications
You must be signed in to change notification settings - Fork 55
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
Propagate existing CNI_ARGS to non-k8s consumers, e.g., podman #70
Conversation
/assign @sameo |
Signed-off-by: Chan Shih-Ping <[email protected]>
72568c4
to
21dcfcd
Compare
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.
LGTM
Signed-off-by: Chan Shih-Ping <[email protected]>
Bump - thank you. |
@squeed can you weigh in? |
Hmm. We solved this a quite different way in rkt - you could do something like It would be good if podman itself decided what CNI_ARGS should be and passed them directly to oci-cni instead. If it wants to gather them from the environment, that's OK but not optimal (since it might have multiple networks). ocicni should probably not be adding additional args. |
"ocicni should probably not be adding additional args." — in this use case, it is trying to propagate the existing env, since that got overwritten by the K8S* stuff. |
@space88man @haircommander @mrunalp @dcbw @squeed @saschagrunert Is this still needed? Just trying to cleanup this Repo so we can do a release. |
@space88man @haircommander @mrunalp @dcbw @squeed @saschagrunert Is this still needed? Just trying to cleanup this Repo so we can do a release. |
It's not essential but it's definitely nice to have. |
@mccv1r0 Can you take a quick look at this and verify the way the args are being passed looks correct? |
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: mccv1r0, saschagrunert, space88man 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 |
|
/lgtm |
Let's try again. Add existing CNI_ARGS in the environment to rt.Args in case the caller is a non-k8s consumer like podman.
Addresses #65