Skip to content

Commit e3b4566

Browse files
prezhaprezha
committed
improve how cni and cruntimes work together
1 parent 222dbec commit e3b4566

File tree

8 files changed

+81
-79
lines changed

8 files changed

+81
-79
lines changed

Diff for: ‎cmd/minikube/cmd/start_flags.go

+1-22
Original file line numberDiff line numberDiff line change
@@ -253,17 +253,14 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k
253253
klog.Info("no existing cluster config was found, will generate one from the flags ")
254254
cc = generateNewConfigFromFlags(cmd, k8sVersion, drvName)
255255

256-
cnm, err := cni.New(cc)
256+
cnm, err := cni.New(&cc)
257257
if err != nil {
258258
return cc, config.Node{}, errors.Wrap(err, "cni")
259259
}
260260

261261
if _, ok := cnm.(cni.Disabled); !ok {
262262
klog.Infof("Found %q CNI - setting NetworkPlugin=cni", cnm)
263263
cc.KubernetesConfig.NetworkPlugin = "cni"
264-
if err := setCNIConfDir(&cc, cnm); err != nil {
265-
klog.Errorf("unable to set CNI Config Directory: %v", err)
266-
}
267264
}
268265
}
269266

@@ -428,24 +425,6 @@ func generateNewConfigFromFlags(cmd *cobra.Command, k8sVersion string, drvName s
428425
return cc
429426
}
430427

431-
// 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.
432-
// ref: https://github.com/kubernetes/minikube/issues/10984
433-
// 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.
434-
func setCNIConfDir(cc *config.ClusterConfig, cnm cni.Manager) error {
435-
if _, kindnet := cnm.(cni.KindNet); kindnet {
436-
// auto-set custom CNI Config Directory, if not user-specified
437-
eo := fmt.Sprintf("kubelet.cni-conf-dir=%s", cni.CustomCNIConfDir)
438-
if !cc.KubernetesConfig.ExtraOptions.Exists(eo) {
439-
klog.Infof("auto-setting extra-config to %q", eo)
440-
if err := cc.KubernetesConfig.ExtraOptions.Set(eo); err != nil {
441-
return fmt.Errorf("failed auto-setting extra-config %q: %v", eo, err)
442-
}
443-
klog.Infof("extra-config set to %q", eo)
444-
}
445-
}
446-
return nil
447-
}
448-
449428
func checkNumaCount(k8sVersion string) {
450429
if viper.GetInt(kvmNUMACount) < 1 || viper.GetInt(kvmNUMACount) > 8 {
451430
exit.Message(reason.Usage, "--kvm-numa-count range is 1-8")

Diff for: ‎pkg/minikube/bootstrapper/bsutil/kubeadm.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func GenerateKubeadmYAML(cc config.ClusterConfig, n config.Node, r cruntime.Mana
7474
return nil, errors.Wrap(err, "generating extra component config for kubeadm")
7575
}
7676

77-
cnm, err := cni.New(cc)
77+
cnm, err := cni.New(&cc)
7878
if err != nil {
7979
return nil, errors.Wrap(err, "cni")
8080
}

Diff for: ‎pkg/minikube/bootstrapper/kubeadm/kubeadm.go

+2-7
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@ func (k *Bootstrapper) init(cfg config.ClusterConfig) error {
259259
}
260260
kw.Close()
261261
wg.Wait()
262+
262263
if err := k.applyCNI(cfg, true); err != nil {
263264
return errors.Wrap(err, "apply cni")
264265
}
@@ -330,7 +331,7 @@ func (k *Bootstrapper) applyCNI(cfg config.ClusterConfig, registerStep ...bool)
330331
regStep = registerStep[0]
331332
}
332333

333-
cnm, err := cni.New(cfg)
334+
cnm, err := cni.New(&cfg)
334335
if err != nil {
335336
return errors.Wrap(err, "cni config")
336337
}
@@ -351,12 +352,6 @@ func (k *Bootstrapper) applyCNI(cfg config.ClusterConfig, registerStep ...bool)
351352
return errors.Wrap(err, "cni apply")
352353
}
353354

