Skip to content
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

can create the directory for the customized cni conf and remove the cni conf file in cleanup command #27933

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

sofat1989
Copy link
Contributor

Usually we will write the customized cni conf file and write it into the directory /etc/cni/net.d in the host. For example,

- mountPath: /host/etc/cni/net.d
          name: etc-cni-netd

- hostPath:
          path: /etc/cni/net.d
          type: DirectoryOrCreate
        name: etc-cni-netd

So the value of write-cni-conf-when-ready is /host/etc/cni/net.d/xxx-cilium.conf.
In the cleanup command, it will not remove this file.

And sometimes we need to specify write-cni-conf-when-ready to any pathes, and the directory may not exist. So the writing cni file will fail.

The commit is to solve these two issues above.

can create the directory for the customized cni conf and remove the cni conf file in cleanup command

…ni conf file in cleanup command

Signed-off-by: Su Fei <sofat1989@126.com>
@sofat1989 sofat1989 requested review from a team as code owners September 5, 2023 05:27
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Sep 5, 2023
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Sep 5, 2023
@gandro
Copy link
Member

gandro commented Sep 5, 2023

cc @squeed as our resident CNI configuration file expert

@asauber
Copy link
Member

asauber commented Sep 11, 2023

The general idea here LGTM, and this does look necessary, given that we have a config option for the path.

A better release note would be:

create directory for custom CNI config path on startup
remove custom CNI config file with cilium cleanup command

Copy link
Member

@gandro gandro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helm looks good to me. I'll defer the logic changes to the other reviewers

@gandro
Copy link
Member

gandro commented Sep 12, 2023

/test

@gandro gandro added area/cni Impacts the Container Networking Interface between Cilium and the orchestrator. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Sep 12, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Sep 12, 2023
@julianwiedmann julianwiedmann merged commit 2e894f3 into cilium:main Sep 12, 2023
61 of 63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cni Impacts the Container Networking Interface between Cilium and the orchestrator. kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants