Merge pull request #3829 from Ankitasw/ami-naming

 Change AMI name generation based on new format in image-builder

Author: k8s-ci-robot

cmd/clusterawsadm/ami/helper.go

@@ -216,7 +216,11 @@ func getAllImages(ec2Client ec2iface.EC2API, ownerID string) (map[string][]*ec2.
 	imagesMap := make(map[string][]*ec2.Image)
 	for _, image := range out.Images {
 		arr := strings.Split(aws.StringValue(image.Name), "-")
-		arr = arr[:len(arr)-2]
+		if arr[len(arr)-2] == "00" {
+			arr = arr[:len(arr)-2]
+		} else {
+			arr = arr[:len(arr)-1]
+		}
 		name := strings.Join(arr, "-")
 		images, ok := imagesMap[name]
 		if !ok {
@@ -230,18 +234,30 @@ func getAllImages(ec2Client ec2iface.EC2API, ownerID string) (map[string][]*ec2.
 
 func findAMI(imagesMap map[string][]*ec2.Image, baseOS, kubernetesVersion string) (*ec2.Image, error) {
 	amiNameFormat := "capa-ami-{{.BaseOS}}-{{.K8sVersion}}"
+	// Support new AMI format capa-ami-<os-version>-?<k8s-version>-*
 	amiName, err := ec2service.GenerateAmiName(amiNameFormat, baseOS, kubernetesVersion)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to process ami format: %q", amiNameFormat)
 	}
-
 	if val, ok := imagesMap[amiName]; ok && val != nil {
-		latestImage, err := ec2service.GetLatestImage(val)
+		return latestAMI(val)
+	} else {
+		amiName, err = ec2service.GenerateAmiName(amiNameFormat, baseOS, strings.TrimPrefix(kubernetesVersion, "v"))
 		if err != nil {
-			return nil, err
+			return nil, errors.Wrapf(err, "failed to process ami format: %q", amiNameFormat)
+		}
+		if val, ok = imagesMap[amiName]; ok && val != nil {
+			return latestAMI(val)
 		}
-		return latestImage, nil
 	}
 
 	return nil, nil
 }
+
+func latestAMI(val []*ec2.Image) (*ec2.Image, error) {
+	latestImage, err := ec2service.GetLatestImage(val)
+	if err != nil {
+		return nil, err
+	}
+	return latestImage, nil
+}

pkg/cloud/services/ec2/ami.go

@@ -77,7 +77,7 @@ type AMILookup struct {
 
 // GenerateAmiName will generate an AMI name.
 func GenerateAmiName(amiNameFormat, baseOS, kubernetesVersion string) (string, error) {
-	amiNameParameters := AMILookup{baseOS, strings.TrimPrefix(kubernetesVersion, "v")}
+	amiNameParameters := AMILookup{baseOS, kubernetesVersion}
 	// revert to default if not specified
 	if amiNameFormat == "" {
 		amiNameFormat = DefaultAmiNameFormat
@@ -106,7 +106,7 @@ func DefaultAMILookup(ec2Client ec2iface.EC2API, ownerID, baseOS, kubernetesVers
 		baseOS = defaultMachineAMILookupBaseOS
 	}
 
-	amiName, err := GenerateAmiName(amiNameFormat, baseOS, kubernetesVersion)
+	amiName, err := GenerateAmiName(amiNameFormat, baseOS, strings.TrimPrefix(kubernetesVersion, "v"))
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to process ami format: %q", amiNameFormat)
 	}

pkg/cloud/services/ec2/ami_test.go

@@ -263,7 +263,7 @@ func TestGenerateAmiName(t *testing.T) {
 		{
 			name: "Should return image name even if OS and amiNameFormat is empty",
 			args: args{
-				kubernetesVersion: "v1.23.3",
+				kubernetesVersion: "1.23.3",
 			},
 			want: "capa-ami--?1.23.3-*",
 		},
@@ -285,6 +285,15 @@ func TestGenerateAmiName(t *testing.T) {
 			},
 			want: "random-centos-7-?1.23.3-*",
 		},
+		{
+			name: "Should return valid amiName if new AMI name format passed",
+			args: args{
+				amiNameFormat:     "random-{{.BaseOS}}-{{.K8sVersion}}",
+				baseOS:            "centos-7",
+				kubernetesVersion: "v1.23.3",
+			},
+			want: "random-centos-7-v1.23.3",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/cloud/services/ec2/instances_test.go

@@ -557,7 +557,7 @@ func TestCreateInstance(t *testing.T) {
 				},
 			},
 			expect: func(m *mocks.MockEC2APIMockRecorder) {
-				amiName, err := GenerateAmiName("capa-ami-{{.BaseOS}}-?{{.K8sVersion}}-*", "ubuntu-18.04", "v1.16.1")
+				amiName, err := GenerateAmiName("capa-ami-{{.BaseOS}}-?{{.K8sVersion}}-*", "ubuntu-18.04", "1.16.1")
 				if err != nil {
 					t.Fatalf("Failed to process ami format: %v", err)
 				}
@@ -687,7 +687,7 @@ func TestCreateInstance(t *testing.T) {
 				},
 			},
 			expect: func(m *mocks.MockEC2APIMockRecorder) {
-				amiName, err := GenerateAmiName("capa-ami-{{.BaseOS}}-?{{.K8sVersion}}-*", "ubuntu-18.04", "v1.16.1")
+				amiName, err := GenerateAmiName("capa-ami-{{.BaseOS}}-?{{.K8sVersion}}-*", "ubuntu-18.04", "1.16.1")
 				if err != nil {
 					t.Fatalf("Failed to process ami format: %v", err)
 				}
@@ -818,7 +818,7 @@ func TestCreateInstance(t *testing.T) {
 				},
 			},
 			expect: func(m *mocks.MockEC2APIMockRecorder) {
-				amiName, err := GenerateAmiName("capa-ami-{{.BaseOS}}-?{{.K8sVersion}}-*", "ubuntu-18.04", "v1.16.1")
+				amiName, err := GenerateAmiName("capa-ami-{{.BaseOS}}-?{{.K8sVersion}}-*", "ubuntu-18.04", "1.16.1")
 				if err != nil {
 					t.Fatalf("Failed to process ami format: %v", err)
 				}