354-
if cfg.KubernetesConfig.ContainerRuntime == constants.CRIO {
355-
if err := cruntime.UpdateCRIONet(k.c, cnm.CIDR()); err != nil {
356-
return errors.Wrap(err, "update crio")
357-
}
358-
}
359-
360355
return nil
361356
}
362357

Diff for: ‎pkg/minikube/cni/cni.go

+62-18
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,31 @@ import (
3030
"k8s.io/minikube/pkg/minikube/assets"
3131
"k8s.io/minikube/pkg/minikube/command"
3232
"k8s.io/minikube/pkg/minikube/config"
33+
"k8s.io/minikube/pkg/minikube/constants"
3334
"k8s.io/minikube/pkg/minikube/driver"
3435
"k8s.io/minikube/pkg/minikube/vmpath"
3536
)
3637

3738
const (
3839
// DefaultPodCIDR is the default CIDR to use in minikube CNI's.
3940
DefaultPodCIDR = "10.244.0.0/16"
41+
42+
// DefaultConfDir is the default CNI Config Directory path
43+
DefaultConfDir = "/etc/cni/net.d"
44+
// CustomConfDir is the custom CNI Config Directory path used to avoid conflicting CNI configs
45+
// ref: https://github.com/kubernetes/minikube/issues/10984 and https://github.com/kubernetes/minikube/pull/11106
46+
CustomConfDir = "/etc/cni/net.mk"
4047
)
4148

4249
var (
43-
// CustomCNIConfDir is the custom CNI Config Directory path used to avoid conflicting CNI configs
44-
// ref: https://github.com/kubernetes/minikube/issues/10984
45-
CustomCNIConfDir = "/etc/cni/net.mk"
50+
// ConfDir is the CNI Config Directory path that can be customised, defaulting to DefaultConfDir
51+
ConfDir = DefaultConfDir
52+
53+
// Network is the network name that CNI should use (eg, "kindnet").
54+
// Currently, only crio (and podman) can use it, so that setting custom ConfDir is not necessary.
55+
// ref: https://github.com/cri-o/cri-o/issues/2121 (and https://github.com/containers/podman/issues/2370)
56+
// ref: https://github.com/cri-o/cri-o/blob/master/docs/crio.conf.5.md#crionetwork-table
57+
Network = ""
4658
)
4759

4860
// Runner is the subset of command.Runner this package consumes
@@ -72,38 +84,40 @@ type tmplInput struct {
7284
}
7385

