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.

Sign up for GitHub

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

Jump to bottom

Clean Up auto generated config #592

Closed
Mmduh-483 opened this issue Dec 13, 2020 · 5 comments
Closed

Clean Up auto generated config #592

Mmduh-483 opened this issue Dec 13, 2020 · 5 comments

Comments

@Mmduh-483
Copy link
Contributor

When multus daemonset is deployed with --multus-conf-file=auto, it generates an auto cnofig file in CNI_DIR, usually it is 00-multus. After removing the multus daemonset and roles, this file will stay which cause to call multus cni that doesn't have the permissions to access k8s api, that cause the multus to fail everytime.

What would you like to be added:
Clean up the auto generated config file

Why is this needed:
Leaving config file for mulus without permission causing the cluster to not be able to schedule pods
@Mmduh-483 Mmduh-483 mentioned this issue Dec 14, 2020
Closed
adrianchiris added a commit to adrianchiris/network-operator that referenced this issue Dec 28, 2020
@adrianchiris
Change default Multus CNI version
6ede799 
Due to Issue: k8snetworkplumbingwg/multus-cni#592
When multus is removed from the cluster it is left in an inoperable
as CNI calls fail due to removed RBAC.

While it makes sense to occur for secondary network when specified
in a pod spec as net-attach-def name, it should not fail on primary
network.

Downgrade multus CNI to v3.4.1 where such an issue does not occur.
@adrianchiris adrianchiris mentioned this issue Dec 28, 2020
Merged
adrianchiris added a commit to adrianchiris/network-operator that referenced this issue Dec 28, 2020
@adrianchiris
Change default Multus CNI version
1d8c790 
Due to Issue: k8snetworkplumbingwg/multus-cni#592
When multus is removed from the cluster it is left in an inoperable
as CNI calls fail due to removed RBAC.

While it makes sense to occur for secondary network when specified
in a pod spec as net-attach-def name, it should not fail on primary
network.

Downgrade multus CNI to v3.4.1 where such an issue does not occur.

Signed-off-by: Adrian Chiris <[email protected]>
@adrianchiris
Copy link
Contributor

It should be noted that in multus v3.4.1 this operation is not observed

so with multus v3.4.1 when you deploy it in a k8s cluster and then delete (daemonset, rbac) pods with primary network keep on working

with multus v3.6 doing the same leaves k8s cluster in an inoperable state as multus CNI calls now fail.

i didnt have time to dig into this issue but it seems that between v3.4.1 and v3.6 we lost the ability of multus to handle failure to access kubernetes API when not nessecary (e.g no secondary network for pod and not clusterNetwork crd specified in multus config)

@adpempx
Copy link

adpempx commented Feb 25, 2021

Hi,

do you please have a proper solution?
This is a critical issue Multus-CNI team.

Please let us know as soon as possibile.

Thanks

@adrianchiris
Copy link
Contributor

In NVIDIA network operator we currently WA this issue by deploying multus daemonset with a pre-stop hook which deletes the generated multus cni config file (we use --multus-config-file=auto).

see: https://github.com/Mellanox/network-operator/blob/2dcf59f17d737b11632da2f41ed640aaf8323ee5/manifests/stage-multus-cni/0050-multus-ds.yml#L44

@adpempx
Copy link

adpempx commented Feb 25, 2021

@adrianchiris

you are my hero!

I confirm it is the solution!

Thanks

@dougbtv
Copy link
Member

dougbtv commented Mar 4, 2021

Great approach Adrian, I think handling this with an operator is great, that's an excellent way to make an opinionated way to handle the lifecycle and to denote node readiness based on a specific scenario.

@dougbtv dougbtv closed this as completed Mar 4, 2021
@qinqon qinqon mentioned this issue Feb 23, 2022
Merged
qinqon added a commit to qinqon/cluster-network-addons-operator that referenced this issue Feb 23, 2022
@qinqon
multus: Delete config preStop hook
f9cf10e 
After multus daemonset removal if the multus config is in place the
multus CNI binary will be used to create/delete network namespaces for
pods, this will fail since the apiserver access done at multus fails at
authentication if the multus pod is gone [1].

This change add the suggested solution adding a preStop hook that will
remove the multus config before stop.

[1] k8snetworkplumbingwg/multus-cni#592

Signed-off-by: Quique Llorente <[email protected]>
qinqon added a commit to qinqon/cluster-network-addons-operator that referenced this issue Feb 24, 2022
@qinqon
multus: Delete config preStop hook
f6f954f 
After multus daemonset removal if the multus config is in place the
multus CNI binary will be used to create/delete network namespaces for
pods, this will fail since the apiserver access done at multus fails at
authentication if the multus pod is gone [1].

This change add the suggested solution adding a preStop hook that will
remove the multus config before stop.

[1] k8snetworkplumbingwg/multus-cni#592

Signed-off-by: Quique Llorente <[email protected]>
qinqon added a commit to qinqon/cluster-network-addons-operator that referenced this issue Feb 24, 2022
@qinqon
multus: Delete config preStop hook
78e3ae0 
After multus daemonset removal if the multus config is in place the
multus CNI binary will be used to create/delete network namespaces for
pods, this will fail since the apiserver access done at multus fails at
authentication if the multus pod is gone [1].

This change add the suggested solution adding a preStop hook that will
remove the multus config before stop.

[1] k8snetworkplumbingwg/multus-cni#592

Signed-off-by: Quique Llorente <[email protected]>
qinqon added a commit to qinqon/cluster-network-addons-operator that referenced this issue Feb 24, 2022
@qinqon
multus: Delete config preStop hook
0703604 
After multus daemonset removal if the multus config is in place the
multus CNI binary will be used to create/delete network namespaces for
pods, this will fail since the apiserver access done at multus fails at
authentication if the multus pod is gone [1].

This change add the suggested solution adding a preStop hook that will
remove the multus config before stop.

[1] k8snetworkplumbingwg/multus-cni#592

Signed-off-by: Quique Llorente <[email protected]>
kubevirt-bot pushed a commit to kubevirt/cluster-network-addons-operator that referenced this issue Feb 24, 2022
@qinqon
multus: Bump to 3.8 (#1219)
1dc64c6 
* multus: Bump to 3.8

Multus project has change a little:
- They don't have a manifests pert arch since they are using multiarch
  registry.
- They have bump golang to 1.17

This change moves away golang origin builder since there is non for
1.17

Signed-off-by: Quique Llorente <[email protected]>

* multus: Delete config preStop hook

After multus daemonset removal if the multus config is in place the
multus CNI binary will be used to create/delete network namespaces for
pods, this will fail since the apiserver access done at multus fails at
authentication if the multus pod is gone [1].

This change add the suggested solution adding a preStop hook that will
remove the multus config before stop.

[1] k8snetworkplumbingwg/multus-cni#592

Signed-off-by: Quique Llorente <[email protected]>
adrianchiris added a commit to adrianchiris/multus-cni that referenced this issue Jun 16, 2024
@adrianchiris
Use same base image as thick plugin
4e1bca1 
change thin plugin dockerfile to use the same
base image as the thick plugin.

this will allow users to implement simple lifecycle
hooks e.g exec simple shell cmd to delete multus conf
file on container exit which allows to remove multus
from the cluster without breaking it.

this functionality was possible in v3.

Related issue: k8snetworkplumbingwg#592

Signed-off-by: adrianc <[email protected]>
@adrianchiris adrianchiris mentioned this issue Jun 16, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Cleanup multus config file 00-multus.conf when deleting daemonset Mmduh-483/multus-cni
4 participants
@dougbtv @adrianchiris @Mmduh-483 @adpempx