k8snetworkplumbingwg / multus-cni

A CNI meta-plugin for multi-homed pods in Kubernetes
Apache License 2.0
2.27k stars 577 forks source link

Use same base image as thick plugin #1298

Open adrianchiris opened 3 weeks ago

adrianchiris commented 3 weeks ago

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 (thin) from the cluster without breaking it.

this functionality was possible in v3.

Related issue: #592

coveralls commented 3 weeks ago

Coverage Status

coverage: 63.364% (+0.2%) from 63.116% when pulling 4e1bca1a369fbc21036301bb5f159dadebaeb881 on adrianchiris:use-same-base-image into aff99fccc5fe3b21aec84bb56a6d1bbaf8ffc45b on k8snetworkplumbingwg:master.

adrianchiris commented 3 weeks ago

An alternative solution is to actually clean up multus cni config file on exit if cleanup-config-on-exit flag is set in thin_entrypoint[1]

note: thick plugin today cleans up multus config file on exit[2] so im thinking we should support the same in thin mode as well.

[1] https://github.com/k8snetworkplumbingwg/multus-cni/blob/aff99fccc5fe3b21aec84bb56a6d1bbaf8ffc45b/cmd/thin_entrypoint/main.go#L607 [2] https://github.com/k8snetworkplumbingwg/multus-cni/blob/aff99fccc5fe3b21aec84bb56a6d1bbaf8ffc45b/pkg/server/config/manager.go#L163

s1061123 commented 2 weeks ago

We use distroless for less maintainance for CVE in container image because distroless image, gcr.io/distroless/base-debian11:latest, has less components than distro image, debian:stable-slim. So we (or I, at least), expects to change thick pluign container image to distroless in the future. Not to change thin plugin to distro image.

Currently our release container image is just for simple use cases and it does not care about whole lifecycle for all deployment scenarios (on all platforms). If you want to do more, we expect that you should have original container image for that.

So I suppose we should not change thin pluign base images.

adrianchiris commented 2 weeks ago

if we are ok with the approach in #1299 then this one can be closed.