From c1e2751ea6ac800edfef61dc5b8d1008f407bcdf Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 24 Jun 2020 11:42:52 -0700 Subject: [PATCH 1/5] Allow passing in extra args to etcd --- .../bootstrapper/bsutil/extraconfig.go | 3 ++- .../bootstrapper/bsutil/ktmpl/v1beta2.go | 4 ++++ pkg/minikube/bootstrapper/bsutil/kubeadm.go | 21 ++++++++++++++++++- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/pkg/minikube/bootstrapper/bsutil/extraconfig.go b/pkg/minikube/bootstrapper/bsutil/extraconfig.go index 5033ad34b973..63d47754027f 100644 --- a/pkg/minikube/bootstrapper/bsutil/extraconfig.go +++ b/pkg/minikube/bootstrapper/bsutil/extraconfig.go @@ -49,6 +49,7 @@ var componentToKubeadmConfigKey = map[string]string{ Apiserver: "apiServer", ControllerManager: "controllerManager", Scheduler: "scheduler", + Etcd: "etcd", Kubeadm: "kubeadm", // The KubeProxy is handled in different config block Kubeproxy: "", @@ -191,7 +192,7 @@ func createExtraComponentConfig(extraOptions config.ExtraOptionSlice, version se } for i, extraArgs := range extraArgsSlice { - if extraArgs.Component == Kubeadm { + if extraArgs.Component == Kubeadm || extraArgs.Component == Etcd { extraArgsSlice = append(extraArgsSlice[:i], extraArgsSlice[i+1:]...) break } diff --git a/pkg/minikube/bootstrapper/bsutil/ktmpl/v1beta2.go b/pkg/minikube/bootstrapper/bsutil/ktmpl/v1beta2.go index a4507c0654a9..66571c1ed994 100644 --- a/pkg/minikube/bootstrapper/bsutil/ktmpl/v1beta2.go +++ b/pkg/minikube/bootstrapper/bsutil/ktmpl/v1beta2.go @@ -63,6 +63,10 @@ dns: etcd: local: dataDir: {{.EtcdDataDir}} + extraArgs: +{{- range $i, $val := printMapInOrder .EtcdExtraArgs ": " }} + {{$val}} +{{- end}} controllerManager: extraArgs: "leader-elect": "false" diff --git a/pkg/minikube/bootstrapper/bsutil/kubeadm.go b/pkg/minikube/bootstrapper/bsutil/kubeadm.go index 155cd1d494ac..e2b721230973 100644 --- a/pkg/minikube/bootstrapper/bsutil/kubeadm.go +++ b/pkg/minikube/bootstrapper/bsutil/kubeadm.go @@ -43,12 +43,14 @@ func GenerateKubeadmYAML(cc config.ClusterConfig, n config.Node, r cruntime.Mana if err != nil { return nil, errors.Wrap(err, "parsing Kubernetes version") } + fmt.Println(k8s.ExtraOptions) // parses a map of the feature gates for kubeadm and component kubeadmFeatureArgs, componentFeatureArgs, err := parseFeatureArgs(k8s.FeatureGates) if err != nil { return nil, errors.Wrap(err, "parses feature gate config for kubeadm and component") } + fmt.Println(k8s.ExtraOptions) // In case of no port assigned, use default cp, err := config.PrimaryControlPlane(&cc) @@ -60,10 +62,12 @@ func GenerateKubeadmYAML(cc config.ClusterConfig, n config.Node, r cruntime.Mana nodePort = constants.APIServerPort } + fmt.Println(k8s.ExtraOptions) componentOpts, err := createExtraComponentConfig(k8s.ExtraOptions, version, componentFeatureArgs, cp) if err != nil { return nil, errors.Wrap(err, "generating extra component config for kubeadm") } + fmt.Println(k8s.ExtraOptions) opts := struct { CertDir string @@ -73,6 +77,7 @@ func GenerateKubeadmYAML(cc config.ClusterConfig, n config.Node, r cruntime.Mana APIServerPort int KubernetesVersion string EtcdDataDir string + EtcdExtraArgs map[string]string ClusterName string NodeName string DNSDomain string @@ -92,6 +97,7 @@ func GenerateKubeadmYAML(cc config.ClusterConfig, n config.Node, r cruntime.Mana APIServerPort: nodePort, KubernetesVersion: k8s.KubernetesVersion, EtcdDataDir: EtcdDataDir(), + EtcdExtraArgs: etcdExtraArgs(k8s.ExtraOptions), ClusterName: cc.Name, //kubeadm uses NodeName as the --hostname-override parameter, so this needs to be the name of the machine NodeName: KubeNodeName(cc, n), @@ -125,7 +131,7 @@ func GenerateKubeadmYAML(cc config.ClusterConfig, n config.Node, r cruntime.Mana if err := configTmpl.Execute(&b, opts); err != nil { return nil, err } - glog.Infof("kubeadm config:\n%s\n", b.String()) + fmt.Printf("kubeadm config:\n%s\n", b.String()) return b.Bytes(), nil } @@ -138,6 +144,7 @@ const ( Scheduler = "scheduler" ControllerManager = "controller-manager" Kubeproxy = "kube-proxy" + Etcd = "etcd" ) // InvokeKubeadm returns the invocation command for Kubeadm @@ -149,3 +156,15 @@ func InvokeKubeadm(version string) string { func EtcdDataDir() string { return path.Join(vmpath.GuestPersistentDir, "etcd") } + +func etcdExtraArgs(extraOpts config.ExtraOptionSlice) map[string]string { + args := map[string]string{} + for _, eo := range extraOpts { + if eo.Component != Etcd { + continue + } + args[eo.Key] = eo.Value + } + fmt.Println("etcd extra args:", args) + return args +} From b32487adb1fbc1a8932ed6e9fa494b4e8abf9bbd Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 24 Jun 2020 15:54:19 -0700 Subject: [PATCH 2/5] Only include etcd extra args if they exist --- .../bootstrapper/bsutil/ktmpl/v1beta2.go | 4 +- pkg/minikube/bootstrapper/bsutil/kubeadm.go | 7 +-- .../bootstrapper/bsutil/kubeadm_test.go | 49 +++++++++++++++++++ 3 files changed, 53 insertions(+), 7 deletions(-) diff --git a/pkg/minikube/bootstrapper/bsutil/ktmpl/v1beta2.go b/pkg/minikube/bootstrapper/bsutil/ktmpl/v1beta2.go index 66571c1ed994..af5d3c6e225c 100644 --- a/pkg/minikube/bootstrapper/bsutil/ktmpl/v1beta2.go +++ b/pkg/minikube/bootstrapper/bsutil/ktmpl/v1beta2.go @@ -63,10 +63,12 @@ dns: etcd: local: dataDir: {{.EtcdDataDir}} +{{- if .EtcdExtraArgs}} extraArgs: {{- range $i, $val := printMapInOrder .EtcdExtraArgs ": " }} {{$val}} -{{- end}} +{{- end}} +{{- end }} controllerManager: extraArgs: "leader-elect": "false" diff --git a/pkg/minikube/bootstrapper/bsutil/kubeadm.go b/pkg/minikube/bootstrapper/bsutil/kubeadm.go index e2b721230973..580983711a88 100644 --- a/pkg/minikube/bootstrapper/bsutil/kubeadm.go +++ b/pkg/minikube/bootstrapper/bsutil/kubeadm.go @@ -43,14 +43,12 @@ func GenerateKubeadmYAML(cc config.ClusterConfig, n config.Node, r cruntime.Mana if err != nil { return nil, errors.Wrap(err, "parsing Kubernetes version") } - fmt.Println(k8s.ExtraOptions) // parses a map of the feature gates for kubeadm and component kubeadmFeatureArgs, componentFeatureArgs, err := parseFeatureArgs(k8s.FeatureGates) if err != nil { return nil, errors.Wrap(err, "parses feature gate config for kubeadm and component") } - fmt.Println(k8s.ExtraOptions) // In case of no port assigned, use default cp, err := config.PrimaryControlPlane(&cc) @@ -62,12 +60,10 @@ func GenerateKubeadmYAML(cc config.ClusterConfig, n config.Node, r cruntime.Mana nodePort = constants.APIServerPort } - fmt.Println(k8s.ExtraOptions) componentOpts, err := createExtraComponentConfig(k8s.ExtraOptions, version, componentFeatureArgs, cp) if err != nil { return nil, errors.Wrap(err, "generating extra component config for kubeadm") } - fmt.Println(k8s.ExtraOptions) opts := struct { CertDir string @@ -131,7 +127,7 @@ func GenerateKubeadmYAML(cc config.ClusterConfig, n config.Node, r cruntime.Mana if err := configTmpl.Execute(&b, opts); err != nil { return nil, err } - fmt.Printf("kubeadm config:\n%s\n", b.String()) + glog.Infof("kubeadm config:\n%s\n", b.String()) return b.Bytes(), nil } @@ -165,6 +161,5 @@ func etcdExtraArgs(extraOpts config.ExtraOptionSlice) map[string]string { } args[eo.Key] = eo.Value } - fmt.Println("etcd extra args:", args) return args } diff --git a/pkg/minikube/bootstrapper/bsutil/kubeadm_test.go b/pkg/minikube/bootstrapper/bsutil/kubeadm_test.go index ff61653a8323..ac1cd3938d4b 100644 --- a/pkg/minikube/bootstrapper/bsutil/kubeadm_test.go +++ b/pkg/minikube/bootstrapper/bsutil/kubeadm_test.go @@ -22,6 +22,7 @@ import ( "strings" "testing" + "github.com/google/go-cmp/cmp" "github.com/pmezard/go-difflib/difflib" "k8s.io/minikube/pkg/minikube/config" "k8s.io/minikube/pkg/minikube/constants" @@ -246,3 +247,51 @@ func TestGenerateKubeadmYAML(t *testing.T) { } } } + +func TestEtcdExtraArgs(t *testing.T) { + tcs := []struct { + description string + extraOpts config.ExtraOptionSlice + expected map[string]string + }{ + { + description: "includes an etcd option", + extraOpts: config.ExtraOptionSlice{ + config.ExtraOption{ + Component: "kubeadm", + Key: "key", + Value: "value", + }, config.ExtraOption{ + Component: "etcd", + Key: "key1", + Value: "value1", + }, + }, + expected: map[string]string{ + "key1": "value1", + }, + }, { + description: "no etcd option", + extraOpts: config.ExtraOptionSlice{ + config.ExtraOption{ + Component: "kubeadm", + Key: "key", + Value: "value", + }, config.ExtraOption{ + Component: "apiserver", + Key: "key1", + Value: "value1", + }, + }, + expected: map[string]string{}, + }, + } + for _, tc := range tcs { + t.Run(tc.description, func(t *testing.T) { + actual := etcdExtraArgs(tc.extraOpts) + if diff := cmp.Diff(tc.expected, actual); diff != "" { + t.Errorf("machines mismatch (-want +got):\n%s", diff) + } + }) + } +} From 57574e3b6d8d0d5f6e760ea61b934de9dc83639d Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 24 Jun 2020 16:14:55 -0700 Subject: [PATCH 3/5] fix space bug in unit test --- .../bootstrapper/bsutil/extraconfig.go | 2 + .../bootstrapper/bsutil/ktmpl/v1beta2.go | 2 +- .../bootstrapper/bsutil/kubeadm_test.go | 55 ++++--------------- 3 files changed, 14 insertions(+), 45 deletions(-) diff --git a/pkg/minikube/bootstrapper/bsutil/extraconfig.go b/pkg/minikube/bootstrapper/bsutil/extraconfig.go index 63d47754027f..9d2aeacbf451 100644 --- a/pkg/minikube/bootstrapper/bsutil/extraconfig.go +++ b/pkg/minikube/bootstrapper/bsutil/extraconfig.go @@ -184,6 +184,8 @@ func optionPairsForComponent(component string, version semver.Version, cp config // kubeadm extra args should not be included in the kubeadm config in the extra args section (instead, they must // be inserted explicitly in the appropriate places or supplied from the command line); here we remove all of the // kubeadm extra args from the slice +// etcd must also not be included in that section, as they must exist in the 'etcd' extra args section +// so, all etcd components are removed as well // createExtraComponentConfig generates a map of component to extra args for all of the components except kubeadm func createExtraComponentConfig(extraOptions config.ExtraOptionSlice, version semver.Version, componentFeatureArgs string, cp config.Node) ([]componentOptions, error) { extraArgsSlice, err := newComponentOptions(extraOptions, version, componentFeatureArgs, cp) diff --git a/pkg/minikube/bootstrapper/bsutil/ktmpl/v1beta2.go b/pkg/minikube/bootstrapper/bsutil/ktmpl/v1beta2.go index af5d3c6e225c..b5dc4f7e2721 100644 --- a/pkg/minikube/bootstrapper/bsutil/ktmpl/v1beta2.go +++ b/pkg/minikube/bootstrapper/bsutil/ktmpl/v1beta2.go @@ -68,7 +68,7 @@ etcd: {{- range $i, $val := printMapInOrder .EtcdExtraArgs ": " }} {{$val}} {{- end}} -{{- end }} +{{- end}} controllerManager: extraArgs: "leader-elect": "false" diff --git a/pkg/minikube/bootstrapper/bsutil/kubeadm_test.go b/pkg/minikube/bootstrapper/bsutil/kubeadm_test.go index ac1cd3938d4b..6362b45132a2 100644 --- a/pkg/minikube/bootstrapper/bsutil/kubeadm_test.go +++ b/pkg/minikube/bootstrapper/bsutil/kubeadm_test.go @@ -241,7 +241,7 @@ func TestGenerateKubeadmYAML(t *testing.T) { t.Fatalf("diff error: %v", err) } if diff != "" { - t.Errorf("unexpected diff:\n%s\n===== [RAW OUTPUT] =====\n%s", diff, got) + t.Errorf("unexpected diff:\n%s\n", diff) } }) } @@ -249,49 +249,16 @@ func TestGenerateKubeadmYAML(t *testing.T) { } func TestEtcdExtraArgs(t *testing.T) { - tcs := []struct { - description string - extraOpts config.ExtraOptionSlice - expected map[string]string - }{ - { - description: "includes an etcd option", - extraOpts: config.ExtraOptionSlice{ - config.ExtraOption{ - Component: "kubeadm", - Key: "key", - Value: "value", - }, config.ExtraOption{ - Component: "etcd", - Key: "key1", - Value: "value1", - }, - }, - expected: map[string]string{ - "key1": "value1", - }, - }, { - description: "no etcd option", - extraOpts: config.ExtraOptionSlice{ - config.ExtraOption{ - Component: "kubeadm", - Key: "key", - Value: "value", - }, config.ExtraOption{ - Component: "apiserver", - Key: "key1", - Value: "value1", - }, - }, - expected: map[string]string{}, - }, + expected := map[string]string{ + "key": "value", } - for _, tc := range tcs { - t.Run(tc.description, func(t *testing.T) { - actual := etcdExtraArgs(tc.extraOpts) - if diff := cmp.Diff(tc.expected, actual); diff != "" { - t.Errorf("machines mismatch (-want +got):\n%s", diff) - } - }) + extraOpts := append(getExtraOpts(), config.ExtraOption{ + Component: Etcd, + Key: "key", + Value: "value", + }) + actual := etcdExtraArgs(extraOpts) + if diff := cmp.Diff(expected, actual); diff != "" { + t.Errorf("machines mismatch (-want +got):\n%s", diff) } } From 9d9d512cd83bc07f0736d316ce2731facbca7002 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 24 Jun 2020 16:34:58 -0700 Subject: [PATCH 4/5] fix function so we don't get an index out of bounds error --- pkg/minikube/bootstrapper/bsutil/extraconfig.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/pkg/minikube/bootstrapper/bsutil/extraconfig.go b/pkg/minikube/bootstrapper/bsutil/extraconfig.go index 9d2aeacbf451..3f18f7abb288 100644 --- a/pkg/minikube/bootstrapper/bsutil/extraconfig.go +++ b/pkg/minikube/bootstrapper/bsutil/extraconfig.go @@ -184,22 +184,21 @@ func optionPairsForComponent(component string, version semver.Version, cp config // kubeadm extra args should not be included in the kubeadm config in the extra args section (instead, they must // be inserted explicitly in the appropriate places or supplied from the command line); here we remove all of the // kubeadm extra args from the slice -// etcd must also not be included in that section, as they must exist in the 'etcd' extra args section -// so, all etcd components are removed as well +// etcd must also not be included in that section, as those extra args exist in the `etcd` section // createExtraComponentConfig generates a map of component to extra args for all of the components except kubeadm func createExtraComponentConfig(extraOptions config.ExtraOptionSlice, version semver.Version, componentFeatureArgs string, cp config.Node) ([]componentOptions, error) { extraArgsSlice, err := newComponentOptions(extraOptions, version, componentFeatureArgs, cp) if err != nil { return nil, err } - - for i, extraArgs := range extraArgsSlice { + validComponenets := []componentOptions{} + for _, extraArgs := range extraArgsSlice { if extraArgs.Component == Kubeadm || extraArgs.Component == Etcd { - extraArgsSlice = append(extraArgsSlice[:i], extraArgsSlice[i+1:]...) - break + continue } + validComponenets = append(validComponenets, extraArgs) } - return extraArgsSlice, nil + return validComponenets, nil } // createKubeProxyOptions generates a map of extra config for kube-proxy From 1b0de27d3de0ae1785e71a163bdb75e87d33804a Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 24 Jun 2020 16:43:32 -0700 Subject: [PATCH 5/5] fix spelling --- pkg/minikube/bootstrapper/bsutil/extraconfig.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/minikube/bootstrapper/bsutil/extraconfig.go b/pkg/minikube/bootstrapper/bsutil/extraconfig.go index 3f18f7abb288..0f64b89f10db 100644 --- a/pkg/minikube/bootstrapper/bsutil/extraconfig.go +++ b/pkg/minikube/bootstrapper/bsutil/extraconfig.go @@ -191,14 +191,14 @@ func createExtraComponentConfig(extraOptions config.ExtraOptionSlice, version se if err != nil { return nil, err } - validComponenets := []componentOptions{} + validComponents := []componentOptions{} for _, extraArgs := range extraArgsSlice { if extraArgs.Component == Kubeadm || extraArgs.Component == Etcd { continue } - validComponenets = append(validComponenets, extraArgs) + validComponents = append(validComponents, extraArgs) } - return validComponenets, nil + return validComponents, nil } // createKubeProxyOptions generates a map of extra config for kube-proxy