7486
// New returns a new CNI manager
75-
func New(cc config.ClusterConfig) (Manager, error) {
87+
func New(cc *config.ClusterConfig) (Manager, error) {
7688
if cc.KubernetesConfig.NetworkPlugin != "" && cc.KubernetesConfig.NetworkPlugin != "cni" {
7789
klog.Infof("network plugin configured as %q, returning disabled", cc.KubernetesConfig.NetworkPlugin)
7890
return Disabled{}, nil
7991
}
8092

8193
klog.Infof("Creating CNI manager for %q", cc.KubernetesConfig.CNI)
8294

83-
// respect user-specified custom CNI Config Directory, if any
84-
userCNIConfDir := cc.KubernetesConfig.ExtraOptions.Get("cni-conf-dir", "kubelet")
85-
if userCNIConfDir != "" {
86-
CustomCNIConfDir = userCNIConfDir
87-
}
88-
95+
var cnm Manager
96+
var err error
8997
switch cc.KubernetesConfig.CNI {
9098
case "", "auto":
91-
return chooseDefault(cc), nil
99+
cnm = chooseDefault(*cc)
92100
case "false":
93-
return Disabled{cc: cc}, nil
101+
cnm = Disabled{cc: *cc}
94102
case "kindnet", "true":
95-
return KindNet{cc: cc}, nil
103+
cnm = KindNet{cc: *cc}
96104
case "bridge":
97-
return Bridge{cc: cc}, nil
105+
cnm = Bridge{cc: *cc}
98106
case "calico":
99-
return Calico{cc: cc}, nil
107+
cnm = Calico{cc: *cc}
100108
case "cilium":
101-
return Cilium{cc: cc}, nil
109+
cnm = Cilium{cc: *cc}
102110
case "flannel":
103-
return Flannel{cc: cc}, nil
111+
cnm = Flannel{cc: *cc}
104112
default:
105-
return NewCustom(cc, cc.KubernetesConfig.CNI)
113+
cnm, err = NewCustom(*cc, cc.KubernetesConfig.CNI)
106114
}
115+
116+
if err := configureCNI(cc, cnm); err != nil {
117+
klog.Errorf("unable to set CNI Config Directory: %v", err)
118+
}
119+
120+
return cnm, err
107121
}
108122

109123
// IsDisabled checks if CNI is disabled
@@ -183,3 +197,33 @@ func applyManifest(cc config.ClusterConfig, r Runner, f assets.CopyableFile) err
183197

184198
return nil
185199
}
200+
201+
// configureCNI - to avoid conflicting CNI configs, it sets:
202+
// - for crio: 'cni_default_network' config param via cni.Network
203+
// - for containerd and docker: kubelet's '--cni-conf-dir' flag to custom CNI Config Directory path (same used also by CNI Deployment).
204+
// ref: https://github.com/kubernetes/minikube/issues/10984 and https://github.com/kubernetes/minikube/pull/11106
205+
// 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.
206+
// Note2: Cilium does not need workaround as they automatically restart pods after CNI is successfully deployed.
207+
func configureCNI(cc *config.ClusterConfig, cnm Manager) error {
208+
if _, kindnet := cnm.(KindNet); kindnet {
209+
// crio only needs CNI network name; hopefully others (containerd, docker and kubeadm/kubelet) will follow eventually
210+
if cc.KubernetesConfig.ContainerRuntime == constants.CRIO {
211+
Network = "kindnet"
212+
return nil
213+
}
214+
// for containerd and docker: auto-set custom CNI via kubelet's 'cni-conf-dir' param, if not user-specified
215+
eo := fmt.Sprintf("kubelet.cni-conf-dir=%s", CustomConfDir)
216+
if !cc.KubernetesConfig.ExtraOptions.Exists(eo) {
217+
klog.Infof("auto-setting extra-config to %q", eo)
218+
if err := cc.KubernetesConfig.ExtraOptions.Set(eo); err != nil {
219+
return fmt.Errorf("failed auto-setting extra-config %q: %v", eo, err)
220+
}
221+
ConfDir = CustomConfDir
222+
klog.Infof("extra-config set to %q", eo)
223+
} else {
224+
// respect user-specified custom CNI Config Directory
225+
ConfDir = cc.KubernetesConfig.ExtraOptions.Get("cni-conf-dir", "kubelet")
226+
}
227+
}
228+
return nil
229+
}

Diff for: ‎pkg/minikube/cni/kindnet.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func (c KindNet) manifest() (assets.CopyableFile, error) {
166166
DefaultRoute: "0.0.0.0/0", // assumes IPv4
167167
PodCIDR: DefaultPodCIDR,
168168
ImageName: images.KindNet(c.cc.KubernetesConfig.ImageRepository),
169-
CNIConfDir: CustomCNIConfDir,
169+
CNIConfDir: ConfDir,
170170
}
171171

172172
b := bytes.Buffer{}

