From e3b45667a6144caf258a1024a61754955d1b97c7 Mon Sep 17 00:00:00 2001 From: Predrag Rogic Date: Fri, 23 Apr 2021 23:27:34 +0100 Subject: [PATCH] improve how cni and cruntimes work together --- cmd/minikube/cmd/start_flags.go | 23 +----- pkg/minikube/bootstrapper/bsutil/kubeadm.go | 2 +- pkg/minikube/bootstrapper/kubeadm/kubeadm.go | 9 +-- pkg/minikube/cni/cni.go | 80 +++++++++++++++----- pkg/minikube/cni/kindnet.go | 2 +- pkg/minikube/cruntime/containerd.go | 5 +- pkg/minikube/cruntime/crio.go | 37 +++------ pkg/minikube/node/start.go | 2 +- 8 files changed, 81 insertions(+), 79 deletions(-) diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index d3a33cd463..597fe2a064 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -253,7 +253,7 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k klog.Info("no existing cluster config was found, will generate one from the flags ") cc = generateNewConfigFromFlags(cmd, k8sVersion, drvName) - cnm, err := cni.New(cc) + cnm, err := cni.New(&cc) if err != nil { return cc, config.Node{}, errors.Wrap(err, "cni") } @@ -261,9 +261,6 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k if _, ok := cnm.(cni.Disabled); !ok { klog.Infof("Found %q CNI - setting NetworkPlugin=cni", cnm) cc.KubernetesConfig.NetworkPlugin = "cni" - if err := setCNIConfDir(&cc, cnm); err != nil { - klog.Errorf("unable to set CNI Config Directory: %v", err) - } } } @@ -428,24 +425,6 @@ func generateNewConfigFromFlags(cmd *cobra.Command, k8sVersion string, drvName s return cc } -// setCNIConfDir sets kubelet's '--cni-conf-dir' flag to custom CNI Config Directory path (same used also by CNI Deployment) to avoid conflicting CNI configs. -// ref: https://github.com/kubernetes/minikube/issues/10984 -// Note: currently, this change affects only Kindnet CNI (and all multinodes using it), but it can be easily expanded to other/all CNIs if needed. -func setCNIConfDir(cc *config.ClusterConfig, cnm cni.Manager) error { - if _, kindnet := cnm.(cni.KindNet); kindnet { - // auto-set custom CNI Config Directory, if not user-specified - eo := fmt.Sprintf("kubelet.cni-conf-dir=%s", cni.CustomCNIConfDir) - if !cc.KubernetesConfig.ExtraOptions.Exists(eo) { - klog.Infof("auto-setting extra-config to %q", eo) - if err := cc.KubernetesConfig.ExtraOptions.Set(eo); err != nil { - return fmt.Errorf("failed auto-setting extra-config %q: %v", eo, err) - } - klog.Infof("extra-config set to %q", eo) - } - } - return nil -} - func checkNumaCount(k8sVersion string) { if viper.GetInt(kvmNUMACount) < 1 || viper.GetInt(kvmNUMACount) > 8 { exit.Message(reason.Usage, "--kvm-numa-count range is 1-8") diff --git a/pkg/minikube/bootstrapper/bsutil/kubeadm.go b/pkg/minikube/bootstrapper/bsutil/kubeadm.go index fbbce5940c..2a1f750cf4 100644 --- a/pkg/minikube/bootstrapper/bsutil/kubeadm.go +++ b/pkg/minikube/bootstrapper/bsutil/kubeadm.go @@ -74,7 +74,7 @@ func GenerateKubeadmYAML(cc config.ClusterConfig, n config.Node, r cruntime.Mana return nil, errors.Wrap(err, "generating extra component config for kubeadm") } - cnm, err := cni.New(cc) + cnm, err := cni.New(&cc) if err != nil { return nil, errors.Wrap(err, "cni") } diff --git a/pkg/minikube/bootstrapper/kubeadm/kubeadm.go b/pkg/minikube/bootstrapper/kubeadm/kubeadm.go index 4b86d4f4ad..959703e973 100644 --- a/pkg/minikube/bootstrapper/kubeadm/kubeadm.go +++ b/pkg/minikube/bootstrapper/kubeadm/kubeadm.go @@ -259,6 +259,7 @@ func (k *Bootstrapper) init(cfg config.ClusterConfig) error { } kw.Close() wg.Wait() + if err := k.applyCNI(cfg, true); err != nil { return errors.Wrap(err, "apply cni") } @@ -330,7 +331,7 @@ func (k *Bootstrapper) applyCNI(cfg config.ClusterConfig, registerStep ...bool) regStep = registerStep[0] } - cnm, err := cni.New(cfg) + cnm, err := cni.New(&cfg) if err != nil { return errors.Wrap(err, "cni config") } @@ -351,12 +352,6 @@ func (k *Bootstrapper) applyCNI(cfg config.ClusterConfig, registerStep ...bool) return errors.Wrap(err, "cni apply") } - if cfg.KubernetesConfig.ContainerRuntime == constants.CRIO { - if err := cruntime.UpdateCRIONet(k.c, cnm.CIDR()); err != nil { - return errors.Wrap(err, "update crio") - } - } - return nil } diff --git a/pkg/minikube/cni/cni.go b/pkg/minikube/cni/cni.go index 63519ae4f8..c007b23608 100644 --- a/pkg/minikube/cni/cni.go +++ b/pkg/minikube/cni/cni.go @@ -30,6 +30,7 @@ import ( "k8s.io/minikube/pkg/minikube/assets" "k8s.io/minikube/pkg/minikube/command" "k8s.io/minikube/pkg/minikube/config" + "k8s.io/minikube/pkg/minikube/constants" "k8s.io/minikube/pkg/minikube/driver" "k8s.io/minikube/pkg/minikube/vmpath" ) @@ -37,12 +38,23 @@ import ( const ( // DefaultPodCIDR is the default CIDR to use in minikube CNI's. DefaultPodCIDR = "10.244.0.0/16" + + // DefaultConfDir is the default CNI Config Directory path + DefaultConfDir = "/etc/cni/net.d" + // CustomConfDir is the custom CNI Config Directory path used to avoid conflicting CNI configs + // ref: https://github.com/kubernetes/minikube/issues/10984 and https://github.com/kubernetes/minikube/pull/11106 + CustomConfDir = "/etc/cni/net.mk" ) var ( - // CustomCNIConfDir is the custom CNI Config Directory path used to avoid conflicting CNI configs - // ref: https://github.com/kubernetes/minikube/issues/10984 - CustomCNIConfDir = "/etc/cni/net.mk" + // ConfDir is the CNI Config Directory path that can be customised, defaulting to DefaultConfDir + ConfDir = DefaultConfDir + + // Network is the network name that CNI should use (eg, "kindnet"). + // Currently, only crio (and podman) can use it, so that setting custom ConfDir is not necessary. + // ref: https://github.com/cri-o/cri-o/issues/2121 (and https://github.com/containers/podman/issues/2370) + // ref: https://github.com/cri-o/cri-o/blob/master/docs/crio.conf.5.md#crionetwork-table + Network = "" ) // Runner is the subset of command.Runner this package consumes @@ -72,7 +84,7 @@ type tmplInput struct { } // New returns a new CNI manager -func New(cc config.ClusterConfig) (Manager, error) { +func New(cc *config.ClusterConfig) (Manager, error) { if cc.KubernetesConfig.NetworkPlugin != "" && cc.KubernetesConfig.NetworkPlugin != "cni" { klog.Infof("network plugin configured as %q, returning disabled", cc.KubernetesConfig.NetworkPlugin) return Disabled{}, nil @@ -80,30 +92,32 @@ func New(cc config.ClusterConfig) (Manager, error) { klog.Infof("Creating CNI manager for %q", cc.KubernetesConfig.CNI) - // respect user-specified custom CNI Config Directory, if any - userCNIConfDir := cc.KubernetesConfig.ExtraOptions.Get("cni-conf-dir", "kubelet") - if userCNIConfDir != "" { - CustomCNIConfDir = userCNIConfDir - } - + var cnm Manager + var err error switch cc.KubernetesConfig.CNI { case "", "auto": - return chooseDefault(cc), nil + cnm = chooseDefault(*cc) case "false": - return Disabled{cc: cc}, nil + cnm = Disabled{cc: *cc} case "kindnet", "true": - return KindNet{cc: cc}, nil + cnm = KindNet{cc: *cc} case "bridge": - return Bridge{cc: cc}, nil + cnm = Bridge{cc: *cc} case "calico": - return Calico{cc: cc}, nil + cnm = Calico{cc: *cc} case "cilium": - return Cilium{cc: cc}, nil + cnm = Cilium{cc: *cc} case "flannel": - return Flannel{cc: cc}, nil + cnm = Flannel{cc: *cc} default: - return NewCustom(cc, cc.KubernetesConfig.CNI) + cnm, err = NewCustom(*cc, cc.KubernetesConfig.CNI) } + + if err := configureCNI(cc, cnm); err != nil { + klog.Errorf("unable to set CNI Config Directory: %v", err) + } + + return cnm, err } // IsDisabled checks if CNI is disabled @@ -183,3 +197,33 @@ func applyManifest(cc config.ClusterConfig, r Runner, f assets.CopyableFile) err return nil } + +// configureCNI - to avoid conflicting CNI configs, it sets: +// - for crio: 'cni_default_network' config param via cni.Network +// - for containerd and docker: kubelet's '--cni-conf-dir' flag to custom CNI Config Directory path (same used also by CNI Deployment). +// ref: https://github.com/kubernetes/minikube/issues/10984 and https://github.com/kubernetes/minikube/pull/11106 +// Note: currently, this change affects only Kindnet CNI (and all multinodes using it), but it can be easily expanded to other/all CNIs if needed. +// Note2: Cilium does not need workaround as they automatically restart pods after CNI is successfully deployed. +func configureCNI(cc *config.ClusterConfig, cnm Manager) error { + if _, kindnet := cnm.(KindNet); kindnet { + // crio only needs CNI network name; hopefully others (containerd, docker and kubeadm/kubelet) will follow eventually + if cc.KubernetesConfig.ContainerRuntime == constants.CRIO { + Network = "kindnet" + return nil + } + // for containerd and docker: auto-set custom CNI via kubelet's 'cni-conf-dir' param, if not user-specified + eo := fmt.Sprintf("kubelet.cni-conf-dir=%s", CustomConfDir) + if !cc.KubernetesConfig.ExtraOptions.Exists(eo) { + klog.Infof("auto-setting extra-config to %q", eo) + if err := cc.KubernetesConfig.ExtraOptions.Set(eo); err != nil { + return fmt.Errorf("failed auto-setting extra-config %q: %v", eo, err) + } + ConfDir = CustomConfDir + klog.Infof("extra-config set to %q", eo) + } else { + // respect user-specified custom CNI Config Directory + ConfDir = cc.KubernetesConfig.ExtraOptions.Get("cni-conf-dir", "kubelet") + } + } + return nil +} diff --git a/pkg/minikube/cni/kindnet.go b/pkg/minikube/cni/kindnet.go index 38fd39cad1..d25d4b6135 100644 --- a/pkg/minikube/cni/kindnet.go +++ b/pkg/minikube/cni/kindnet.go @@ -166,7 +166,7 @@ func (c KindNet) manifest() (assets.CopyableFile, error) { DefaultRoute: "0.0.0.0/0", // assumes IPv4 PodCIDR: DefaultPodCIDR, ImageName: images.KindNet(c.cc.KubernetesConfig.ImageRepository), - CNIConfDir: CustomCNIConfDir, + CNIConfDir: ConfDir, } b := bytes.Buffer{} diff --git a/pkg/minikube/cruntime/containerd.go b/pkg/minikube/cruntime/containerd.go index 78738be5c9..e0e38336c7 100644 --- a/pkg/minikube/cruntime/containerd.go +++ b/pkg/minikube/cruntime/containerd.go @@ -34,6 +34,7 @@ import ( "k8s.io/klog/v2" "k8s.io/minikube/pkg/minikube/assets" "k8s.io/minikube/pkg/minikube/bootstrapper/images" + "k8s.io/minikube/pkg/minikube/cni" "k8s.io/minikube/pkg/minikube/command" "k8s.io/minikube/pkg/minikube/config" "k8s.io/minikube/pkg/minikube/download" @@ -94,7 +95,7 @@ oom_score = 0 runtime_root = "" [plugins.cri.cni] bin_dir = "/opt/cni/bin" - conf_dir = "/etc/cni/net.d" + conf_dir = "{{.CNIConfDir}}" conf_template = "" [plugins.cri.registry] [plugins.cri.registry.mirrors] @@ -190,10 +191,12 @@ func generateContainerdConfig(cr CommandRunner, imageRepository string, kv semve PodInfraContainerImage string SystemdCgroup bool InsecureRegistry []string + CNIConfDir string }{ PodInfraContainerImage: pauseImage, SystemdCgroup: forceSystemd, InsecureRegistry: insecureRegistry, + CNIConfDir: cni.ConfDir, } var b bytes.Buffer if err := t.Execute(&b, opts); err != nil { diff --git a/pkg/minikube/cruntime/crio.go b/pkg/minikube/cruntime/crio.go index faea475faf..ba4272fdc9 100644 --- a/pkg/minikube/cruntime/crio.go +++ b/pkg/minikube/cruntime/crio.go @@ -19,7 +19,6 @@ package cruntime import ( "encoding/json" "fmt" - "net" "os" "os/exec" "path" @@ -31,6 +30,7 @@ import ( "k8s.io/klog/v2" "k8s.io/minikube/pkg/minikube/assets" "k8s.io/minikube/pkg/minikube/bootstrapper/images" + "k8s.io/minikube/pkg/minikube/cni" "k8s.io/minikube/pkg/minikube/command" "k8s.io/minikube/pkg/minikube/config" "k8s.io/minikube/pkg/minikube/download" @@ -61,6 +61,14 @@ func generateCRIOConfig(cr CommandRunner, imageRepository string, kv semver.Vers if _, err := cr.RunCmd(c); err != nil { return errors.Wrap(err, "generateCRIOConfig.") } + + if cni.Network != "" { + klog.Infof("Updating CRIO to use the custom CNI network %q", cni.Network) + if _, err := cr.RunCmd(exec.Command("/bin/bash", "-c", fmt.Sprintf("sudo sed -e 's|^.*cni_default_network = .*$|cni_default_network = \"%s\"|' -i %s", cni.Network, crioConfigFile))); err != nil { + return errors.Wrap(err, "update network_dir") + } + } + return nil } @@ -405,30 +413,3 @@ func crioImagesPreloaded(runner command.Runner, images []string) bool { func (r *CRIO) ImagesPreloaded(images []string) bool { return crioImagesPreloaded(r.Runner, images) } - -// UpdateCRIONet updates CRIO CNI network configuration and restarts it -func UpdateCRIONet(r CommandRunner, cidr string) error { - klog.Infof("Updating CRIO to use CIDR: %q", cidr) - ip, net, err := net.ParseCIDR(cidr) - if err != nil { - return errors.Wrap(err, "parse cidr") - } - - oldNet := "10.88.0.0/16" - oldGw := "10.88.0.1" - - newNet := cidr - - // Assume gateway is first IP in netmask (10.244.0.1, for instance) - newGw := ip.Mask(net.Mask) - newGw[3]++ - - // Update subnets used by 100-crio-bridge.conf & 87-podman-bridge.conflist - // avoids: "Error adding network: failed to set bridge addr: could not add IP address to \"cni0\": permission denied" - sed := fmt.Sprintf("sed -i -e s#%s#%s# -e s#%s#%s# /etc/cni/net.d/*bridge*", oldNet, newNet, oldGw, newGw) - if _, err := r.RunCmd(exec.Command("sudo", "/bin/bash", "-c", sed)); err != nil { - klog.Errorf("netconf update failed: %v", err) - } - - return sysinit.New(r).Restart("crio") -} diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index acbf73b96f..b35bd39ff4 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -188,7 +188,7 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) { return nil, errors.Wrap(err, "joining cp") } - cnm, err := cni.New(*starter.Cfg) + cnm, err := cni.New(starter.Cfg) if err != nil { return nil, errors.Wrap(err, "cni") }