From dedf1392889e6966f61f55d47f52495ec58409af Mon Sep 17 00:00:00 2001 From: "Rostislav M. Georgiev" Date: Wed, 8 Aug 2018 12:27:50 +0300 Subject: [PATCH] kubeadm: Deduplicate kube-proxy image logic Until now, kube-proxy image was handled in two separate places: - In images.go along with the pre-pull code and without having the image override capabilities (via UnifiedControlPlaneImage) - In the kube-proxy manifest, where image override was possible. This duplicates the kube-proxy image logic and makes it prone to errors. Therefore, this change aims to deduplicate it and make it more straightforward. This is achieved in the following ways: - GetKubeControlPlaneImage is used for kube-proxy image fetching, thus allowing for the image to be overriden by UnifiedControlPlaneImage. - Remove duplicated logic from the manifest and use GetKubeControlPlaneImage to generate the image for the manifest. Additionally, GetKubeControlPlaneImageNoOverride is removed as the only use case for the function is now invalid. Signed-off-by: Rostislav M. Georgiev --- cmd/kubeadm/app/images/images.go | 13 ++--- cmd/kubeadm/app/images/images_test.go | 53 ------------------- cmd/kubeadm/app/phases/addons/proxy/BUILD | 2 + .../app/phases/addons/proxy/manifests.go | 2 +- cmd/kubeadm/app/phases/addons/proxy/proxy.go | 10 ++-- .../app/phases/addons/proxy/proxy_test.go | 10 ++-- 6 files changed, 15 insertions(+), 75 deletions(-) diff --git a/cmd/kubeadm/app/images/images.go b/cmd/kubeadm/app/images/images.go index 416f2054381..6bea526ad23 100644 --- a/cmd/kubeadm/app/images/images.go +++ b/cmd/kubeadm/app/images/images.go @@ -36,19 +36,14 @@ func GetGenericArchImage(prefix, image, tag string) string { return fmt.Sprintf("%s/%s-%s:%s", prefix, image, runtime.GOARCH, tag) } -// GetKubeControlPlaneImageNoOverride generates and returns the image for the core Kubernetes components ignoring the unified control plane image -func GetKubeControlPlaneImageNoOverride(image string, cfg *kubeadmapi.InitConfiguration) string { - repoPrefix := cfg.GetControlPlaneImageRepository() - kubernetesImageTag := kubeadmutil.KubernetesVersionToImageTag(cfg.KubernetesVersion) - return GetGenericArchImage(repoPrefix, image, kubernetesImageTag) -} - // GetKubeControlPlaneImage generates and returns the image for the core Kubernetes components or returns the unified control plane image if specified func GetKubeControlPlaneImage(image string, cfg *kubeadmapi.InitConfiguration) string { if cfg.UnifiedControlPlaneImage != "" { return cfg.UnifiedControlPlaneImage } - return GetKubeControlPlaneImageNoOverride(image, cfg) + repoPrefix := cfg.GetControlPlaneImageRepository() + kubernetesImageTag := kubeadmutil.KubernetesVersionToImageTag(cfg.KubernetesVersion) + return GetGenericArchImage(repoPrefix, image, kubernetesImageTag) } // GetEtcdImage generates and returns the image for etcd or returns cfg.Etcd.Local.Image if specified @@ -70,7 +65,7 @@ func GetAllImages(cfg *kubeadmapi.InitConfiguration) []string { imgs = append(imgs, GetKubeControlPlaneImage(constants.KubeAPIServer, cfg)) imgs = append(imgs, GetKubeControlPlaneImage(constants.KubeControllerManager, cfg)) imgs = append(imgs, GetKubeControlPlaneImage(constants.KubeScheduler, cfg)) - imgs = append(imgs, GetKubeControlPlaneImageNoOverride(constants.KubeProxy, cfg)) + imgs = append(imgs, GetKubeControlPlaneImage(constants.KubeProxy, cfg)) // pause, etcd and kube-dns are not available on the ci image repository so use the default image repository. imgs = append(imgs, GetGenericImage(cfg.ImageRepository, "pause", "3.1")) diff --git a/cmd/kubeadm/app/images/images_test.go b/cmd/kubeadm/app/images/images_test.go index b0fe41e866a..3475629d5e2 100644 --- a/cmd/kubeadm/app/images/images_test.go +++ b/cmd/kubeadm/app/images/images_test.go @@ -45,59 +45,6 @@ func TestGetGenericArchImage(t *testing.T) { } } -func TestGetKubeControlPlaneImageNoOverride(t *testing.T) { - var tests = []struct { - image string - expected string - cfg *kubeadmapi.InitConfiguration - }{ - { - // UnifiedControlPlaneImage should be ignored by GetKubeImage - image: constants.KubeAPIServer, - expected: GetGenericArchImage(gcrPrefix, "kube-apiserver", expected), - cfg: &kubeadmapi.InitConfiguration{ - UnifiedControlPlaneImage: "nooverride", - ImageRepository: gcrPrefix, - KubernetesVersion: testversion, - }, - }, - { - image: constants.KubeAPIServer, - expected: GetGenericArchImage(gcrPrefix, "kube-apiserver", expected), - cfg: &kubeadmapi.InitConfiguration{ - ImageRepository: gcrPrefix, - KubernetesVersion: testversion, - }, - }, - { - image: constants.KubeControllerManager, - expected: GetGenericArchImage(gcrPrefix, "kube-controller-manager", expected), - cfg: &kubeadmapi.InitConfiguration{ - ImageRepository: gcrPrefix, - KubernetesVersion: testversion, - }, - }, - { - image: constants.KubeScheduler, - expected: GetGenericArchImage(gcrPrefix, "kube-scheduler", expected), - cfg: &kubeadmapi.InitConfiguration{ - ImageRepository: gcrPrefix, - KubernetesVersion: testversion, - }, - }, - } - for _, rt := range tests { - actual := GetKubeControlPlaneImageNoOverride(rt.image, rt.cfg) - if actual != rt.expected { - t.Errorf( - "failed GetKubeControlPlaneImageNoOverride:\n\texpected: %s\n\t actual: %s", - rt.expected, - actual, - ) - } - } -} - func TestGetKubeControlPlaneImage(t *testing.T) { var tests = []struct { image string diff --git a/cmd/kubeadm/app/phases/addons/proxy/BUILD b/cmd/kubeadm/app/phases/addons/proxy/BUILD index a2f903507f6..4fca0d4a37d 100644 --- a/cmd/kubeadm/app/phases/addons/proxy/BUILD +++ b/cmd/kubeadm/app/phases/addons/proxy/BUILD @@ -35,6 +35,8 @@ go_library( deps = [ "//cmd/kubeadm/app/apis/kubeadm:go_default_library", "//cmd/kubeadm/app/componentconfigs:go_default_library", + "//cmd/kubeadm/app/constants:go_default_library", + "//cmd/kubeadm/app/images:go_default_library", "//cmd/kubeadm/app/util:go_default_library", "//cmd/kubeadm/app/util/apiclient:go_default_library", "//staging/src/k8s.io/api/apps/v1:go_default_library", diff --git a/cmd/kubeadm/app/phases/addons/proxy/manifests.go b/cmd/kubeadm/app/phases/addons/proxy/manifests.go index 77caea96f8a..50986755d3a 100644 --- a/cmd/kubeadm/app/phases/addons/proxy/manifests.go +++ b/cmd/kubeadm/app/phases/addons/proxy/manifests.go @@ -75,7 +75,7 @@ spec: priorityClassName: system-node-critical containers: - name: kube-proxy - image: {{ if .ImageOverride }}{{ .ImageOverride }}{{ else }}{{ .ImageRepository }}/kube-proxy-{{ .Arch }}:{{ .Version }}{{ end }} + image: {{ .Image }} imagePullPolicy: IfNotPresent command: - /usr/local/bin/kube-proxy diff --git a/cmd/kubeadm/app/phases/addons/proxy/proxy.go b/cmd/kubeadm/app/phases/addons/proxy/proxy.go index 931ed486a69..f8f70f18937 100644 --- a/cmd/kubeadm/app/phases/addons/proxy/proxy.go +++ b/cmd/kubeadm/app/phases/addons/proxy/proxy.go @@ -30,6 +30,8 @@ import ( clientsetscheme "k8s.io/client-go/kubernetes/scheme" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" "k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs" + "k8s.io/kubernetes/cmd/kubeadm/app/constants" + "k8s.io/kubernetes/cmd/kubeadm/app/images" kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" "k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient" ) @@ -73,11 +75,9 @@ func EnsureProxyAddon(cfg *kubeadmapi.InitConfiguration, client clientset.Interf if err != nil { return fmt.Errorf("error when parsing kube-proxy configmap template: %v", err) } - proxyDaemonSetBytes, err = kubeadmutil.ParseTemplate(KubeProxyDaemonSet19, struct{ ImageRepository, Arch, Version, ImageOverride string }{ - ImageRepository: cfg.GetControlPlaneImageRepository(), - Arch: runtime.GOARCH, - Version: kubeadmutil.KubernetesVersionToImageTag(cfg.KubernetesVersion), - ImageOverride: cfg.UnifiedControlPlaneImage, + proxyDaemonSetBytes, err = kubeadmutil.ParseTemplate(KubeProxyDaemonSet19, struct{ Image, Arch string }{ + Image: images.GetKubeControlPlaneImage(constants.KubeProxy, cfg), + Arch: runtime.GOARCH, }) if err != nil { return fmt.Errorf("error when parsing kube-proxy daemonset template: %v", err) diff --git a/cmd/kubeadm/app/phases/addons/proxy/proxy_test.go b/cmd/kubeadm/app/phases/addons/proxy/proxy_test.go index f4be8e798c2..17d307f5c6d 100644 --- a/cmd/kubeadm/app/phases/addons/proxy/proxy_test.go +++ b/cmd/kubeadm/app/phases/addons/proxy/proxy_test.go @@ -108,13 +108,9 @@ func TestCompileManifests(t *testing.T) { }, { manifest: KubeProxyDaemonSet19, - data: struct{ ImageRepository, Arch, Version, ImageOverride, MasterTaintKey, CloudTaintKey string }{ - ImageRepository: "foo", - Arch: "foo", - Version: "foo", - ImageOverride: "foo", - MasterTaintKey: "foo", - CloudTaintKey: "foo", + data: struct{ Image, Arch string }{ + Image: "foo", + Arch: "foo", }, expected: true, },