From 773fa66d3c91bfe1c7648181732cf16e8ba06b7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Sat, 11 Mar 2023 09:35:32 +0100 Subject: [PATCH 1/5] Always use the new registry.k8s.io repository This will break air-gapped versions using old releases, since they will first try to contact the new registry. But for other installations, it should still work with older preloads after downloading new image manifests. --- cmd/minikube/cmd/start_flags.go | 7 ++ cmd/minikube/cmd/start_test.go | 44 +++++++++ .../bootstrapper/images/images_test.go | 72 +++++++------- .../bootstrapper/images/kubeadm_test.go | 98 ++++++++++--------- pkg/minikube/bootstrapper/images/repo.go | 10 +- pkg/minikube/bootstrapper/images/repo_test.go | 4 +- 6 files changed, 149 insertions(+), 86 deletions(-) diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index 12e22c982f24..c95bc1c2f254 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -32,6 +32,7 @@ import ( "k8s.io/minikube/pkg/drivers/kic/oci" "k8s.io/minikube/pkg/minikube/bootstrapper/bsutil" "k8s.io/minikube/pkg/minikube/bootstrapper/bsutil/kverify" + "k8s.io/minikube/pkg/minikube/bootstrapper/images" "k8s.io/minikube/pkg/minikube/cni" "k8s.io/minikube/pkg/minikube/config" "k8s.io/minikube/pkg/minikube/constants" @@ -443,6 +444,12 @@ func getRepository(cmd *cobra.Command, k8sVersion string) string { download.SetAliyunMirror() } + // Need to override the default of old kubeadm versions + v := semver.MustParse(strings.TrimPrefix(k8sVersion, version.VersionPrefix)) + if repository == "" && images.NeedsImageRepository(v) { + repository = images.DefaultKubernetesRepo(v) + } + if cmd.Flags().Changed(imageRepository) || cmd.Flags().Changed(imageMirrorCountry) { out.Styled(style.Success, "Using image repository {{.name}}", out.V{"name": repository}) } diff --git a/cmd/minikube/cmd/start_test.go b/cmd/minikube/cmd/start_test.go index 9b0088f1e3a7..625c9ea320dd 100644 --- a/cmd/minikube/cmd/start_test.go +++ b/cmd/minikube/cmd/start_test.go @@ -167,6 +167,50 @@ func TestMirrorCountry(t *testing.T) { } } +func TestImageRepository(t *testing.T) { + // Set default disk size value in lieu of flag init + viper.SetDefault(humanReadableDiskSize, defaultDiskSize) + checkRepository = checkRepoMock + rtime := constants.DefaultContainerRuntime + var tests = []struct { + description string + k8sVersion string + imageRepository string + needsOverride bool + cfg *cfg.ClusterConfig + }{ + { + description: "repository-registry_old", + imageRepository: "", + k8sVersion: "v1.21.0", + needsOverride: true, + }, + { + description: "repository-registry_new", + imageRepository: "", + k8sVersion: "v1.25.0", + needsOverride: false, + }, + } + + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + cmd := &cobra.Command{} + viper.SetDefault(imageRepository, test.imageRepository) + viper.SetDefault(imageMirrorCountry, "") + viper.SetDefault(kvmNUMACount, 1) + config, _, err := generateClusterConfig(cmd, nil, test.k8sVersion, rtime, driver.Mock) + if err != nil { + t.Fatalf("Got unexpected error %v during config generation", err) + } + repo := config.KubernetesConfig.ImageRepository + if repo == "" && test.needsOverride { + t.Fatalf("Version %v needs image repository to be defined: %s", test.k8sVersion, repo) + } + }) + } +} + func TestGenerateCfgFromFlagsHTTPProxyHandling(t *testing.T) { // Set default disk size value in lieu of flag init viper.SetDefault(humanReadableDiskSize, defaultDiskSize) diff --git a/pkg/minikube/bootstrapper/images/images_test.go b/pkg/minikube/bootstrapper/images/images_test.go index 67078d294a2d..96d97fa4d9fc 100644 --- a/pkg/minikube/bootstrapper/images/images_test.go +++ b/pkg/minikube/bootstrapper/images/images_test.go @@ -33,49 +33,49 @@ func TestEssentials(t *testing.T) { images []string }{ {"v1.18.0", strings.Split(strings.Trim(` -k8s.gcr.io/kube-apiserver:v1.18.0 -k8s.gcr.io/kube-controller-manager:v1.18.0 -k8s.gcr.io/kube-scheduler:v1.18.0 -k8s.gcr.io/kube-proxy:v1.18.0 -k8s.gcr.io/pause:3.2 -k8s.gcr.io/etcd:3.4.3-0 -k8s.gcr.io/coredns:1.6.7 +registry.k8s.io/kube-apiserver:v1.18.0 +registry.k8s.io/kube-controller-manager:v1.18.0 +registry.k8s.io/kube-scheduler:v1.18.0 +registry.k8s.io/kube-proxy:v1.18.0 +registry.k8s.io/pause:3.2 +registry.k8s.io/etcd:3.4.3-0 +registry.k8s.io/coredns:1.6.7 `, "\n"), "\n")}, {"v1.19.0", strings.Split(strings.Trim(` -k8s.gcr.io/kube-apiserver:v1.19.0 -k8s.gcr.io/kube-controller-manager:v1.19.0 -k8s.gcr.io/kube-scheduler:v1.19.0 -k8s.gcr.io/kube-proxy:v1.19.0 -k8s.gcr.io/pause:3.2 -k8s.gcr.io/etcd:3.4.9-1 -k8s.gcr.io/coredns:1.7.0 +registry.k8s.io/kube-apiserver:v1.19.0 +registry.k8s.io/kube-controller-manager:v1.19.0 +registry.k8s.io/kube-scheduler:v1.19.0 +registry.k8s.io/kube-proxy:v1.19.0 +registry.k8s.io/pause:3.2 +registry.k8s.io/etcd:3.4.9-1 +registry.k8s.io/coredns:1.7.0 `, "\n"), "\n")}, {"v1.20.0", strings.Split(strings.Trim(` -k8s.gcr.io/kube-apiserver:v1.20.0 -k8s.gcr.io/kube-controller-manager:v1.20.0 -k8s.gcr.io/kube-scheduler:v1.20.0 -k8s.gcr.io/kube-proxy:v1.20.0 -k8s.gcr.io/pause:3.2 -k8s.gcr.io/etcd:3.4.13-0 -k8s.gcr.io/coredns:1.7.0 +registry.k8s.io/kube-apiserver:v1.20.0 +registry.k8s.io/kube-controller-manager:v1.20.0 +registry.k8s.io/kube-scheduler:v1.20.0 +registry.k8s.io/kube-proxy:v1.20.0 +registry.k8s.io/pause:3.2 +registry.k8s.io/etcd:3.4.13-0 +registry.k8s.io/coredns:1.7.0 `, "\n"), "\n")}, {"v1.21.0", strings.Split(strings.Trim(` -k8s.gcr.io/kube-apiserver:v1.21.0 -k8s.gcr.io/kube-controller-manager:v1.21.0 -k8s.gcr.io/kube-scheduler:v1.21.0 -k8s.gcr.io/kube-proxy:v1.21.0 -k8s.gcr.io/pause:3.4.1 -k8s.gcr.io/etcd:3.4.13-0 -k8s.gcr.io/coredns/coredns:v1.8.0 +registry.k8s.io/kube-apiserver:v1.21.0 +registry.k8s.io/kube-controller-manager:v1.21.0 +registry.k8s.io/kube-scheduler:v1.21.0 +registry.k8s.io/kube-proxy:v1.21.0 +registry.k8s.io/pause:3.4.1 +registry.k8s.io/etcd:3.4.13-0 +registry.k8s.io/coredns/coredns:v1.8.0 `, "\n"), "\n")}, {"v1.22.0", strings.Split(strings.Trim(` -k8s.gcr.io/kube-apiserver:v1.22.0 -k8s.gcr.io/kube-controller-manager:v1.22.0 -k8s.gcr.io/kube-scheduler:v1.22.0 -k8s.gcr.io/kube-proxy:v1.22.0 -k8s.gcr.io/pause:3.5 -k8s.gcr.io/etcd:3.5.0-0 -k8s.gcr.io/coredns/coredns:v1.8.4 +registry.k8s.io/kube-apiserver:v1.22.0 +registry.k8s.io/kube-controller-manager:v1.22.0 +registry.k8s.io/kube-scheduler:v1.22.0 +registry.k8s.io/kube-proxy:v1.22.0 +registry.k8s.io/pause:3.5 +registry.k8s.io/etcd:3.5.0-0 +registry.k8s.io/coredns/coredns:v1.8.4 `, "\n"), "\n")}, } for _, tc := range testCases { @@ -85,7 +85,7 @@ k8s.gcr.io/coredns/coredns:v1.8.4 t.Fatal(err) } want := tc.images - got := essentials("k8s.gcr.io", v) + got := essentials("registry.k8s.io", v) if diff := cmp.Diff(want, got); diff != "" { t.Errorf("images mismatch (-want +got):\n%s", diff) } diff --git a/pkg/minikube/bootstrapper/images/kubeadm_test.go b/pkg/minikube/bootstrapper/images/kubeadm_test.go index cac1bc267279..ccc058eb8db2 100644 --- a/pkg/minikube/bootstrapper/images/kubeadm_test.go +++ b/pkg/minikube/bootstrapper/images/kubeadm_test.go @@ -18,23 +18,26 @@ package images import ( "sort" + "strings" "testing" + "github.com/blang/semver/v4" "github.com/google/go-cmp/cmp" "k8s.io/minikube/pkg/version" ) func TestKubeadmImages(t *testing.T) { tests := []struct { - version string - mirror string - invalid bool - want []string + version string + mirror string + invalid bool + override bool + want []string }{ - {"invalid", "", true, nil}, - {"v0.0.1", "", true, nil}, // too old - {"v2.0.0", "", true, nil}, // too new - {"v1.26.0-rc.0", "", false, []string{ + {"invalid", "", true, false, nil}, + {"v0.0.1", "", true, true, nil}, // too old + {"v2.0.0", "", true, false, nil}, // too new + {"v1.26.0-rc.0", "", false, false, []string{ "registry.k8s.io/kube-apiserver:v1.26.0-rc.0", "registry.k8s.io/kube-controller-manager:v1.26.0-rc.0", "registry.k8s.io/kube-scheduler:v1.26.0-rc.0", @@ -44,7 +47,7 @@ func TestKubeadmImages(t *testing.T) { "registry.k8s.io/pause:3.9", "gcr.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.25.4", "", false, []string{ + {"v1.25.4", "", false, false, []string{ "registry.k8s.io/kube-apiserver:v1.25.4", "registry.k8s.io/kube-controller-manager:v1.25.4", "registry.k8s.io/kube-scheduler:v1.25.4", @@ -54,7 +57,7 @@ func TestKubeadmImages(t *testing.T) { "registry.k8s.io/pause:3.8", "gcr.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.25.0", "", false, []string{ + {"v1.25.0", "", false, false, []string{ "registry.k8s.io/kube-proxy:v1.25.0", "registry.k8s.io/kube-scheduler:v1.25.0", "registry.k8s.io/kube-controller-manager:v1.25.0", @@ -64,7 +67,7 @@ func TestKubeadmImages(t *testing.T) { "registry.k8s.io/pause:3.8", "gcr.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.25.0", "mirror.k8s.io", false, []string{ + {"v1.25.0", "mirror.k8s.io", false, false, []string{ "mirror.k8s.io/kube-proxy:v1.25.0", "mirror.k8s.io/kube-scheduler:v1.25.0", "mirror.k8s.io/kube-controller-manager:v1.25.0", @@ -74,48 +77,48 @@ func TestKubeadmImages(t *testing.T) { "mirror.k8s.io/pause:3.8", "mirror.k8s.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.24.0", "", false, []string{ - "k8s.gcr.io/kube-proxy:v1.24.0", - "k8s.gcr.io/kube-scheduler:v1.24.0", - "k8s.gcr.io/kube-controller-manager:v1.24.0", - "k8s.gcr.io/kube-apiserver:v1.24.0", - "k8s.gcr.io/coredns/coredns:v1.8.6", - "k8s.gcr.io/etcd:3.5.3-0", - "k8s.gcr.io/pause:3.7", + {"v1.24.0", "", false, true, []string{ + "registry.k8s.io/kube-proxy:v1.24.0", + "registry.k8s.io/kube-scheduler:v1.24.0", + "registry.k8s.io/kube-controller-manager:v1.24.0", + "registry.k8s.io/kube-apiserver:v1.24.0", + "registry.k8s.io/coredns/coredns:v1.8.6", + "registry.k8s.io/etcd:3.5.3-0", + "registry.k8s.io/pause:3.7", "gcr.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.23.0", "", false, []string{ - "k8s.gcr.io/kube-proxy:v1.23.0", - "k8s.gcr.io/kube-scheduler:v1.23.0", - "k8s.gcr.io/kube-controller-manager:v1.23.0", - "k8s.gcr.io/kube-apiserver:v1.23.0", - "k8s.gcr.io/coredns/coredns:v1.8.6", - "k8s.gcr.io/etcd:3.5.1-0", - "k8s.gcr.io/pause:3.6", + {"v1.23.0", "", false, true, []string{ + "registry.k8s.io/kube-proxy:v1.23.0", + "registry.k8s.io/kube-scheduler:v1.23.0", + "registry.k8s.io/kube-controller-manager:v1.23.0", + "registry.k8s.io/kube-apiserver:v1.23.0", + "registry.k8s.io/coredns/coredns:v1.8.6", + "registry.k8s.io/etcd:3.5.1-0", + "registry.k8s.io/pause:3.6", "gcr.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.22.0", "", false, []string{ - "k8s.gcr.io/kube-proxy:v1.22.0", - "k8s.gcr.io/kube-scheduler:v1.22.0", - "k8s.gcr.io/kube-controller-manager:v1.22.0", - "k8s.gcr.io/kube-apiserver:v1.22.0", - "k8s.gcr.io/coredns/coredns:v1.8.4", - "k8s.gcr.io/etcd:3.5.0-0", - "k8s.gcr.io/pause:3.5", + {"v1.22.0", "", false, true, []string{ + "registry.k8s.io/kube-proxy:v1.22.0", + "registry.k8s.io/kube-scheduler:v1.22.0", + "registry.k8s.io/kube-controller-manager:v1.22.0", + "registry.k8s.io/kube-apiserver:v1.22.0", + "registry.k8s.io/coredns/coredns:v1.8.4", + "registry.k8s.io/etcd:3.5.0-0", + "registry.k8s.io/pause:3.5", "gcr.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.16.0", "", false, []string{ - "k8s.gcr.io/kube-proxy:v1.16.0", - "k8s.gcr.io/kube-scheduler:v1.16.0", - "k8s.gcr.io/kube-controller-manager:v1.16.0", - "k8s.gcr.io/kube-apiserver:v1.16.0", - "k8s.gcr.io/coredns:1.6.2", - "k8s.gcr.io/etcd:3.3.15-0", - "k8s.gcr.io/pause:3.1", + {"v1.16.0", "", false, true, []string{ + "registry.k8s.io/kube-proxy:v1.16.0", + "registry.k8s.io/kube-scheduler:v1.16.0", + "registry.k8s.io/kube-controller-manager:v1.16.0", + "registry.k8s.io/kube-apiserver:v1.16.0", + "registry.k8s.io/coredns:1.6.2", + "registry.k8s.io/etcd:3.3.15-0", + "registry.k8s.io/pause:3.1", "gcr.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.11.0", "", true, nil}, - {"v1.10.0", "", true, nil}, + {"v1.11.0", "", true, true, nil}, + {"v1.10.0", "", true, true, nil}, } for _, tc := range tests { got, err := Kubeadm(tc.mirror, tc.version) @@ -125,6 +128,11 @@ func TestKubeadmImages(t *testing.T) { if err != nil && !tc.invalid { t.Fatalf("unexpected err (%s): %v", tc.version, err) } + v, err := semver.Make(strings.TrimPrefix(tc.version, "v")) + needs := NeedsImageRepository(v) + if err == nil && !cmp.Equal(needs, tc.override) { + t.Errorf("needs mismatch, want: %v, got: %v", tc.override, needs) + } sort.Strings(got) sort.Strings(tc.want) if diff := cmp.Diff(tc.want, got); diff != "" { diff --git a/pkg/minikube/bootstrapper/images/repo.go b/pkg/minikube/bootstrapper/images/repo.go index 956d9d13f8f1..a30717ef1cde 100644 --- a/pkg/minikube/bootstrapper/images/repo.go +++ b/pkg/minikube/bootstrapper/images/repo.go @@ -34,10 +34,14 @@ func kubernetesRepo(mirror string, v semver.Version) string { return DefaultKubernetesRepo(v) } -func DefaultKubernetesRepo(kv semver.Version) string { - // these (-1.24) should probably be moved too +func NeedsImageRepository(kv semver.Version) bool { + // make sure to override any old image repository if kv.LT(semver.MustParse("1.25.0-alpha.1")) { - return OldDefaultKubernetesRepo + return true } + return false +} + +func DefaultKubernetesRepo(kv semver.Version) string { return NewDefaultKubernetesRepo } diff --git a/pkg/minikube/bootstrapper/images/repo_test.go b/pkg/minikube/bootstrapper/images/repo_test.go index 9167befd7a45..5ab5dec65161 100644 --- a/pkg/minikube/bootstrapper/images/repo_test.go +++ b/pkg/minikube/bootstrapper/images/repo_test.go @@ -43,7 +43,7 @@ func Test_kubernetesRepo(t *testing.T) { { "", semver.MustParse("1.24.0"), - OldDefaultKubernetesRepo, + NewDefaultKubernetesRepo, }, { "", @@ -54,7 +54,7 @@ func Test_kubernetesRepo(t *testing.T) { for _, tc := range tests { got := kubernetesRepo(tc.mirror, tc.version) if !cmp.Equal(got, tc.want) { - t.Errorf("mirror miss match, want: %s, got: %s", tc.want, got) + t.Errorf("mirror mismatch, want: %s, got: %s", tc.want, got) } } From c6c90abb14b06f1beaa900398ef35cc87f0fda71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Sat, 11 Mar 2023 13:21:06 +0100 Subject: [PATCH 2/5] Change conditional syntax to pass the linter --- pkg/minikube/bootstrapper/images/repo.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/minikube/bootstrapper/images/repo.go b/pkg/minikube/bootstrapper/images/repo.go index a30717ef1cde..04446b815aac 100644 --- a/pkg/minikube/bootstrapper/images/repo.go +++ b/pkg/minikube/bootstrapper/images/repo.go @@ -36,10 +36,7 @@ func kubernetesRepo(mirror string, v semver.Version) string { func NeedsImageRepository(kv semver.Version) bool { // make sure to override any old image repository - if kv.LT(semver.MustParse("1.25.0-alpha.1")) { - return true - } - return false + return kv.LT(semver.MustParse("1.25.0-alpha.1")) } func DefaultKubernetesRepo(kv semver.Version) string { From da9065fce4a1369d61c5b6fd73376d28f4540a29 Mon Sep 17 00:00:00 2001 From: Steven Powell Date: Tue, 28 Mar 2023 17:06:28 -0700 Subject: [PATCH 3/5] remove setting ImageRepository --- cmd/minikube/cmd/start_flags.go | 7 --- cmd/minikube/cmd/start_test.go | 44 ------------------- pkg/minikube/bootstrapper/images/images.go | 10 ++--- .../bootstrapper/images/kubeadm_test.go | 42 +++++++----------- pkg/minikube/bootstrapper/images/repo.go | 15 ++----- pkg/minikube/bootstrapper/images/repo_test.go | 15 ++----- pkg/minikube/node/start.go | 11 +++-- 7 files changed, 34 insertions(+), 110 deletions(-) diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index c95bc1c2f254..12e22c982f24 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -32,7 +32,6 @@ import ( "k8s.io/minikube/pkg/drivers/kic/oci" "k8s.io/minikube/pkg/minikube/bootstrapper/bsutil" "k8s.io/minikube/pkg/minikube/bootstrapper/bsutil/kverify" - "k8s.io/minikube/pkg/minikube/bootstrapper/images" "k8s.io/minikube/pkg/minikube/cni" "k8s.io/minikube/pkg/minikube/config" "k8s.io/minikube/pkg/minikube/constants" @@ -444,12 +443,6 @@ func getRepository(cmd *cobra.Command, k8sVersion string) string { download.SetAliyunMirror() } - // Need to override the default of old kubeadm versions - v := semver.MustParse(strings.TrimPrefix(k8sVersion, version.VersionPrefix)) - if repository == "" && images.NeedsImageRepository(v) { - repository = images.DefaultKubernetesRepo(v) - } - if cmd.Flags().Changed(imageRepository) || cmd.Flags().Changed(imageMirrorCountry) { out.Styled(style.Success, "Using image repository {{.name}}", out.V{"name": repository}) } diff --git a/cmd/minikube/cmd/start_test.go b/cmd/minikube/cmd/start_test.go index 625c9ea320dd..9b0088f1e3a7 100644 --- a/cmd/minikube/cmd/start_test.go +++ b/cmd/minikube/cmd/start_test.go @@ -167,50 +167,6 @@ func TestMirrorCountry(t *testing.T) { } } -func TestImageRepository(t *testing.T) { - // Set default disk size value in lieu of flag init - viper.SetDefault(humanReadableDiskSize, defaultDiskSize) - checkRepository = checkRepoMock - rtime := constants.DefaultContainerRuntime - var tests = []struct { - description string - k8sVersion string - imageRepository string - needsOverride bool - cfg *cfg.ClusterConfig - }{ - { - description: "repository-registry_old", - imageRepository: "", - k8sVersion: "v1.21.0", - needsOverride: true, - }, - { - description: "repository-registry_new", - imageRepository: "", - k8sVersion: "v1.25.0", - needsOverride: false, - }, - } - - for _, test := range tests { - t.Run(test.description, func(t *testing.T) { - cmd := &cobra.Command{} - viper.SetDefault(imageRepository, test.imageRepository) - viper.SetDefault(imageMirrorCountry, "") - viper.SetDefault(kvmNUMACount, 1) - config, _, err := generateClusterConfig(cmd, nil, test.k8sVersion, rtime, driver.Mock) - if err != nil { - t.Fatalf("Got unexpected error %v during config generation", err) - } - repo := config.KubernetesConfig.ImageRepository - if repo == "" && test.needsOverride { - t.Fatalf("Version %v needs image repository to be defined: %s", test.k8sVersion, repo) - } - }) - } -} - func TestGenerateCfgFromFlagsHTTPProxyHandling(t *testing.T) { // Set default disk size value in lieu of flag init viper.SetDefault(humanReadableDiskSize, defaultDiskSize) diff --git a/pkg/minikube/bootstrapper/images/images.go b/pkg/minikube/bootstrapper/images/images.go index b16127e3442f..d4444ce50a2c 100644 --- a/pkg/minikube/bootstrapper/images/images.go +++ b/pkg/minikube/bootstrapper/images/images.go @@ -47,7 +47,7 @@ func Pause(v semver.Version, mirror string) string { imageName := "pause" pv := imageVersion(v, mirror, imageName, "3.6") - return fmt.Sprintf("%s:%s", path.Join(kubernetesRepo(mirror, v), imageName), pv) + return fmt.Sprintf("%s:%s", path.Join(kubernetesRepo(mirror), imageName), pv) } // essentials returns images needed too bootstrap a Kubernetes @@ -67,7 +67,7 @@ func essentials(mirror string, v semver.Version) []string { // componentImage returns a Kubernetes component image to pull func componentImage(name string, v semver.Version, mirror string) string { - return fmt.Sprintf("%s:v%s", path.Join(kubernetesRepo(mirror, v), name), v) + return fmt.Sprintf("%s:v%s", path.Join(kubernetesRepo(mirror), name), v) } // fixes 13136 by getting the latest image version from the k8s.gcr.io repository instead of hardcoded @@ -120,7 +120,7 @@ func coreDNS(v semver.Version, mirror string) string { imageName = "coredns" } - return fmt.Sprintf("%s:%s", path.Join(kubernetesRepo(mirror, v), imageName), cv) + return fmt.Sprintf("%s:%s", path.Join(kubernetesRepo(mirror), imageName), cv) } // etcd returns the image used for etcd @@ -131,7 +131,7 @@ func etcd(v semver.Version, mirror string) string { imageName := "etcd" ev := imageVersion(v, mirror, imageName, "3.5.0-0") - return fmt.Sprintf("%s:%s", path.Join(kubernetesRepo(mirror, v), imageName), ev) + return fmt.Sprintf("%s:%s", path.Join(kubernetesRepo(mirror), imageName), ev) } func imageVersion(v semver.Version, mirror, imageName, defaultVersion string) string { @@ -139,7 +139,7 @@ func imageVersion(v semver.Version, mirror, imageName, defaultVersion string) st if ver, ok := constants.KubeadmImages[versionString][imageName]; ok { return ver } - return findLatestTagFromRepository(fmt.Sprintf(tagURLTemplate, kubernetesRepo(mirror, v), imageName), defaultVersion) + return findLatestTagFromRepository(fmt.Sprintf(tagURLTemplate, kubernetesRepo(mirror), imageName), defaultVersion) } // auxiliary returns images that are helpful for running minikube diff --git a/pkg/minikube/bootstrapper/images/kubeadm_test.go b/pkg/minikube/bootstrapper/images/kubeadm_test.go index ccc058eb8db2..7fb07394e3d7 100644 --- a/pkg/minikube/bootstrapper/images/kubeadm_test.go +++ b/pkg/minikube/bootstrapper/images/kubeadm_test.go @@ -18,26 +18,23 @@ package images import ( "sort" - "strings" "testing" - "github.com/blang/semver/v4" "github.com/google/go-cmp/cmp" "k8s.io/minikube/pkg/version" ) func TestKubeadmImages(t *testing.T) { tests := []struct { - version string - mirror string - invalid bool - override bool - want []string + version string + mirror string + invalid bool + want []string }{ - {"invalid", "", true, false, nil}, - {"v0.0.1", "", true, true, nil}, // too old - {"v2.0.0", "", true, false, nil}, // too new - {"v1.26.0-rc.0", "", false, false, []string{ + {"invalid", "", true, nil}, + {"v0.0.1", "", true, nil}, // too old + {"v2.0.0", "", true, nil}, // too new + {"v1.26.0-rc.0", "", false, []string{ "registry.k8s.io/kube-apiserver:v1.26.0-rc.0", "registry.k8s.io/kube-controller-manager:v1.26.0-rc.0", "registry.k8s.io/kube-scheduler:v1.26.0-rc.0", @@ -47,7 +44,7 @@ func TestKubeadmImages(t *testing.T) { "registry.k8s.io/pause:3.9", "gcr.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.25.4", "", false, false, []string{ + {"v1.25.4", "", false, []string{ "registry.k8s.io/kube-apiserver:v1.25.4", "registry.k8s.io/kube-controller-manager:v1.25.4", "registry.k8s.io/kube-scheduler:v1.25.4", @@ -57,7 +54,7 @@ func TestKubeadmImages(t *testing.T) { "registry.k8s.io/pause:3.8", "gcr.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.25.0", "", false, false, []string{ + {"v1.25.0", "", false, []string{ "registry.k8s.io/kube-proxy:v1.25.0", "registry.k8s.io/kube-scheduler:v1.25.0", "registry.k8s.io/kube-controller-manager:v1.25.0", @@ -67,7 +64,7 @@ func TestKubeadmImages(t *testing.T) { "registry.k8s.io/pause:3.8", "gcr.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.25.0", "mirror.k8s.io", false, false, []string{ + {"v1.25.0", "mirror.k8s.io", false, []string{ "mirror.k8s.io/kube-proxy:v1.25.0", "mirror.k8s.io/kube-scheduler:v1.25.0", "mirror.k8s.io/kube-controller-manager:v1.25.0", @@ -77,7 +74,7 @@ func TestKubeadmImages(t *testing.T) { "mirror.k8s.io/pause:3.8", "mirror.k8s.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.24.0", "", false, true, []string{ + {"v1.24.0", "", false, []string{ "registry.k8s.io/kube-proxy:v1.24.0", "registry.k8s.io/kube-scheduler:v1.24.0", "registry.k8s.io/kube-controller-manager:v1.24.0", @@ -87,7 +84,7 @@ func TestKubeadmImages(t *testing.T) { "registry.k8s.io/pause:3.7", "gcr.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.23.0", "", false, true, []string{ + {"v1.23.0", "", false, []string{ "registry.k8s.io/kube-proxy:v1.23.0", "registry.k8s.io/kube-scheduler:v1.23.0", "registry.k8s.io/kube-controller-manager:v1.23.0", @@ -97,7 +94,7 @@ func TestKubeadmImages(t *testing.T) { "registry.k8s.io/pause:3.6", "gcr.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.22.0", "", false, true, []string{ + {"v1.22.0", "", false, []string{ "registry.k8s.io/kube-proxy:v1.22.0", "registry.k8s.io/kube-scheduler:v1.22.0", "registry.k8s.io/kube-controller-manager:v1.22.0", @@ -107,7 +104,7 @@ func TestKubeadmImages(t *testing.T) { "registry.k8s.io/pause:3.5", "gcr.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.16.0", "", false, true, []string{ + {"v1.16.0", "", false, []string{ "registry.k8s.io/kube-proxy:v1.16.0", "registry.k8s.io/kube-scheduler:v1.16.0", "registry.k8s.io/kube-controller-manager:v1.16.0", @@ -117,8 +114,8 @@ func TestKubeadmImages(t *testing.T) { "registry.k8s.io/pause:3.1", "gcr.io/k8s-minikube/storage-provisioner:" + version.GetStorageProvisionerVersion(), }}, - {"v1.11.0", "", true, true, nil}, - {"v1.10.0", "", true, true, nil}, + {"v1.11.0", "", true, nil}, + {"v1.10.0", "", true, nil}, } for _, tc := range tests { got, err := Kubeadm(tc.mirror, tc.version) @@ -128,11 +125,6 @@ func TestKubeadmImages(t *testing.T) { if err != nil && !tc.invalid { t.Fatalf("unexpected err (%s): %v", tc.version, err) } - v, err := semver.Make(strings.TrimPrefix(tc.version, "v")) - needs := NeedsImageRepository(v) - if err == nil && !cmp.Equal(needs, tc.override) { - t.Errorf("needs mismatch, want: %v, got: %v", tc.override, needs) - } sort.Strings(got) sort.Strings(tc.want) if diff := cmp.Diff(tc.want, got); diff != "" { diff --git a/pkg/minikube/bootstrapper/images/repo.go b/pkg/minikube/bootstrapper/images/repo.go index 04446b815aac..3472e82ee273 100644 --- a/pkg/minikube/bootstrapper/images/repo.go +++ b/pkg/minikube/bootstrapper/images/repo.go @@ -16,10 +16,6 @@ limitations under the License. package images -import ( - "github.com/blang/semver/v4" -) - // OldDefaultKubernetesRepo is the old default Kubernetes repository const OldDefaultKubernetesRepo = "k8s.gcr.io" @@ -27,18 +23,13 @@ const OldDefaultKubernetesRepo = "k8s.gcr.io" const NewDefaultKubernetesRepo = "registry.k8s.io" // kubernetesRepo returns the official Kubernetes repository, or an alternate -func kubernetesRepo(mirror string, v semver.Version) string { +func kubernetesRepo(mirror string) string { if mirror != "" { return mirror } - return DefaultKubernetesRepo(v) -} - -func NeedsImageRepository(kv semver.Version) bool { - // make sure to override any old image repository - return kv.LT(semver.MustParse("1.25.0-alpha.1")) + return DefaultKubernetesRepo() } -func DefaultKubernetesRepo(kv semver.Version) string { +func DefaultKubernetesRepo() string { return NewDefaultKubernetesRepo } diff --git a/pkg/minikube/bootstrapper/images/repo_test.go b/pkg/minikube/bootstrapper/images/repo_test.go index 5ab5dec65161..6979c6ef0c20 100644 --- a/pkg/minikube/bootstrapper/images/repo_test.go +++ b/pkg/minikube/bootstrapper/images/repo_test.go @@ -19,40 +19,33 @@ package images import ( "testing" - "github.com/blang/semver/v4" "github.com/google/go-cmp/cmp" ) func Test_kubernetesRepo(t *testing.T) { - kv := semver.MustParse("1.23.0") tests := []struct { - mirror string - version semver.Version - want string + mirror string + want string }{ { "", - kv, - DefaultKubernetesRepo(kv), + DefaultKubernetesRepo(), }, { "mirror.k8s.io", - kv, "mirror.k8s.io", }, { "", - semver.MustParse("1.24.0"), NewDefaultKubernetesRepo, }, { "", - semver.MustParse("1.25.0"), NewDefaultKubernetesRepo, }, } for _, tc := range tests { - got := kubernetesRepo(tc.mirror, tc.version) + got := kubernetesRepo(tc.mirror) if !cmp.Equal(got, tc.want) { t.Errorf("mirror mismatch, want: %s, got: %s", tc.want, got) } diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index 14ec9ff74307..fa8b240bc024 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -641,7 +641,7 @@ func startMachine(cfg *config.ClusterConfig, node *config.Node, delOnFail bool) return runner, preExists, m, host, errors.Wrap(err, "Failed to get command runner") } - ip, err := validateNetwork(host, runner, cfg.KubernetesConfig.ImageRepository, cfg.KubernetesConfig.KubernetesVersion) + ip, err := validateNetwork(host, runner, cfg.KubernetesConfig.ImageRepository) if err != nil { return runner, preExists, m, host, errors.Wrap(err, "Failed to validate network") } @@ -724,7 +724,7 @@ func startHostInternal(api libmachine.API, cc *config.ClusterConfig, n *config.N } // validateNetwork tries to catch network problems as soon as possible -func validateNetwork(h *host.Host, r command.Runner, imageRepository string, kubernetesVersion string) (string, error) { +func validateNetwork(h *host.Host, r command.Runner, imageRepository string) (string, error) { ip, err := h.Driver.GetIP() if err != nil { return ip, err @@ -756,7 +756,7 @@ func validateNetwork(h *host.Host, r command.Runner, imageRepository string, kub } // Non-blocking - go tryRegistry(r, h.Driver.DriverName(), imageRepository, kubernetesVersion, ip) + go tryRegistry(r, h.Driver.DriverName(), imageRepository, ip) return ip, nil } @@ -812,7 +812,7 @@ func trySSH(h *host.Host, ip string) error { } // tryRegistry tries to connect to the image repository -func tryRegistry(r command.Runner, driverName string, imageRepository string, kubernetesVersion string, ip string) { +func tryRegistry(r command.Runner, driverName, imageRepository, ip string) { // 2 second timeout. For best results, call tryRegistry in a non-blocking manner. opts := []string{"-sS", "-m", "2"} @@ -822,8 +822,7 @@ func tryRegistry(r command.Runner, driverName string, imageRepository string, ku } if imageRepository == "" { - v, _ := util.ParseKubernetesVersion(kubernetesVersion) - imageRepository = images.DefaultKubernetesRepo(v) + imageRepository = images.DefaultKubernetesRepo() } opts = append(opts, fmt.Sprintf("https://%s/", imageRepository)) From 77f207e01b4c4b129336c3e10181d33a16c6bf08 Mon Sep 17 00:00:00 2001 From: Steven Powell Date: Wed, 29 Mar 2023 09:58:47 -0700 Subject: [PATCH 4/5] small clean and fix tests with old k8s versions --- pkg/minikube/bootstrapper/images/repo.go | 13 +++---------- pkg/minikube/bootstrapper/images/repo_test.go | 6 +++--- pkg/minikube/node/start.go | 2 +- test/integration/start_stop_delete_test.go | 10 ++++++++-- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/pkg/minikube/bootstrapper/images/repo.go b/pkg/minikube/bootstrapper/images/repo.go index 3472e82ee273..9d739f6ce491 100644 --- a/pkg/minikube/bootstrapper/images/repo.go +++ b/pkg/minikube/bootstrapper/images/repo.go @@ -16,20 +16,13 @@ limitations under the License. package images -// OldDefaultKubernetesRepo is the old default Kubernetes repository -const OldDefaultKubernetesRepo = "k8s.gcr.io" - -// NewDefaultKubernetesRepo is the new default Kubernetes repository -const NewDefaultKubernetesRepo = "registry.k8s.io" +// DefaultKubernetesRepo is the default Kubernetes repository +const DefaultKubernetesRepo = "registry.k8s.io" // kubernetesRepo returns the official Kubernetes repository, or an alternate func kubernetesRepo(mirror string) string { if mirror != "" { return mirror } - return DefaultKubernetesRepo() -} - -func DefaultKubernetesRepo() string { - return NewDefaultKubernetesRepo + return DefaultKubernetesRepo } diff --git a/pkg/minikube/bootstrapper/images/repo_test.go b/pkg/minikube/bootstrapper/images/repo_test.go index 6979c6ef0c20..6744b1607014 100644 --- a/pkg/minikube/bootstrapper/images/repo_test.go +++ b/pkg/minikube/bootstrapper/images/repo_test.go @@ -29,7 +29,7 @@ func Test_kubernetesRepo(t *testing.T) { }{ { "", - DefaultKubernetesRepo(), + DefaultKubernetesRepo, }, { "mirror.k8s.io", @@ -37,11 +37,11 @@ func Test_kubernetesRepo(t *testing.T) { }, { "", - NewDefaultKubernetesRepo, + DefaultKubernetesRepo, }, { "", - NewDefaultKubernetesRepo, + DefaultKubernetesRepo, }, } for _, tc := range tests { diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index fa8b240bc024..b1ddf65bbb1c 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -822,7 +822,7 @@ func tryRegistry(r command.Runner, driverName, imageRepository, ip string) { } if imageRepository == "" { - imageRepository = images.DefaultKubernetesRepo() + imageRepository = images.DefaultKubernetesRepo } opts = append(opts, fmt.Sprintf("https://%s/", imageRepository)) diff --git a/test/integration/start_stop_delete_test.go b/test/integration/start_stop_delete_test.go index edfe9be87770..9f4b4d606e84 100644 --- a/test/integration/start_stop_delete_test.go +++ b/test/integration/start_stop_delete_test.go @@ -354,7 +354,7 @@ func testPulledImages(ctx context.Context, t *testing.T, profile, version string rr, err := Run(t, exec.CommandContext(ctx, Target(), "ssh", "-p", profile, "sudo crictl images -o json")) if err != nil { - t.Errorf("failed tp get images inside minikube. args %q: %v", rr.Command(), err) + t.Errorf("failed to get images inside minikube. args %q: %v", rr.Command(), err) } jv := map[string][]struct { Tags []string `json:"repoTags"` @@ -377,7 +377,13 @@ func testPulledImages(ctx context.Context, t *testing.T, profile, version string } } } - wantRaw, err := images.Kubeadm("", version) + + mirror := "" + // Kubernetes versions prior to v1.25 will contain the old registry due to the preload + if v, _ := util.ParseKubernetesVersion(kubernetesVersion); v.LT(semver.MustParse("1.25.0-alpha.1")) { + mirror = "k8s.gcr.io" + } + wantRaw, err := images.Kubeadm(mirror, version) if err != nil { t.Errorf("failed to get kubeadm images for %s : %v", version, err) } From 201062203971ea436e7b593bda58506fe067530d Mon Sep 17 00:00:00 2001 From: Steven Powell Date: Wed, 29 Mar 2023 10:02:46 -0700 Subject: [PATCH 5/5] remove duplicate test cases --- pkg/minikube/bootstrapper/images/repo_test.go | 8 -------- test/integration/start_stop_delete_test.go | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/pkg/minikube/bootstrapper/images/repo_test.go b/pkg/minikube/bootstrapper/images/repo_test.go index 6744b1607014..65dea939257b 100644 --- a/pkg/minikube/bootstrapper/images/repo_test.go +++ b/pkg/minikube/bootstrapper/images/repo_test.go @@ -35,14 +35,6 @@ func Test_kubernetesRepo(t *testing.T) { "mirror.k8s.io", "mirror.k8s.io", }, - { - "", - DefaultKubernetesRepo, - }, - { - "", - DefaultKubernetesRepo, - }, } for _, tc := range tests { got := kubernetesRepo(tc.mirror) diff --git a/test/integration/start_stop_delete_test.go b/test/integration/start_stop_delete_test.go index 9f4b4d606e84..8541d555ea7a 100644 --- a/test/integration/start_stop_delete_test.go +++ b/test/integration/start_stop_delete_test.go @@ -380,7 +380,7 @@ func testPulledImages(ctx context.Context, t *testing.T, profile, version string mirror := "" // Kubernetes versions prior to v1.25 will contain the old registry due to the preload - if v, _ := util.ParseKubernetesVersion(kubernetesVersion); v.LT(semver.MustParse("1.25.0-alpha.1")) { + if v, _ := util.ParseKubernetesVersion(version); v.LT(semver.MustParse("1.25.0-alpha.1")) { mirror = "k8s.gcr.io" } wantRaw, err := images.Kubeadm(mirror, version)