Merge pull request #67131 from rosti/kube-proxy-manifest-fix2

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: Deduplicate kube-proxy image logic

**What this PR does / why we need it**:

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.

This PR also removes GetKubeControlPlaneImageNoOverride as it's no longer needed.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
None

**Special notes for your reviewer**:

/cc @kubernetes/sig-cluster-lifecycle-pr-reviews
/area kubeadm
/assign @luxas
/assign @timothysc

**Release note**:

```release-note
kubeadm: make sure pre-pulled kube-proxy image and the one specified in its daemon set manifest are the same
```

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"))

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

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",

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

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)

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,
 		},