-
Notifications
You must be signed in to change notification settings - Fork 366
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
Force create cni folders when starting Windows containerd agent #4685
Conversation
Create /opt/cni/bin/ and /etc/cni/net.d/ folders to avoid errors caused by non-existent paths. Fixes antrea-io#4680 Signed-off-by: Shuyang Xin <gavinx@vmware.com>
/test-windows-all |
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
@@ -9,6 +9,8 @@ data: | |||
mkdir -force C:/var/run/secrets/kubernetes.io/serviceaccount | |||
cp $mountPath/var/run/secrets/kubernetes.io/serviceaccount/ca.crt C:/var/run/secrets/kubernetes.io/serviceaccount | |||
cp $mountPath/var/run/secrets/kubernetes.io/serviceaccount/token C:/var/run/secrets/kubernetes.io/serviceaccount | |||
mkdir -force c:/opt/cni/bin/ |
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.
As the directory is created in the initContainers, the path may be deleted when Pod is deleted. Is it as expected?
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.
really? then how could cp $mountPath/k/antrea/cni/* c:/opt/cni/bin/
below take effect if c:/opt/cni/bin/
is not in host filesystem?
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.
Yes they are created in host filesystem and deleting pod won't delete these directories.
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.
It should by my fault. Before it can be merged, I also want to know the difference between this solution and the one that uses hostPath in container volumes?
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.
I don't know how this actually works because there is no configuration saying c:/
is a host volume, I guess Windows hostProcess Pod is different from Linux Pod and it has direct access to host filesystem. Then perhaps there is no point to leverage hostPath to achieve it just for these two directories given the script already creates other directories in host filesystem.
/test-windows-all |
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
/skip-all |
…ea-io#4685) Create /opt/cni/bin/ and /etc/cni/net.d/ folders to avoid errors caused by non-existent paths. Fixes antrea-io#4680 Signed-off-by: Shuyang Xin <gavinx@vmware.com>
Create /opt/cni/bin/ and /etc/cni/net.d/ folders to avoid errors caused by non-existent paths.
Fixes #4680