Fix for tarball image names after 1.16

Image names from 1.16 on include an architecture suffix,
e.g. "-amd64"; the generic alias continues to work when pulling, but
when loading from a tarball (i.e. running in CI) we must use the
per-architecture name.

pkg/model/components/apiserver.go

@@ -142,7 +142,7 @@ func (b *KubeAPIServerOptionsBuilder) BuildOptions(o interface{}) error {
 		return nil
 	}
 
-	image, err := Image("kube-apiserver", clusterSpec, b.AssetBuilder)
+	image, err := Image("kube-apiserver", b.Architecture(), clusterSpec, b.AssetBuilder)
 	if err != nil {
 		return err
 	}

pkg/model/components/context.go

@@ -51,6 +51,13 @@ func (c *OptionsContext) IsKubernetesLT(version string) bool {
 	return !c.IsKubernetesGTE(version)
 }
 
+// Architecture returns the architecture we are using
+// We currently only support amd64, and we probably need to pass the InstanceGroup in
+// But we can start collecting the architectural dependencies
+func (c *OptionsContext) Architecture() string {
+	return "amd64"
+}
+
 // KubernetesVersion parses the semver version of kubernetes, from the cluster spec
 // Deprecated: prefer using OptionsContext.KubernetesVersion
 func KubernetesVersion(clusterSpec *kops.ClusterSpec) (*semver.Version, error) {
@@ -128,7 +135,7 @@ func IsBaseURL(kubernetesVersion string) bool {
 }
 
 // Image returns the docker image name for the specified component
-func Image(component string, clusterSpec *kops.ClusterSpec, assetsBuilder *assets.AssetBuilder) (string, error) {
+func Image(component string, architecture string, clusterSpec *kops.ClusterSpec, assetsBuilder *assets.AssetBuilder) (string, error) {
 	if assetsBuilder == nil {
 		return "", fmt.Errorf("unable to parse assets as assetBuilder is not defined")
 	}
@@ -143,8 +150,10 @@ func Image(component string, clusterSpec *kops.ClusterSpec, assetsBuilder *asset
 		return "", err
 	}
 
+	imageName := component
+
 	if !IsBaseURL(clusterSpec.KubernetesVersion) {
-		image := "k8s.gcr.io/" + component + ":" + "v" + kubernetesVersion.String()
+		image := "k8s.gcr.io/" + imageName + ":" + "v" + kubernetesVersion.String()
 
 		image, err := assetsBuilder.RemapImage(image)
 		if err != nil {
@@ -153,11 +162,28 @@ func Image(component string, clusterSpec *kops.ClusterSpec, assetsBuilder *asset
 		return image, nil
 	}
 
+	// The simple name is valid when pulling (before 1.16 it was
+	// only amd64, as of 1.16 it is a manifest list).  But if we
+	// are loading from a tarfile then the image is tagged with
+	// the architecture suffix.
+	//
+	// i.e. k8s.gcr.io/kube-apiserver:v1.16.0 is a manifest list
+	// and we _can_ also pull
+	// k8s.gcr.io/kube-apiserver-amd64:v1.16.0 directly.  But if
+	// we load https://.../v1.16.0/amd64/kube-apiserver.tar then
+	// the image inside that tar file is named
+	// "k8s.gcr.io/kube-apiserver-amd64:v1.16.0"
+	//
+	// But ... this is only the case from 1.16 on...
+	if kubernetesVersion.IsGTE("1.16") {
+		imageName += "-" + architecture
+	}
+
 	baseURL := clusterSpec.KubernetesVersion
 	baseURL = strings.TrimSuffix(baseURL, "/")
 
 	// TODO path.Join here?
-	tagURL := baseURL + "/bin/linux/amd64/" + component + ".docker_tag"
+	tagURL := baseURL + "/bin/linux/" + architecture + "/" + component + ".docker_tag"
 	klog.V(2).Infof("Downloading docker tag for %s from: %s", component, tagURL)
 
 	b, err := vfs.Context.ReadFile(tagURL)
@@ -167,7 +193,7 @@ func Image(component string, clusterSpec *kops.ClusterSpec, assetsBuilder *asset
 	tag := strings.TrimSpace(string(b))
 	klog.V(2).Infof("Found tag %q for %q", tag, component)
 
-	image := "k8s.gcr.io/" + component + ":" + tag
+	image := "k8s.gcr.io/" + imageName + ":" + tag
 
 	// When we're using a docker load-ed image, we are likely a CI build.
 	// But the k8s.gcr.io prefix is an alias, and we only double-tagged from 1.10 onwards.

pkg/model/components/image_test.go

@@ -86,6 +86,27 @@ func TestImage(t *testing.T) {
 			},
 			Expected: "k8s.gcr.io/kube-apiserver:1-10-0dockertag",
 		},
+		{
+			Component: "kube-apiserver",
+			Cluster: &kops.Cluster{
+				Spec: kops.ClusterSpec{
+					KubernetesVersion: "memfs://v1.16.0-download/",
+				},
+			},
+			VFS: map[string]string{
+				"memfs://v1.16.0-download/bin/linux/amd64/kube-apiserver.docker_tag": "1-16-0dockertag",
+			},
+			Expected: "k8s.gcr.io/kube-apiserver-amd64:1-16-0dockertag",
+		},
+		{
+			Component: "kube-apiserver",
+			Cluster: &kops.Cluster{
+				Spec: kops.ClusterSpec{
+					KubernetesVersion: "1.16.0",
+				},
+			},
+			Expected: "k8s.gcr.io/kube-apiserver:v1.16.0",
+		},
 	}
 
 	for _, g := range grid {
@@ -104,8 +125,10 @@ func TestImage(t *testing.T) {
 			}
 		}
 
+		architecture := "amd64"
+
 		assetBuilder := assets.NewAssetBuilder(g.Cluster, "")
-		actual, err := Image(g.Component, &g.Cluster.Spec, assetBuilder)
+		actual, err := Image(g.Component, architecture, &g.Cluster.Spec, assetBuilder)
 		if err != nil {
 			t.Errorf("unexpected error from image %q %v: %v",
 				g.Component, g.Cluster.Spec.KubernetesVersion, err)

pkg/model/components/kubecontrollermanager.go

@@ -136,7 +136,7 @@ func (b *KubeControllerManagerOptionsBuilder) BuildOptions(o interface{}) error
 
 	kcm.LogLevel = 2
 
-	image, err := Image("kube-controller-manager", clusterSpec, b.Context.AssetBuilder)
+	image, err := Image("kube-controller-manager", b.Context.Architecture(), clusterSpec, b.Context.AssetBuilder)
 	if err != nil {
 		return err
 	}

pkg/model/components/kubeproxy.go

@@ -47,7 +47,7 @@ func (b *KubeProxyOptionsBuilder) BuildOptions(o interface{}) error {
 		config.CPURequest = "100m"
 	}
 
-	image, err := Image("kube-proxy", clusterSpec, b.Context.AssetBuilder)
+	image, err := Image("kube-proxy", b.Context.Architecture(), clusterSpec, b.Context.AssetBuilder)
 	if err != nil {
 		return err
 	}

pkg/model/components/kubescheduler.go

@@ -49,7 +49,7 @@ func (b *KubeSchedulerOptionsBuilder) BuildOptions(o interface{}) error {
 	}
 
 	if config.Image == "" {
-		image, err := Image("kube-scheduler", clusterSpec, b.AssetBuilder)
+		image, err := Image("kube-scheduler", b.Architecture(), clusterSpec, b.AssetBuilder)
 		if err != nil {
 			return err
 		}