From b107504dfabd7d139fb4630c6bd06b3a32c0a3fb Mon Sep 17 00:00:00 2001 From: Ashwin Hendre Date: Fri, 22 Nov 2024 11:41:16 +0530 Subject: [PATCH] Create region-zone-sysType hierarchy --- pkg/asset/cluster/tfvars.go | 10 + .../installconfig/platformprovisioncheck.go | 5 + pkg/asset/installconfig/powervs/regions.go | 6 +- pkg/asset/installconfig/powervs/validation.go | 25 ++- .../installconfig/powervs/validation_test.go | 140 ++++++++++++- pkg/asset/machines/worker.go | 9 + pkg/types/powervs/powervs_regions.go | 186 ++++++++++++++---- 7 files changed, 341 insertions(+), 40 deletions(-) diff --git a/pkg/asset/cluster/tfvars.go b/pkg/asset/cluster/tfvars.go index f1b066373ab..c2cc6b4f819 100644 --- a/pkg/asset/cluster/tfvars.go +++ b/pkg/asset/cluster/tfvars.go @@ -875,6 +875,16 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error { err = powervsconfig.ValidatePERAvailability(client, installConfig.Config) transitGatewayEnabled := err == nil + cpStanza := installConfig.Config.ControlPlane + if cpStanza == nil || cpStanza.Platform.PowerVS == nil || cpStanza.Platform.PowerVS.SysType == "" { + sysTypes, err := powervs.AvailableSysTypes(installConfig.Config.PowerVS.Region, installConfig.Config.PowerVS.Zone) + if err != nil { + return err + } + for i := range masters { + masterConfigs[i].SystemType = sysTypes[0] + } + } serviceInstanceCRN, err := client.ServiceInstanceIDToCRN(ctx, installConfig.Config.PowerVS.ServiceInstanceID) if err != nil { diff --git a/pkg/asset/installconfig/platformprovisioncheck.go b/pkg/asset/installconfig/platformprovisioncheck.go index 8d34b3b7c5b..2e2b5510f28 100644 --- a/pkg/asset/installconfig/platformprovisioncheck.go +++ b/pkg/asset/installconfig/platformprovisioncheck.go @@ -175,6 +175,11 @@ func (a *PlatformProvisionCheck) Generate(dependencies asset.Parents) error { return err } + err = powervsconfig.ValidateSystemTypeForZone(client, ic.Config) + if err != nil { + return err + } + err = powervsconfig.ValidateServiceInstance(client, ic.Config) if err != nil { return err diff --git a/pkg/asset/installconfig/powervs/regions.go b/pkg/asset/installconfig/powervs/regions.go index 26d9426eef4..91873252894 100644 --- a/pkg/asset/installconfig/powervs/regions.go +++ b/pkg/asset/installconfig/powervs/regions.go @@ -31,7 +31,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 index c6c0534fcb3..79df6c89948 100644 --- a/pkg/asset/installconfig/powervs/validation.go +++ b/pkg/asset/installconfig/powervs/validation.go @@ -263,7 +263,30 @@ func ValidateResourceGroup(client API, ic *types.InstallConfig) error { return nil } -// ValidateServiceInstance validates the service instance in our install config. +// 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, ic.PowerVS.Zone) + if err != nil { + return fmt.Errorf("failed to obtain available SysTypes for: %s", ic.PowerVS.Zone) + } + requested := ic.ControlPlane.Platform.PowerVS.SysType + found := false + for i := range availableOnes { + if requested == availableOnes[i] { + found = true + break + } + } + if found { + return nil + } + return fmt.Errorf("%s is not available in: %s", requested, ic.PowerVS.Zone) +} + +// ValidateServiceInstance validates the optional service instance GUID in our install config. func ValidateServiceInstance(client API, ic *types.InstallConfig) error { ctx, cancel := context.WithTimeout(context.TODO(), 5*time.Minute) defer cancel() diff --git a/pkg/asset/installconfig/powervs/validation_test.go b/pkg/asset/installconfig/powervs/validation_test.go index 34b9c488817..6d5563f0535 100644 --- a/pkg/asset/installconfig/powervs/validation_test.go +++ b/pkg/asset/installconfig/powervs/validation_test.go @@ -45,7 +45,7 @@ var ( validPrivateSubnetUSSouth2ID, } validUserID = "valid-user@example.com" - validZone = "dal10" + validZone = "dal12" existingDNSRecordsResponse = []powervs.DNSRecordResponse{ { @@ -134,6 +134,10 @@ var ( "disaster-recover-site": true, "power-vpn-connections": false, } + defaultSysType = "s922" + newSysType = "s1022" + invalidZone = "dal11" + validServiceInstanceGUID = "" ) func validInstallConfig() *types.InstallConfig { @@ -734,6 +738,140 @@ func TestValidatePERAvailability(t *testing.T) { } } +func TestValidateSystemTypeForZone(t *testing.T) { + cases := []struct { + name string + edits editFunctions + errorMsg string + }{ + { + name: "Unknown Zone specified", + edits: editFunctions{ + func(ic *types.InstallConfig) { + 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", invalidZone), + }, + { + name: "No Platform block", + edits: editFunctions{ + func(ic *types.InstallConfig) { + ic.ControlPlane.Platform.PowerVS = nil + }, + }, + errorMsg: "", + }, + { + name: "Structure present, but no SysType specified", + edits: editFunctions{ + func(ic *types.InstallConfig) { + ic.ControlPlane.Platform.PowerVS = validMachinePool() + }, + }, + errorMsg: "", + }, + { + 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, validZone), + }, + { + 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 + }, + }, + errorMsg: "", + }, + } + setMockEnvVars() + + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + powervsClient := mock.NewMockAPI(mockCtrl) + + // Run tests + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + editedInstallConfig := validInstallConfig() + for _, edit := range tc.edits { + edit(editedInstallConfig) + } + + aggregatedErrors := powervs.ValidateSystemTypeForZone(powervsClient, editedInstallConfig) + if tc.errorMsg != "" { + assert.Regexp(t, tc.errorMsg, aggregatedErrors) + } else { + assert.NoError(t, aggregatedErrors) + } + }) + } +} + +func TestValidateServiceInstance(t *testing.T) { + cases := []struct { + name string + edits editFunctions + errorMsg string + }{ + { + name: "valid install config", + edits: editFunctions{}, + errorMsg: "", + }, + { + name: "invalid install config", + edits: editFunctions{ + func(ic *types.InstallConfig) { + ic.Platform.PowerVS.ServiceInstanceGUID = "invalid-uuid" + }, + }, + errorMsg: "platform:powervs:serviceInstanceGUID has an invalid guid", + }, + } + setMockEnvVars() + + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + powervsClient := mock.NewMockAPI(mockCtrl) + + // FIX: Unexpected call to *mock.MockAPI.ListServiceInstances([context.TODO.WithDeadline(2023-12-02 08:38:15.542340268 -0600 CST m=+300.012357408 [4m59.999979046s])]) at validation.go:289 because: there are no expected calls of the method "ListServiceInstances" for that receiver + powervsClient.EXPECT().ListServiceInstances(gomock.Any()).AnyTimes() + + // Run tests + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + editedInstallConfig := validInstallConfig() + for _, edit := range tc.edits { + edit(editedInstallConfig) + } + + aggregatedErrors := powervs.ValidateServiceInstance(powervsClient, editedInstallConfig) + if tc.errorMsg != "" { + assert.Regexp(t, tc.errorMsg, aggregatedErrors) + } else { + assert.NoError(t, aggregatedErrors) + } + }) + } +} + func setMockEnvVars() { os.Setenv("POWERVS_AUTH_FILEPATH", "./tmp/powervs/config.json") os.Setenv("IBMID", "foo") diff --git a/pkg/asset/machines/worker.go b/pkg/asset/machines/worker.go index 39ed23ae15d..9de23262736 100644 --- a/pkg/asset/machines/worker.go +++ b/pkg/asset/machines/worker.go @@ -186,6 +186,15 @@ func defaultPowerVSMachinePoolPlatform() powervstypes.MachinePool { ProcType: machinev1.PowerVSProcessorTypeShared, SysType: "s922", } + + sysTypes, err = powervstypes.AvailableSysTypes(ic.PowerVS.Region, ic.PowerVS.Zone) + if err == nil { + defaultMp.SysType = sysTypes[0] + } else { + logrus.Warnf("For given zone %v, AvailableSysTypes returns %v", ic.PowerVS.Zone, err) + } + + return defaultMp } func defaultNutanixMachinePoolPlatform() nutanixtypes.MachinePool { diff --git a/pkg/types/powervs/powervs_regions.go b/pkg/types/powervs/powervs_regions.go index b7fe244cde7..63fe809bdd6 100644 --- a/pkg/types/powervs/powervs_regions.go +++ b/pkg/types/powervs/powervs_regions.go @@ -12,7 +12,14 @@ import ( type Region struct { Description string VPCRegion string - Zones []string + COSRegion string + Zones map[string]Zone + VPCZones []string +} + +// Zone holds the sysTypes for a zone in a IBM Power VS region. +type Zone struct { + SysTypes []string } // Regions holds the regions for IBM Power VS, and descriptions used during the survey. @@ -20,64 +27,105 @@ var Regions = map[string]Region{ "dal": { Description: "Dallas, USA", VPCRegion: "us-south", - Zones: []string{ - "dal10", - "dal12", + COSRegion: "us-south", + Zones: map[string]Zone{ + "dal10": { + SysTypes: []string{"s922", "s1022", "e980", "e1080"}, + }, + "dal12": { + SysTypes: []string{"s922", "e980"}, + }, }, + VPCZones: []string{"us-south-1", "us-south-2", "us-south-3"}, }, "eu-de": { Description: "Frankfurt, Germany", VPCRegion: "eu-de", - Zones: []string{ - "eu-de-1", - "eu-de-2", + COSRegion: "eu-de", + Zones: map[string]Zone{ + "eu-de-1": { + SysTypes: []string{"s922", "s1022", "e980"}, + }, + "eu-de-2": { + SysTypes: []string{"s922", "e980"}, + }, }, + VPCZones: []string{"eu-de-1", "eu-de-2", "eu-de-3"}, }, "lon": { Description: "London, UK.", VPCRegion: "eu-gb", - Zones: []string{ - "lon04", - "lon06", + COSRegion: "eu-gb", + Zones: map[string]Zone{ + "lon06": { + SysTypes: []string{"s922", "e980"}, + }, }, + VPCZones: []string{"eu-gb-1", "eu-gb-2", "eu-gb-3"}, }, - "mon": { - Description: "Montreal, Canada", - VPCRegion: "ca-tor", - Zones: []string{"mon01"}, + "mad": { + Description: "Madrid, Spain", + VPCRegion: "eu-es", + COSRegion: "eu-de", // @HACK - PowerVS says COS not supported in this region + Zones: map[string]Zone{ + "mad02": { + SysTypes: []string{"s922", "s1022", "e980"}, + }, + "mad04": { + SysTypes: []string{"s1022", "e980", "e1080"}, + }, + }, + VPCZones: []string{"eu-es-1", "eu-es-2"}, }, "osa": { Description: "Osaka, Japan", VPCRegion: "jp-osa", - Zones: []string{"osa21"}, - }, - "syd": { - Description: "Sydney, Australia", - VPCRegion: "au-syd", - Zones: []string{ - "syd04", - "syd05", + COSRegion: "jp-osa", + Zones: map[string]Zone{ + "osa21": { + SysTypes: []string{"s922", "s1022", "e980"}, + }, }, + VPCZones: []string{"jp-osa-1", "jp-osa-2", "jp-osa-3"}, }, "sao": { Description: "São Paulo, Brazil", VPCRegion: "br-sao", - Zones: []string{"sao01"}, - }, - "tor": { - Description: "Toronto, Canada", - VPCRegion: "ca-tor", - Zones: []string{"tor01"}, + COSRegion: "br-sao", + Zones: map[string]Zone{ + "sao01": { + SysTypes: []string{"s922", "e980"}, + }, + "sao04": { + SysTypes: []string{"s922", "e980"}, + }, + }, + VPCZones: []string{"br-sao-1", "br-sao-2", "br-sao-3"}, }, - "tok": { - Description: "Tokyo, Japan", - VPCRegion: "jp-tok", - Zones: []string{"tok04"}, + "syd": { + Description: "Sydney, Australia", + VPCRegion: "au-syd", + COSRegion: "au-syd", + Zones: map[string]Zone{ + "syd04": { + SysTypes: []string{"s922", "e980"}, + }, + }, + VPCZones: []string{"au-syd-1", "au-syd-2", "au-syd-3"}, }, - "us-east": { + "wdc": { Description: "Washington DC, USA", VPCRegion: "us-east", - Zones: []string{"us-east"}, + COSRegion: "us-east", + Zones: map[string]Zone{ + "wdc06": { + SysTypes: []string{"s922", "e980"}, + }, + "wdc07": { + SysTypes: []string{"s922", "s1022", "e980", "e1080"}, + }, + }, + VPCZones: []string{"us-east-1", "us-east-2", "us-east-3"}, }, } @@ -117,7 +165,7 @@ func ValidateVPCRegion(region string) bool { func ValidateZone(zone string) bool { for r := range Regions { for z := range Regions[r].Zones { - if zone == Regions[r].Zones[z] { + if zone == z { return true } } @@ -130,7 +178,7 @@ func ZoneNames() []string { zones := []string{} for r := range Regions { for z := range Regions[r].Zones { - zones = append(zones, Regions[r].Zones[z]) + zones = append(zones, z) } } return zones @@ -140,10 +188,74 @@ func ZoneNames() []string { func RegionFromZone(zone string) string { for r := range Regions { for z := range Regions[r].Zones { - if zone == Regions[r].Zones[z] { + if zone == z { return r } } } return "" } + +// AvailableSysTypes returns the default system type for the zone. +func AvailableSysTypes(region string, zone string) ([]string, error) { + knownRegion, ok := Regions[region] + if !ok { + return nil, fmt.Errorf("unknown region name provided") + } + var knownZone Zone + knownZone, ok = knownRegion.Zones[zone] + if !ok { + return nil, fmt.Errorf("unknown zone name provided") + } + return knownZone.SysTypes, nil +} + +// AllKnownSysTypes returns aggregated known system types from all regions. +func AllKnownSysTypes() sets.Set[string] { + sysTypes := sets.New[string]() + for region := range Regions { + for _, zones := range Regions[region].Zones { + sysTypes.Insert(zones.SysTypes...) + } + } + return sysTypes +} + +// AvailableVPCZones returns the known VPC zones for a specified region. +func AvailableVPCZones(region string) ([]string, error) { + knownRegion, ok := Regions[region] + if !ok { + return nil, fmt.Errorf("unknown region name provided") + } + return knownRegion.VPCZones, nil +} + +// COSRegionForVPCRegion returns the corresponding COS region for the given VPC region. +func COSRegionForVPCRegion(vpcRegion string) (string, error) { + for r := range Regions { + if vpcRegion == Regions[r].VPCRegion { + return Regions[r].COSRegion, nil + } + } + + return "", fmt.Errorf("COS region corresponding to a VPC region %s not found ", vpcRegion) +} + +// COSRegionForPowerVSRegion returns the IBM COS region for the specified PowerVS region. +func COSRegionForPowerVSRegion(region string) (string, error) { + if r, ok := Regions[region]; ok { + return r.COSRegion, nil + } + + return "", fmt.Errorf("COS region corresponding to a PowerVS region %s not found ", region) +} + +// ValidateCOSRegion validates that given COS region is known/tested. +func ValidateCOSRegion(region string) bool { + for r := range Regions { + if region == Regions[r].COSRegion { + return true + } + } + return false +}