Diff for: ‎pkg/minikube/cruntime/containerd.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"k8s.io/klog/v2"
3535
"k8s.io/minikube/pkg/minikube/assets"
3636
"k8s.io/minikube/pkg/minikube/bootstrapper/images"
37+
"k8s.io/minikube/pkg/minikube/cni"
3738
"k8s.io/minikube/pkg/minikube/command"
3839
"k8s.io/minikube/pkg/minikube/config"
3940
"k8s.io/minikube/pkg/minikube/download"
@@ -94,7 +95,7 @@ oom_score = 0
9495
runtime_root = ""
9596
[plugins.cri.cni]
9697
bin_dir = "/opt/cni/bin"
97-
conf_dir = "/etc/cni/net.d"
98+
conf_dir = "{{.CNIConfDir}}"
9899
conf_template = ""
99100
[plugins.cri.registry]
100101
[plugins.cri.registry.mirrors]
@@ -190,10 +191,12 @@ func generateContainerdConfig(cr CommandRunner, imageRepository string, kv semve
190191
PodInfraContainerImage string
191192
SystemdCgroup bool
192193
InsecureRegistry []string
194+
CNIConfDir string
193195
}{
194196
PodInfraContainerImage: pauseImage,
195197
SystemdCgroup: forceSystemd,
196198
InsecureRegistry: insecureRegistry,
199+
CNIConfDir: cni.ConfDir,
197200
}
198201
var b bytes.Buffer
199202
if err := t.Execute(&b, opts); err != nil {

Diff for: ‎pkg/minikube/cruntime/crio.go

+9-28
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package cruntime
1919
import (
2020
"encoding/json"
2121
"fmt"
22-
"net"
2322
"os"
2423
"os/exec"
2524
"path"
@@ -31,6 +30,7 @@ import (
3130
"k8s.io/klog/v2"
3231
"k8s.io/minikube/pkg/minikube/assets"
3332
"k8s.io/minikube/pkg/minikube/bootstrapper/images"
33+
"k8s.io/minikube/pkg/minikube/cni"
3434
"k8s.io/minikube/pkg/minikube/command"
3535
"k8s.io/minikube/pkg/minikube/config"
3636
"k8s.io/minikube/pkg/minikube/download"
@@ -61,6 +61,14 @@ func generateCRIOConfig(cr CommandRunner, imageRepository string, kv semver.Vers
6161
if _, err := cr.RunCmd(c); err != nil {
6262
return errors.Wrap(err, "generateCRIOConfig.")
6363
}
64+
65+
if cni.Network != "" {
66+
klog.Infof("Updating CRIO to use the custom CNI network %q", cni.Network)
67+
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 {
68+
return errors.Wrap(err, "update network_dir")
69+
}
70+
}
71+
6472
return nil
6573
}
6674

@@ -405,30 +413,3 @@ func crioImagesPreloaded(runner command.Runner, images []string) bool {
405413
func (r *CRIO) ImagesPreloaded(images []string) bool {
406414
return crioImagesPreloaded(r.Runner, images)
407415
}
408-
409-
// UpdateCRIONet updates CRIO CNI network configuration and restarts it
410-
func UpdateCRIONet(r CommandRunner, cidr string) error {
411-
klog.Infof("Updating CRIO to use CIDR: %q", cidr)
412-
ip, net, err := net.ParseCIDR(cidr)
413-
if err != nil {
414-
return errors.Wrap(err, "parse cidr")
415-
}
416-
417-
oldNet := "10.88.0.0/16"
418-
oldGw := "10.88.0.1"
419-
420-
newNet := cidr
421-
422-
// Assume gateway is first IP in netmask (10.244.0.1, for instance)
423-
newGw := ip.Mask(net.Mask)
424-
newGw[3]++
425-
426-
// Update subnets used by 100-crio-bridge.conf & 87-podman-bridge.conflist
427-
// avoids: "Error adding network: failed to set bridge addr: could not add IP address to \"cni0\": permission denied"
428-
sed := fmt.Sprintf("sed -i -e s#%s#%s# -e s#%s#%s# /etc/cni/net.d/*bridge*", oldNet, newNet, oldGw, newGw)
429-
if _, err := r.RunCmd(exec.Command("sudo", "/bin/bash", "-c", sed)); err != nil {
430-
klog.Errorf("netconf update failed: %v", err)
431-
}
432-
433-
return sysinit.New(r).Restart("crio")
434-
}

Diff for: ‎pkg/minikube/node/start.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) {
188188
return nil, errors.Wrap(err, "joining cp")
189189
}
190190

191-
cnm, err := cni.New(*starter.Cfg)
191+
cnm, err := cni.New(starter.Cfg)
192192
if err != nil {
193193
return nil, errors.Wrap(err, "cni")
194194
}

0 commit comments

Comments
 (0)