Merge pull request #9303 from openshift-cherrypick-robot/cherry-pick-…

…9245-to-release-4.18 [release-4.18] OCPBUGS-46033: Power VS: Create region-zone-sysType hierarchy
openshift · Jan 8, 2025 · 0f9827f · 0f9827f
2 parents bb8c7d5 + 32165ad
commit 0f9827f
Show file tree

Hide file tree

Showing 7 changed files with 131 additions and 63 deletions.
diff --git a/pkg/asset/cluster/tfvars/tfvars.go b/pkg/asset/cluster/tfvars/tfvars.go
@@ -921,7 +921,7 @@ func (t *TerraformVariables) Generate(ctx context.Context, parents asset.Parents
 
 		cpStanza := installConfig.Config.ControlPlane
 		if cpStanza == nil || cpStanza.Platform.PowerVS == nil || cpStanza.Platform.PowerVS.SysType == "" {
-			sysTypes, err := powervs.AvailableSysTypes(installConfig.Config.PowerVS.Region)
+			sysTypes, err := powervs.AvailableSysTypes(installConfig.Config.PowerVS.Region, installConfig.Config.PowerVS.Zone)
 			if err != nil {
 				return err
 			}

diff --git a/pkg/asset/installconfig/platformprovisioncheck.go b/pkg/asset/installconfig/platformprovisioncheck.go
@@ -157,7 +157,7 @@ func (a *PlatformProvisionCheck) Generate(ctx context.Context, dependencies asse
 			return err
 		}
 
-		err = powervsconfig.ValidateSystemTypeForRegion(client, ic.Config)
+		err = powervsconfig.ValidateSystemTypeForZone(client, ic.Config)
 		if err != nil {
 			return err
 		}

diff --git a/pkg/asset/installconfig/powervs/regions.go b/pkg/asset/installconfig/powervs/regions.go
@@ -30,7 +30,11 @@ func IsKnownRegion(region string) bool {
 }
 
 func knownZones(region string) []string {
-	return powervs.Regions[region].Zones
+	zones := make([]string, 0, len(powervs.Regions[region].Zones))
+	for z := range powervs.Regions[region].Zones {
+		zones = append(zones, z)
+	}
+	return zones
 }
 
 // IsKnownZone return true is a specified zone is Known to the installer.

diff --git a/pkg/asset/installconfig/powervs/validation.go b/pkg/asset/installconfig/powervs/validation.go
@@ -255,14 +255,14 @@ func ValidateResourceGroup(client API, ic *types.InstallConfig) error {
 	return nil
 }
 
-// ValidateSystemTypeForRegion checks if the specified sysType is available in the target region.
-func ValidateSystemTypeForRegion(client API, ic *types.InstallConfig) error {
+// ValidateSystemTypeForZone checks if the specified sysType is available in the target zone.
+func ValidateSystemTypeForZone(client API, ic *types.InstallConfig) error {
 	if ic.ControlPlane == nil || ic.ControlPlane.Platform.PowerVS == nil || ic.ControlPlane.Platform.PowerVS.SysType == "" {
 		return nil
 	}
-	availableOnes, err := powervstypes.AvailableSysTypes(ic.PowerVS.Region)
+	availableOnes, err := powervstypes.AvailableSysTypes(ic.PowerVS.Region, ic.PowerVS.Zone)
 	if err != nil {
-		return fmt.Errorf("failed to obtain available SysTypes for: %s", ic.PowerVS.Region)
+		return fmt.Errorf("failed to obtain available SysTypes for: %s", ic.PowerVS.Zone)
 	}
 	requested := ic.ControlPlane.Platform.PowerVS.SysType
 	found := false
@@ -275,7 +275,7 @@ func ValidateSystemTypeForRegion(client API, ic *types.InstallConfig) error {
 	if found {
 		return nil
 	}
-	return fmt.Errorf("%s is not available in: %s", requested, ic.PowerVS.Region)
+	return fmt.Errorf("%s is not available in: %s", requested, ic.PowerVS.Zone)
 }
 
 // ValidateServiceInstance validates the optional service instance GUID in our install config.

diff --git a/pkg/asset/installconfig/powervs/validation_test.go b/pkg/asset/installconfig/powervs/validation_test.go
@@ -41,7 +41,7 @@ var (
 		validPrivateSubnetUSSouth2ID,
 	}
 	validUserID = "valid-user@example.com"
-	validZone   = "dal10"
+	validZone   = "dal12"
 
 	existingDNSRecordsResponse = []powervs.DNSRecordResponse{
 		{
@@ -132,7 +132,7 @@ var (
 	}
 	defaultSysType           = "s922"
 	newSysType               = "s1022"
-	invalidRegion            = "foo"
+	invalidZone              = "dal11"
 	validServiceInstanceGUID = ""
 )
 
@@ -561,22 +561,22 @@ func TestValidatePERAvailability(t *testing.T) {
 	}
 }
 
-func TestValidateSystemTypeForRegion(t *testing.T) {
+func TestValidateSystemTypeForZone(t *testing.T) {
 	cases := []struct {
 		name     string
 		edits    editFunctions
 		errorMsg string
 	}{
 		{
-			name: "Unknown Region specified",
+			name: "Unknown Zone specified",
 			edits: editFunctions{
 				func(ic *types.InstallConfig) {
-					ic.Platform.PowerVS.Region = invalidRegion
+					ic.Platform.PowerVS.Zone = invalidZone
 					ic.ControlPlane.Platform.PowerVS = validMachinePool()
 					ic.ControlPlane.Platform.PowerVS.SysType = defaultSysType
 				},
 			},
-			errorMsg: fmt.Sprintf("failed to obtain available SysTypes for: %s", invalidRegion),
+			errorMsg: fmt.Sprintf("failed to obtain available SysTypes for: %s", invalidZone),
 		},
 		{
 			name: "No Platform block",
@@ -597,21 +597,23 @@ func TestValidateSystemTypeForRegion(t *testing.T) {
 			errorMsg: "",
 		},
 		{
-			name: "Unavailable SysType specified for Dallas Region",
+			name: "Unavailable SysType specified for dal12 zone",
 			edits: editFunctions{
 				func(ic *types.InstallConfig) {
 					ic.Platform.PowerVS.Region = validRegion
+					ic.Platform.PowerVS.Zone = validZone
 					ic.ControlPlane.Platform.PowerVS = validMachinePool()
 					ic.ControlPlane.Platform.PowerVS.SysType = newSysType
 				},
 			},
-			errorMsg: fmt.Sprintf("%s is not available in: %s", newSysType, validRegion),
+			errorMsg: fmt.Sprintf("%s is not available in: %s", newSysType, validZone),
 		},
 		{
-			name: "Good Region/SysType combo specified",
+			name: "Good Zone/SysType combo specified",
 			edits: editFunctions{
 				func(ic *types.InstallConfig) {
 					ic.Platform.PowerVS.Region = validRegion
+					ic.Platform.PowerVS.Zone = validZone
 					ic.ControlPlane.Platform.PowerVS = validMachinePool()
 					ic.ControlPlane.Platform.PowerVS.SysType = defaultSysType
 				},
@@ -634,7 +636,7 @@ func TestValidateSystemTypeForRegion(t *testing.T) {
 				edit(editedInstallConfig)
 			}
 
-			aggregatedErrors := powervs.ValidateSystemTypeForRegion(powervsClient, editedInstallConfig)
+			aggregatedErrors := powervs.ValidateSystemTypeForZone(powervsClient, editedInstallConfig)
 			if tc.errorMsg != "" {
 				assert.Regexp(t, tc.errorMsg, aggregatedErrors)
 			} else {

diff --git a/pkg/asset/machines/worker.go b/pkg/asset/machines/worker.go
@@ -187,11 +187,11 @@ func defaultPowerVSMachinePoolPlatform(ic *types.InstallConfig) powervstypes.Mac
 		SysType:    "s922",
 	}
 
-	sysTypes, err = powervstypes.AvailableSysTypes(ic.PowerVS.Region)
+	sysTypes, err = powervstypes.AvailableSysTypes(ic.PowerVS.Region, ic.PowerVS.Zone)
 	if err == nil {
 		defaultMp.SysType = sysTypes[0]
 	} else {
-		logrus.Warnf("For given region %v, AvailableSysTypes returns %v", ic.PowerVS.Region, err)
+		logrus.Warnf("For given zone %v, AvailableSysTypes returns %v", ic.PowerVS.Zone, err)
 	}
 
 	return defaultMp