From e4f8938c586a979834900bf1a4fbea51f791cfdc Mon Sep 17 00:00:00 2001 From: "Rostislav M. Georgiev" Date: Fri, 8 Jun 2018 15:01:35 +0300 Subject: [PATCH] kubeadm: Replace GetCoreImage with less error prone functions GetCoreImage is a too generic function, that takes too many arguments. This makes it prone to errors that may be difficult to trace. The solution is to split it into the following couple of functions with a more targeted interface: - GetKubeControlPlaneImage used to fetch Kubernetes control plane images or the unified control plane image (if one is specified). - GetEtcdImage is used to fetch the etcd image. In addition to these, a couple of new utility functions are also created: - GetKubeControlPlaneImageNoOverride used like GetKubeControlPlaneImage but does not return the unified control plane image (even if it is set). - GetGenericArchImage returns image path in the form of "prefix/image-goarch:tag" Signed-off-by: Rostislav M. Georgiev --- cmd/kubeadm/app/cmd/init.go | 8 +- cmd/kubeadm/app/images/images.go | 55 ++++--- cmd/kubeadm/app/images/images_test.go | 146 +++++++++++++++--- .../app/phases/controlplane/manifests.go | 6 +- cmd/kubeadm/app/phases/etcd/local.go | 2 +- cmd/kubeadm/app/phases/upgrade/prepull.go | 7 +- 6 files changed, 174 insertions(+), 50 deletions(-) diff --git a/cmd/kubeadm/app/cmd/init.go b/cmd/kubeadm/app/cmd/init.go index a72a0e015b4..c16dc6fd6ba 100644 --- a/cmd/kubeadm/app/cmd/init.go +++ b/cmd/kubeadm/app/cmd/init.go @@ -399,13 +399,13 @@ func (i *Init) Run(out io.Writer) error { if err := waitForKubeletAndFunc(waiter, waiter.WaitForAPI); err != nil { ctx := map[string]string{ "Error": fmt.Sprintf("%v", err), - "APIServerImage": images.GetCoreImage(kubeadmconstants.KubeAPIServer, i.cfg.GetControlPlaneImageRepository(), i.cfg.KubernetesVersion, i.cfg.UnifiedControlPlaneImage), - "ControllerManagerImage": images.GetCoreImage(kubeadmconstants.KubeControllerManager, i.cfg.GetControlPlaneImageRepository(), i.cfg.KubernetesVersion, i.cfg.UnifiedControlPlaneImage), - "SchedulerImage": images.GetCoreImage(kubeadmconstants.KubeScheduler, i.cfg.GetControlPlaneImageRepository(), i.cfg.KubernetesVersion, i.cfg.UnifiedControlPlaneImage), + "APIServerImage": images.GetKubeControlPlaneImage(kubeadmconstants.KubeAPIServer, i.cfg), + "ControllerManagerImage": images.GetKubeControlPlaneImage(kubeadmconstants.KubeControllerManager, i.cfg), + "SchedulerImage": images.GetKubeControlPlaneImage(kubeadmconstants.KubeScheduler, i.cfg), } // Set .EtcdImage conditionally if i.cfg.Etcd.Local != nil { - ctx["EtcdImage"] = fmt.Sprintf(" - %s", images.GetCoreImage(kubeadmconstants.Etcd, i.cfg.ImageRepository, i.cfg.KubernetesVersion, i.cfg.Etcd.Local.Image)) + ctx["EtcdImage"] = fmt.Sprintf(" - %s", images.GetEtcdImage(i.cfg)) } else { ctx["EtcdImage"] = "" } diff --git a/cmd/kubeadm/app/images/images.go b/cmd/kubeadm/app/images/images.go index f9260148c80..a84469c31c6 100644 --- a/cmd/kubeadm/app/images/images.go +++ b/cmd/kubeadm/app/images/images.go @@ -26,45 +26,58 @@ import ( kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" ) -// GetCoreImage generates and returns the image for the core Kubernetes components or returns overrideImage if specified -func GetCoreImage(image, repoPrefix, k8sVersion, overrideImage string) string { - if overrideImage != "" { - return overrideImage +// GetGenericArchImage generates and returns an image based on the current runtime arch +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.MasterConfiguration) 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.MasterConfiguration) string { + if cfg.UnifiedControlPlaneImage != "" { + return cfg.UnifiedControlPlaneImage + } + return GetKubeControlPlaneImageNoOverride(image, cfg) +} + +// GetEtcdImage generates and returns the image for etcd or returns cfg.Etcd.Local.Image if specified +func GetEtcdImage(cfg *kubeadmapi.MasterConfiguration) string { + if cfg.Etcd.Local != nil && cfg.Etcd.Local.Image != "" { + return cfg.Etcd.Local.Image } - kubernetesImageTag := kubeadmutil.KubernetesVersionToImageTag(k8sVersion) etcdImageTag := constants.DefaultEtcdVersion - etcdImageVersion, err := constants.EtcdSupportedVersion(k8sVersion) + etcdImageVersion, err := constants.EtcdSupportedVersion(cfg.KubernetesVersion) if err == nil { etcdImageTag = etcdImageVersion.String() } - return map[string]string{ - constants.Etcd: fmt.Sprintf("%s/%s-%s:%s", repoPrefix, "etcd", runtime.GOARCH, etcdImageTag), - constants.KubeAPIServer: fmt.Sprintf("%s/%s-%s:%s", repoPrefix, "kube-apiserver", runtime.GOARCH, kubernetesImageTag), - constants.KubeControllerManager: fmt.Sprintf("%s/%s-%s:%s", repoPrefix, "kube-controller-manager", runtime.GOARCH, kubernetesImageTag), - constants.KubeScheduler: fmt.Sprintf("%s/%s-%s:%s", repoPrefix, "kube-scheduler", runtime.GOARCH, kubernetesImageTag), - }[image] + return GetGenericArchImage(cfg.ImageRepository, constants.Etcd, etcdImageTag) } // GetAllImages returns a list of container images kubeadm expects to use on a control plane node func GetAllImages(cfg *kubeadmapi.MasterConfiguration) []string { - repoPrefix := cfg.GetControlPlaneImageRepository() imgs := []string{} - imgs = append(imgs, GetCoreImage(constants.KubeAPIServer, repoPrefix, cfg.KubernetesVersion, cfg.UnifiedControlPlaneImage)) - imgs = append(imgs, GetCoreImage(constants.KubeControllerManager, repoPrefix, cfg.KubernetesVersion, cfg.UnifiedControlPlaneImage)) - imgs = append(imgs, GetCoreImage(constants.KubeScheduler, repoPrefix, cfg.KubernetesVersion, cfg.UnifiedControlPlaneImage)) - imgs = append(imgs, fmt.Sprintf("%v/%v-%v:%v", repoPrefix, constants.KubeProxy, runtime.GOARCH, kubeadmutil.KubernetesVersionToImageTag(cfg.KubernetesVersion))) + 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)) // pause, etcd and kube-dns are not available on the ci image repository so use the default image repository. - imgs = append(imgs, fmt.Sprintf("%v/pause-%v:%v", cfg.ImageRepository, runtime.GOARCH, "3.1")) + imgs = append(imgs, GetGenericArchImage(cfg.ImageRepository, "pause", "3.1")) // if etcd is not external then add the image as it will be required if cfg.Etcd.Local != nil { - imgs = append(imgs, GetCoreImage(constants.Etcd, cfg.ImageRepository, cfg.KubernetesVersion, cfg.Etcd.Local.Image)) + imgs = append(imgs, GetEtcdImage(cfg)) } - dnsImage := fmt.Sprintf("%v/k8s-dns-kube-dns-%v:%v", cfg.ImageRepository, runtime.GOARCH, constants.KubeDNSVersion) + dnsImage := GetGenericArchImage(cfg.ImageRepository, "k8s-dns-kube-dns", constants.KubeDNSVersion) if features.Enabled(cfg.FeatureGates, features.CoreDNS) { - dnsImage = fmt.Sprintf("%v/coredns:%v", cfg.ImageRepository, constants.CoreDNSVersion) + dnsImage = GetGenericArchImage(cfg.ImageRepository, "coredns", constants.CoreDNSVersion) } imgs = append(imgs, dnsImage) return imgs diff --git a/cmd/kubeadm/app/images/images_test.go b/cmd/kubeadm/app/images/images_test.go index 38f237cc9d7..3da101d7d8d 100644 --- a/cmd/kubeadm/app/images/images_test.go +++ b/cmd/kubeadm/app/images/images_test.go @@ -32,43 +32,149 @@ const ( gcrPrefix = "k8s.gcr.io" ) -func TestGetCoreImage(t *testing.T) { +func TestGetGenericArchImage(t *testing.T) { + const ( + prefix = "foo" + image = "bar" + tag = "baz" + ) + expected := fmt.Sprintf("%s/%s-%s:%s", prefix, image, runtime.GOARCH, tag) + actual := GetGenericArchImage(prefix, image, tag) + if actual != expected { + t.Errorf("failed GetGenericArchImage:\n\texpected: %s\n\t actual: %s", expected, actual) + } +} + +func TestGetKubeControlPlaneImageNoOverride(t *testing.T) { var tests = []struct { - image, repo, version, override, expected string + image string + expected string + cfg *kubeadmapi.MasterConfiguration }{ { - override: "override", - expected: "override", - }, - { - image: constants.Etcd, - repo: gcrPrefix, - expected: fmt.Sprintf("%s/%s-%s:%s", gcrPrefix, "etcd", runtime.GOARCH, constants.DefaultEtcdVersion), + // UnifiedControlPlaneImage should be ignored by GetKubeImage + image: constants.KubeAPIServer, + expected: GetGenericArchImage(gcrPrefix, "kube-apiserver", expected), + cfg: &kubeadmapi.MasterConfiguration{ + UnifiedControlPlaneImage: "nooverride", + ImageRepository: gcrPrefix, + KubernetesVersion: testversion, + }, }, { image: constants.KubeAPIServer, - repo: gcrPrefix, - version: testversion, - expected: fmt.Sprintf("%s/%s-%s:%s", gcrPrefix, "kube-apiserver", runtime.GOARCH, expected), + expected: GetGenericArchImage(gcrPrefix, "kube-apiserver", expected), + cfg: &kubeadmapi.MasterConfiguration{ + ImageRepository: gcrPrefix, + KubernetesVersion: testversion, + }, }, { image: constants.KubeControllerManager, - repo: gcrPrefix, - version: testversion, - expected: fmt.Sprintf("%s/%s-%s:%s", gcrPrefix, "kube-controller-manager", runtime.GOARCH, expected), + expected: GetGenericArchImage(gcrPrefix, "kube-controller-manager", expected), + cfg: &kubeadmapi.MasterConfiguration{ + ImageRepository: gcrPrefix, + KubernetesVersion: testversion, + }, }, { image: constants.KubeScheduler, - repo: gcrPrefix, - version: testversion, - expected: fmt.Sprintf("%s/%s-%s:%s", gcrPrefix, "kube-scheduler", runtime.GOARCH, expected), + expected: GetGenericArchImage(gcrPrefix, "kube-scheduler", expected), + cfg: &kubeadmapi.MasterConfiguration{ + ImageRepository: gcrPrefix, + KubernetesVersion: testversion, + }, }, } for _, rt := range tests { - actual := GetCoreImage(rt.image, rt.repo, rt.version, rt.override) + actual := GetKubeControlPlaneImageNoOverride(rt.image, rt.cfg) if actual != rt.expected { t.Errorf( - "failed GetCoreImage:\n\texpected: %s\n\t actual: %s", + "failed GetKubeControlPlaneImageNoOverride:\n\texpected: %s\n\t actual: %s", + rt.expected, + actual, + ) + } + } +} + +func TestGetKubeControlPlaneImage(t *testing.T) { + var tests = []struct { + image string + expected string + cfg *kubeadmapi.MasterConfiguration + }{ + { + expected: "override", + cfg: &kubeadmapi.MasterConfiguration{ + UnifiedControlPlaneImage: "override", + }, + }, + { + image: constants.KubeAPIServer, + expected: GetGenericArchImage(gcrPrefix, "kube-apiserver", expected), + cfg: &kubeadmapi.MasterConfiguration{ + ImageRepository: gcrPrefix, + KubernetesVersion: testversion, + }, + }, + { + image: constants.KubeControllerManager, + expected: GetGenericArchImage(gcrPrefix, "kube-controller-manager", expected), + cfg: &kubeadmapi.MasterConfiguration{ + ImageRepository: gcrPrefix, + KubernetesVersion: testversion, + }, + }, + { + image: constants.KubeScheduler, + expected: GetGenericArchImage(gcrPrefix, "kube-scheduler", expected), + cfg: &kubeadmapi.MasterConfiguration{ + ImageRepository: gcrPrefix, + KubernetesVersion: testversion, + }, + }, + } + for _, rt := range tests { + actual := GetKubeControlPlaneImage(rt.image, rt.cfg) + if actual != rt.expected { + t.Errorf( + "failed GetKubeControlPlaneImage:\n\texpected: %s\n\t actual: %s", + rt.expected, + actual, + ) + } + } +} + +func TestGetEtcdImage(t *testing.T) { + var tests = []struct { + expected string + cfg *kubeadmapi.MasterConfiguration + }{ + { + expected: "override", + cfg: &kubeadmapi.MasterConfiguration{ + Etcd: kubeadmapi.Etcd{ + Local: &kubeadmapi.LocalEtcd{ + Image: "override", + }, + }, + }, + }, + { + expected: GetGenericArchImage(gcrPrefix, "etcd", constants.DefaultEtcdVersion), + cfg: &kubeadmapi.MasterConfiguration{ + ImageRepository: gcrPrefix, + KubernetesVersion: testversion, + }, + }, + } + for _, rt := range tests { + actual := GetEtcdImage(rt.cfg) + if actual != rt.expected { + t.Errorf( + "failed GetEtcdImage:\n\texpected: %s\n\t actual: %s", rt.expected, actual, ) diff --git a/cmd/kubeadm/app/phases/controlplane/manifests.go b/cmd/kubeadm/app/phases/controlplane/manifests.go index 1295ab5df78..6283a796eff 100644 --- a/cmd/kubeadm/app/phases/controlplane/manifests.go +++ b/cmd/kubeadm/app/phases/controlplane/manifests.go @@ -73,7 +73,7 @@ func GetStaticPodSpecs(cfg *kubeadmapi.MasterConfiguration, k8sVersion *version. staticPodSpecs := map[string]v1.Pod{ kubeadmconstants.KubeAPIServer: staticpodutil.ComponentPod(v1.Container{ Name: kubeadmconstants.KubeAPIServer, - Image: images.GetCoreImage(kubeadmconstants.KubeAPIServer, cfg.GetControlPlaneImageRepository(), cfg.KubernetesVersion, cfg.UnifiedControlPlaneImage), + Image: images.GetKubeControlPlaneImage(kubeadmconstants.KubeAPIServer, cfg), ImagePullPolicy: v1.PullIfNotPresent, Command: getAPIServerCommand(cfg), VolumeMounts: staticpodutil.VolumeMountMapToSlice(mounts.GetVolumeMounts(kubeadmconstants.KubeAPIServer)), @@ -83,7 +83,7 @@ func GetStaticPodSpecs(cfg *kubeadmapi.MasterConfiguration, k8sVersion *version. }, mounts.GetVolumes(kubeadmconstants.KubeAPIServer)), kubeadmconstants.KubeControllerManager: staticpodutil.ComponentPod(v1.Container{ Name: kubeadmconstants.KubeControllerManager, - Image: images.GetCoreImage(kubeadmconstants.KubeControllerManager, cfg.GetControlPlaneImageRepository(), cfg.KubernetesVersion, cfg.UnifiedControlPlaneImage), + Image: images.GetKubeControlPlaneImage(kubeadmconstants.KubeControllerManager, cfg), ImagePullPolicy: v1.PullIfNotPresent, Command: getControllerManagerCommand(cfg, k8sVersion), VolumeMounts: staticpodutil.VolumeMountMapToSlice(mounts.GetVolumeMounts(kubeadmconstants.KubeControllerManager)), @@ -93,7 +93,7 @@ func GetStaticPodSpecs(cfg *kubeadmapi.MasterConfiguration, k8sVersion *version. }, mounts.GetVolumes(kubeadmconstants.KubeControllerManager)), kubeadmconstants.KubeScheduler: staticpodutil.ComponentPod(v1.Container{ Name: kubeadmconstants.KubeScheduler, - Image: images.GetCoreImage(kubeadmconstants.KubeScheduler, cfg.GetControlPlaneImageRepository(), cfg.KubernetesVersion, cfg.UnifiedControlPlaneImage), + Image: images.GetKubeControlPlaneImage(kubeadmconstants.KubeScheduler, cfg), ImagePullPolicy: v1.PullIfNotPresent, Command: getSchedulerCommand(cfg), VolumeMounts: staticpodutil.VolumeMountMapToSlice(mounts.GetVolumeMounts(kubeadmconstants.KubeScheduler)), diff --git a/cmd/kubeadm/app/phases/etcd/local.go b/cmd/kubeadm/app/phases/etcd/local.go index 5240d335748..8183f351055 100644 --- a/cmd/kubeadm/app/phases/etcd/local.go +++ b/cmd/kubeadm/app/phases/etcd/local.go @@ -60,7 +60,7 @@ func GetEtcdPodSpec(cfg *kubeadmapi.MasterConfiguration) v1.Pod { return staticpodutil.ComponentPod(v1.Container{ Name: kubeadmconstants.Etcd, Command: getEtcdCommand(cfg), - Image: images.GetCoreImage(kubeadmconstants.Etcd, cfg.ImageRepository, cfg.KubernetesVersion, cfg.Etcd.Local.Image), + Image: images.GetEtcdImage(cfg), ImagePullPolicy: v1.PullIfNotPresent, // Mount the etcd datadir path read-write so etcd can store data in a more persistent manner VolumeMounts: []v1.VolumeMount{ diff --git a/cmd/kubeadm/app/phases/upgrade/prepull.go b/cmd/kubeadm/app/phases/upgrade/prepull.go index 848c5f58076..f875cd5eae3 100644 --- a/cmd/kubeadm/app/phases/upgrade/prepull.go +++ b/cmd/kubeadm/app/phases/upgrade/prepull.go @@ -59,7 +59,12 @@ func NewDaemonSetPrepuller(client clientset.Interface, waiter apiclient.Waiter, // CreateFunc creates a DaemonSet for making the image available on every relevant node func (d *DaemonSetPrepuller) CreateFunc(component string) error { - image := images.GetCoreImage(component, d.cfg.GetControlPlaneImageRepository(), d.cfg.KubernetesVersion, d.cfg.UnifiedControlPlaneImage) + var image string + if component == constants.Etcd { + image = images.GetEtcdImage(d.cfg) + } else { + image = images.GetKubeControlPlaneImage(component, d.cfg) + } ds := buildPrePullDaemonSet(component, image) // Create the DaemonSet in the API Server