diff --git a/.golangci.yaml b/.golangci.yaml index 3b597b1aeaa..f2b4d7db122 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -33,7 +33,6 @@ linters: - revive - staticcheck - stylecheck - - tenv - thelper - typecheck - unconvert @@ -148,6 +147,8 @@ linters-settings: - name: redefines-builtin-id - name: bool-literal-in-expr - name: constant-logical-expr + gocyclo: + min-complexity: 45 issues: include: - EXC0012 # EXC0012 revive: issue about not having a comment on exported. diff --git a/pkg/asset/cluster/tfvars/tfvars.go b/pkg/asset/cluster/tfvars/tfvars.go index 4e9ec119c1e..45b5437c96d 100644 --- a/pkg/asset/cluster/tfvars/tfvars.go +++ b/pkg/asset/cluster/tfvars/tfvars.go @@ -373,14 +373,6 @@ func (t *TerraformVariables) Generate(ctx context.Context, parents asset.Parents for i, w := range workers { workerConfigs[i] = w.Spec.Template.Spec.ProviderSpec.Value.Object.(*machinev1beta1.AzureMachineProviderSpec) //nolint:errcheck // legacy, pre-linter } - client, err := installConfig.Azure.Client() - if err != nil { - return err - } - hyperVGeneration, err := client.GetHyperVGenerationVersion(ctx, masterConfigs[0].VMSize, masterConfigs[0].Location, "") - if err != nil { - return err - } preexistingnetwork := installConfig.Config.Azure.VirtualNetwork != "" @@ -427,7 +419,6 @@ func (t *TerraformVariables) Generate(ctx context.Context, parents asset.Parents OutboundType: installConfig.Config.Azure.OutboundType, BootstrapIgnStub: bootstrapIgnStub, BootstrapIgnitionURLPlaceholder: bootstrapIgnURLPlaceholder, - HyperVGeneration: hyperVGeneration, VMArchitecture: installConfig.Config.ControlPlane.Architecture, InfrastructureName: clusterID.InfraID, KeyVault: managedKeys.KeyVault, diff --git a/pkg/asset/installconfig/azure/client.go b/pkg/asset/installconfig/azure/client.go index da45904fed8..e1376195283 100644 --- a/pkg/asset/installconfig/azure/client.go +++ b/pkg/asset/installconfig/azure/client.go @@ -38,7 +38,6 @@ type API interface { ListResourceIDsByGroup(ctx context.Context, groupName string) ([]string, error) GetStorageEndpointSuffix(ctx context.Context) (string, error) GetDiskEncryptionSet(ctx context.Context, subscriptionID, groupName string, diskEncryptionSetName string) (*azenc.DiskEncryptionSet, error) - GetHyperVGenerationVersion(ctx context.Context, instanceType string, region string, imageHyperVGen string) (string, error) GetMarketplaceImage(ctx context.Context, region, publisher, offer, sku, version string) (azenc.VirtualMachineImage, error) AreMarketplaceImageTermsAccepted(ctx context.Context, publisher, offer, sku string) (bool, error) GetVMCapabilities(ctx context.Context, instanceType, region string) (map[string]string, error) @@ -364,17 +363,6 @@ func (c *Client) GetVMCapabilities(ctx context.Context, instanceType, region str return capabilities, nil } -// GetHyperVGenerationVersion gets the HyperVGeneration version for the given instance type and marketplace image version, if specified. Defaults to V2 if either V1 or V2 -// available. -func (c *Client) GetHyperVGenerationVersion(ctx context.Context, instanceType string, region string, imageHyperVGen string) (version string, err error) { - capabilities, err := c.GetVMCapabilities(ctx, instanceType, region) - if err != nil { - return "", err - } - - return GetHyperVGenerationVersion(capabilities, imageHyperVGen) -} - // GetMarketplaceImage get the specified marketplace VM image. func (c *Client) GetMarketplaceImage(ctx context.Context, region, publisher, offer, sku, version string) (azenc.VirtualMachineImage, error) { client := azenc.NewVirtualMachineImagesClientWithBaseURI(c.ssn.Environment.ResourceManagerEndpoint, c.ssn.Credentials.SubscriptionID) diff --git a/pkg/asset/installconfig/azure/metadata.go b/pkg/asset/installconfig/azure/metadata.go index 59536706a4f..7311d4a3cfb 100644 --- a/pkg/asset/installconfig/azure/metadata.go +++ b/pkg/asset/installconfig/azure/metadata.go @@ -8,7 +8,9 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + "github.com/openshift/installer/pkg/types" typesazure "github.com/openshift/installer/pkg/types/azure" + azuredefaults "github.com/openshift/installer/pkg/types/azure/defaults" ) // Metadata holds additional metadata for InstallConfig resources that @@ -23,6 +25,13 @@ type Metadata struct { region string ZonesSubnetMap map[string][]string + controlPlane *types.MachinePool + compute *types.MachinePool + defaultPlatform *typesazure.MachinePool + + controlPlaneCapabilities map[string]string + computeCapabilities map[string]string + // CloudName indicates the Azure cloud environment (e.g. public, gov't). CloudName typesazure.CloudEnvironment `json:"cloudName,omitempty"` @@ -42,18 +51,21 @@ type Metadata struct { } // NewMetadata initializes a new Metadata object. -func NewMetadata(cloudName typesazure.CloudEnvironment, armEndpoint string, region string) *Metadata { - return NewMetadataWithCredentials(cloudName, armEndpoint, nil, region) +func NewMetadata(az *typesazure.Platform, controlPlane, compute *types.MachinePool) *Metadata { + return NewMetadataWithCredentials(az, controlPlane, compute, nil) } // NewMetadataWithCredentials initializes a new Metadata object // with prepopulated Azure credentials. -func NewMetadataWithCredentials(cloudName typesazure.CloudEnvironment, armEndpoint string, credentials *Credentials, region string) *Metadata { +func NewMetadataWithCredentials(az *typesazure.Platform, controlPlane, compute *types.MachinePool, credentials *Credentials) *Metadata { return &Metadata{ - CloudName: cloudName, - ARMEndpoint: armEndpoint, - Credentials: credentials, - region: region, + CloudName: az.CloudName, + ARMEndpoint: az.ARMEndpoint, + Credentials: credentials, + controlPlane: controlPlane, + compute: compute, + region: az.Region, + defaultPlatform: az.DefaultMachinePlatform, } } @@ -198,3 +210,73 @@ func (m *Metadata) GenerateZonesSubnetMap(subnetSpec []typesazure.SubnetSpec, de } return m.ZonesSubnetMap, nil } + +// ControlPlaneCapabilities returns the capabilities for the instance type of control-plane +// nodes from the Azure API. +func (m *Metadata) ControlPlaneCapabilities() (map[string]string, error) { + if m.controlPlaneCapabilities == nil { + caps, err := m.getCapabilities(m.controlPlane, azuredefaults.ControlPlaneInstanceType) + if err != nil { + return nil, fmt.Errorf("failed to get control plane capabilities: %w", err) + } + m.controlPlaneCapabilities = caps + } + return m.controlPlaneCapabilities, nil +} + +// ComputeCapabilities returns the capabilities for the instance type of compute nodes +// from the Azure API. +func (m *Metadata) ComputeCapabilities() (map[string]string, error) { + if m.computeCapabilities == nil { + caps, err := m.getCapabilities(m.compute, azuredefaults.ComputeInstanceType) + if err != nil { + return nil, fmt.Errorf("failed to get compute capabilities: %w", err) + } + m.computeCapabilities = caps + } + return m.computeCapabilities, nil +} + +// ControlPlaneHyperVGeneration returns the HyperVGeneration for the control-plane +// instances. If the instance type supports both V1 & V2, V2 is returned. +func (m *Metadata) ControlPlaneHyperVGeneration() (string, error) { + caps, err := m.ControlPlaneCapabilities() + if err != nil { + return "", fmt.Errorf("unable to get control plane capabilities: %w", err) + } + return GetHyperVGenerationVersion(caps, "") +} + +// ComputeHyperVGeneration returns the HyperVGeneration for the compute instances. +// If the instance type supports both V1 & V2, V2 is returned. +func (m *Metadata) ComputeHyperVGeneration() (string, error) { + caps, err := m.ComputeCapabilities() + if err != nil { + return "", fmt.Errorf("unable to get compute capabilities: %w", err) + } + return GetHyperVGenerationVersion(caps, "") +} + +type machinePoolInstanceTypeFunc func(typesazure.CloudEnvironment, string, types.Architecture) string + +func (m *Metadata) getCapabilities(mPool *types.MachinePool, defaultInstanceType machinePoolInstanceTypeFunc) (map[string]string, error) { + if mPool == nil { + return nil, fmt.Errorf("unable to get capabilities because machinepool is not populated in metadata") + } + instType := defaultInstanceType(m.CloudName, m.region, mPool.Architecture) + if dmp := m.defaultPlatform; dmp != nil && dmp.InstanceType != "" { + instType = dmp.InstanceType + } + if mPool.Platform.Azure != nil && mPool.Platform.Azure.InstanceType != "" { + instType = mPool.Platform.Azure.InstanceType + } + client, err := m.Client() + if err != nil { + return nil, fmt.Errorf("failed to get azure client %w", err) + } + caps, err := client.GetVMCapabilities(context.TODO(), instType, m.region) + if err != nil { + return nil, err + } + return caps, nil +} diff --git a/pkg/asset/installconfig/azure/mock/azureclient_generated.go b/pkg/asset/installconfig/azure/mock/azureclient_generated.go index 77e286c1b18..ee62a871bc2 100644 --- a/pkg/asset/installconfig/azure/mock/azureclient_generated.go +++ b/pkg/asset/installconfig/azure/mock/azureclient_generated.go @@ -193,21 +193,6 @@ func (mr *MockAPIMockRecorder) GetGroup(ctx, groupName any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGroup", reflect.TypeOf((*MockAPI)(nil).GetGroup), ctx, groupName) } -// GetHyperVGenerationVersion mocks base method. -func (m *MockAPI) GetHyperVGenerationVersion(ctx context.Context, instanceType, region, imageHyperVGen string) (string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetHyperVGenerationVersion", ctx, instanceType, region, imageHyperVGen) - ret0, _ := ret[0].(string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetHyperVGenerationVersion indicates an expected call of GetHyperVGenerationVersion. -func (mr *MockAPIMockRecorder) GetHyperVGenerationVersion(ctx, instanceType, region, imageHyperVGen any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetHyperVGenerationVersion", reflect.TypeOf((*MockAPI)(nil).GetHyperVGenerationVersion), ctx, instanceType, region, imageHyperVGen) -} - // GetLocationInfo mocks base method. func (m *MockAPI) GetLocationInfo(ctx context.Context, region, instanceType string) (*compute.ResourceSkuLocationInfo, error) { m.ctrl.T.Helper() diff --git a/pkg/asset/installconfig/azure/validation.go b/pkg/asset/installconfig/azure/validation.go index 269a08ef445..6b4f58a5fe9 100644 --- a/pkg/asset/installconfig/azure/validation.go +++ b/pkg/asset/installconfig/azure/validation.go @@ -5,7 +5,6 @@ import ( "fmt" "net" "net/url" - "os" "slices" "sort" "strconv" @@ -53,7 +52,7 @@ var computeReq = resourceRequirements{ } // Validate executes platform-specific validation. -func Validate(client API, ic *types.InstallConfig) error { +func Validate(client API, meta *Metadata, ic *types.InstallConfig) error { allErrs := field.ErrorList{} allErrs = append(allErrs, validateNetworks(client, ic.Azure, field.NewPath("platform").Child("azure"))...) @@ -61,7 +60,7 @@ func Validate(client API, ic *types.InstallConfig) error { if ic.Azure.CloudName == aztypes.StackCloud { allErrs = append(allErrs, validateAzureStackDiskType(client, ic)...) } - allErrs = append(allErrs, validateInstanceTypes(client, ic)...) + allErrs = append(allErrs, validateInstanceTypes(client, meta, ic)...) if ic.Azure.CloudName == aztypes.StackCloud && ic.Azure.ClusterOSImage != "" { StorageEndpointSuffix, err := client.GetStorageEndpointSuffix(context.TODO()) if err != nil { @@ -69,7 +68,7 @@ func Validate(client API, ic *types.InstallConfig) error { } allErrs = append(allErrs, validateAzureStackClusterOSImage(StorageEndpointSuffix, ic.Azure.ClusterOSImage, field.NewPath("platform").Child("azure"))...) } - allErrs = append(allErrs, validateMarketplaceImages(client, ic)...) + allErrs = append(allErrs, validateMarketplaceImages(client, meta, ic)...) allErrs = append(allErrs, validateBootDiagnostics(client, ic)...) allErrs = append(allErrs, validateCustomSubnets(client, field.NewPath("platform").Child("azure").Child("subnetSpec"), ic)...) return allErrs.ToAggregate() @@ -353,12 +352,15 @@ func validateUltraSSD(client API, fieldPath *field.Path, icZones []string, regio } // ValidateInstanceType ensures the instance type has sufficient Vcpu, Memory, and a valid family type. -func ValidateInstanceType(client API, fieldPath *field.Path, region, instanceType, diskType string, req resourceRequirements, ultraSSDEnabled bool, vmNetworkingType string, icZones []string, architecture types.Architecture, securityType aztypes.SecurityTypes) field.ErrorList { +func ValidateInstanceType(client API, fieldPath *field.Path, region, instanceType, diskType string, req resourceRequirements, ultraSSDEnabled bool, vmNetworkingType string, icZones []string, architecture types.Architecture, securityType aztypes.SecurityTypes, capabilities map[string]string) field.ErrorList { allErrs := field.ErrorList{} - capabilities, err := client.GetVMCapabilities(context.TODO(), instanceType, region) - if err != nil { - return append(allErrs, field.Invalid(fieldPath.Child("type"), instanceType, err.Error())) + var err error + if capabilities == nil { + capabilities, err = client.GetVMCapabilities(context.TODO(), instanceType, region) + if err != nil { + return append(allErrs, field.Invalid(fieldPath.Child("type"), instanceType, err.Error())) + } } allErrs = append(allErrs, validateMininumRequirements(fieldPath.Child("type"), req, instanceType, capabilities)...) @@ -386,7 +388,7 @@ func ValidateInstanceType(client API, fieldPath *field.Path, region, instanceTyp } // validateInstanceTypes checks that the user-provided instance types are valid. -func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList { +func validateInstanceTypes(client API, meta *Metadata, ic *types.InstallConfig) field.ErrorList { allErrs := field.ErrorList{} var securityType aztypes.SecurityTypes @@ -451,7 +453,7 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList zones = defaultZones } ultraSSDEnabled := strings.EqualFold(ultraSSDCapability, "Enabled") - allErrs = append(allErrs, ValidateInstanceType(client, fieldPath, ic.Azure.Region, instanceType, diskType, controlPlaneReq, ultraSSDEnabled, vmNetworkingType, zones, architecture, securityType)...) + allErrs = append(allErrs, ValidateInstanceType(client, fieldPath, ic.Azure.Region, instanceType, diskType, controlPlaneReq, ultraSSDEnabled, vmNetworkingType, zones, architecture, securityType, meta.controlPlaneCapabilities)...) } for idx, compute := range ic.Compute { @@ -488,7 +490,7 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList } ultraSSDEnabled := strings.EqualFold(ultraSSDCapability, "Enabled") allErrs = append(allErrs, ValidateInstanceType(client, fieldPath.Child("platform", "azure"), - ic.Azure.Region, instanceType, diskType, computeReq, ultraSSDEnabled, vmNetworkingType, zones, architecture, securityType)...) + ic.Azure.Region, instanceType, diskType, computeReq, ultraSSDEnabled, vmNetworkingType, zones, architecture, securityType, meta.computeCapabilities)...) } } @@ -507,7 +509,7 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList fieldPath := field.NewPath("platform", "azure", "defaultMachinePlatform") ultraSSDEnabled := strings.EqualFold(defaultUltraSSDCapability, "Enabled") allErrs = append(allErrs, ValidateInstanceType(client, fieldPath, - ic.Azure.Region, defaultInstanceType, defaultDiskType, minReq, ultraSSDEnabled, defaultVMNetworkingType, defaultZones, architecture, securityType)...) + ic.Azure.Region, defaultInstanceType, defaultDiskType, minReq, ultraSSDEnabled, defaultVMNetworkingType, defaultZones, architecture, securityType, nil)...) } return allErrs } @@ -662,46 +664,12 @@ func ValidateForProvisioning(client API, ic *types.InstallConfig) error { allErrs = append(allErrs, validateResourceGroup(client, field.NewPath("platform").Child("azure"), ic.Azure)...) allErrs = append(allErrs, ValidateDiskEncryptionSet(client, ic)...) allErrs = append(allErrs, ValidateSecurityProfileDiskEncryptionSet(client, ic)...) - allErrs = append(allErrs, validateSkipImageUpload(field.NewPath("image"), ic)...) if ic.Azure.CloudName == aztypes.StackCloud { allErrs = append(allErrs, checkAzureStackClusterOSImageSet(ic.Azure.ClusterOSImage, field.NewPath("platform").Child("azure"))...) } return allErrs.ToAggregate() } -func validateSkipImageUpload(fieldPath *field.Path, ic *types.InstallConfig) field.ErrorList { - allErrs := field.ErrorList{} - defaultOSImage := aztypes.OSImage{} - if ic.Azure.DefaultMachinePlatform != nil { - defaultOSImage = aztypes.OSImage{ - Plan: ic.Azure.DefaultMachinePlatform.OSImage.Plan, - Publisher: ic.Azure.DefaultMachinePlatform.OSImage.Publisher, - SKU: ic.Azure.DefaultMachinePlatform.OSImage.SKU, - Version: ic.Azure.DefaultMachinePlatform.OSImage.Version, - Offer: ic.Azure.DefaultMachinePlatform.OSImage.Offer, - } - } - controlPlaneOSImage := defaultOSImage - if ic.ControlPlane.Platform.Azure != nil { - controlPlaneOSImage = ic.ControlPlane.Platform.Azure.OSImage - } - allErrs = append(allErrs, validateOSImage(fieldPath.Child("controlplane"), controlPlaneOSImage)...) - computeOSImage := defaultOSImage - if len(ic.Compute) > 0 && ic.Compute[0].Platform.Azure != nil { - computeOSImage = ic.Compute[0].Platform.Azure.OSImage - } - allErrs = append(allErrs, validateOSImage(fieldPath.Child("compute"), computeOSImage)...) - return allErrs -} -func validateOSImage(fieldPath *field.Path, osImage aztypes.OSImage) field.ErrorList { - if _, ok := os.LookupEnv("OPENSHIFT_INSTALL_SKIP_IMAGE_UPLOAD"); ok { - if len(osImage.SKU) > 0 { - return nil - } - return field.ErrorList{field.Invalid(fieldPath, "image", "cannot skip image upload without marketplace image specified")} - } - return nil -} func validateResourceGroup(client API, fieldPath *field.Path, platform *aztypes.Platform) field.ErrorList { allErrs := field.ErrorList{} if len(platform.ResourceGroupName) == 0 { @@ -766,7 +734,7 @@ func validateAzureStackClusterOSImage(StorageEndpointSuffix string, ClusterOSIma return allErrs } -func validateMarketplaceImages(client API, installConfig *types.InstallConfig) field.ErrorList { +func validateMarketplaceImages(client API, meta *Metadata, installConfig *types.InstallConfig) field.ErrorList { var allErrs field.ErrorList region := installConfig.Azure.Region @@ -796,7 +764,7 @@ func validateMarketplaceImages(client API, installConfig *types.InstallConfig) f instanceType = defaults.ControlPlaneInstanceType(cloudName, region, installConfig.ControlPlane.Architecture) } - capabilities, err := client.GetVMCapabilities(context.Background(), instanceType, region) + capabilities, err := meta.ControlPlaneCapabilities() if err != nil { allErrs = append(allErrs, field.Invalid(fldPath.Child("platform", "azure", "type"), instanceType, err.Error())) } @@ -838,7 +806,7 @@ func validateMarketplaceImages(client API, installConfig *types.InstallConfig) f instanceType = defaults.ComputeInstanceType(cloudName, region, compute.Architecture) } - capabilities, err := client.GetVMCapabilities(context.Background(), instanceType, region) + capabilities, err := meta.ComputeCapabilities() if err != nil { allErrs = append(allErrs, field.Invalid(fldPath.Child("platform", "azure", "type"), instanceType, err.Error())) continue diff --git a/pkg/asset/installconfig/azure/validation_test.go b/pkg/asset/installconfig/azure/validation_test.go index 2150d31ded5..400eb363792 100644 --- a/pkg/asset/installconfig/azure/validation_test.go +++ b/pkg/asset/installconfig/azure/validation_test.go @@ -427,6 +427,7 @@ func validInstallConfig() *types.InstallConfig { }, Compute: []types.MachinePool{{ Architecture: types.ArchitectureAMD64, + Name: types.MachinePoolComputeRoleName, Platform: types.MachinePoolPlatform{ Azure: &azure.MachinePool{}, }, @@ -503,7 +504,7 @@ func TestAzureInstallConfigValidation(t *testing.T) { { name: "Undefined default instance types", edits: editFunctions{undefinedDefaultInstanceTypes}, - errorMsg: `\[controlPlane.platform.azure.type: Invalid value: "Dne_D2_v4": not found in region centralus, compute\[0\].platform.azure.type: Invalid value: "Dne_D2_v4": not found in region centralus, controlPlane.platform.azure.type: Invalid value: "Dne_D2_v4": unable to determine HyperVGeneration version\]`, + errorMsg: `\[controlPlane.platform.azure.type: Invalid value: "Dne_D2_v4": not found in region centralus, compute\[0\].platform.azure.type: Invalid value: "Dne_D2_v4": not found in region centralus, controlPlane.platform.azure.type: Invalid value: "Dne_D2_v4": failed to get control plane capabilities: not found in region centralus, controlPlane.platform.azure.type: Invalid value: "Dne_D2_v4": unable to determine HyperVGeneration version, compute\[0\].platform.azure.type: Invalid value: "Dne_D2_v4": failed to get compute capabilities: not found in region centralus\]`, }, { name: "Invalid compute instance types", @@ -669,12 +670,13 @@ func TestAzureInstallConfigValidation(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - editedInstallConfig := validInstallConfig() + ic := validInstallConfig() for _, edit := range tc.edits { - edit(editedInstallConfig) + edit(ic) } - - aggregatedErrors := Validate(azureClient, editedInstallConfig) + metadata := NewMetadata(ic.Azure, ic.ControlPlane, ic.WorkerMachinePool()) + metadata.UseMockClient(azureClient) + aggregatedErrors := Validate(azureClient, metadata, ic) if tc.errorMsg != "" { assert.Regexp(t, tc.errorMsg, aggregatedErrors) } else { @@ -1225,12 +1227,13 @@ func TestAzureUltraSSDCapability(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - editedInstallConfig := validInstallConfig() + ic := validInstallConfig() for _, edit := range tc.edits { - edit(editedInstallConfig) + edit(ic) } - - aggregatedErrors := Validate(azureClient, editedInstallConfig) + metadata := NewMetadata(ic.Azure, ic.ControlPlane, ic.WorkerMachinePool()) + metadata.UseMockClient(azureClient) + aggregatedErrors := Validate(azureClient, metadata, ic) if tc.errorMsg != "" { assert.Regexp(t, tc.errorMsg, aggregatedErrors) } else { @@ -1534,17 +1537,15 @@ func TestAzureMarketplaceImages(t *testing.T) { azureClient.EXPECT().GetVMCapabilities(gomock.Any(), "Dne_D2_v4", validRegion).Return(nil, fmt.Errorf("not found in region centralus")).AnyTimes() azureClient.EXPECT().GetVMCapabilities(gomock.Any(), gomock.Any(), gomock.Any()).Return(vmCapabilities["Standard_D8s_v3"], nil).AnyTimes() - // HyperVGenerations - azureClient.EXPECT().GetHyperVGenerationVersion(gomock.Any(), gomock.Any(), gomock.Any(), "V1").Return("", fmt.Errorf("instance type Standard_D8s_v3 supports HyperVGenerations [V2] but the specified image is for HyperVGeneration V1; to correct this issue either specify a compatible instance type or change the HyperVGeneration for the image by using a different SKU")).AnyTimes() - azureClient.EXPECT().GetHyperVGenerationVersion(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return("V2", nil).AnyTimes() - for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - editedInstallConfig := validInstallConfig() + ic := validInstallConfig() for _, edit := range tc.edits { - edit(editedInstallConfig) + edit(ic) } - aggregatedErrors := validateMarketplaceImages(azureClient, editedInstallConfig) + metadata := NewMetadata(ic.Azure, ic.ControlPlane, ic.WorkerMachinePool()) + metadata.UseMockClient(azureClient) + aggregatedErrors := validateMarketplaceImages(azureClient, metadata, ic) err := aggregatedErrors.ToAggregate() if tc.errorMsg != "" { assert.Regexp(t, tc.errorMsg, err) diff --git a/pkg/asset/installconfig/installconfig.go b/pkg/asset/installconfig/installconfig.go index 4aac864b127..153e79f28e1 100644 --- a/pkg/asset/installconfig/installconfig.go +++ b/pkg/asset/installconfig/installconfig.go @@ -164,7 +164,7 @@ func (a *InstallConfig) finish(ctx context.Context, filename string) error { } } if a.Config.Azure != nil { - a.Azure = icazure.NewMetadata(a.Config.Azure.CloudName, a.Config.Azure.ARMEndpoint, a.Config.Azure.Region) + a.Azure = icazure.NewMetadata(a.Config.Azure, a.Config.ControlPlane, &a.Config.Compute[0]) } if a.Config.GCP != nil { if err := a.finishGCP(); err != nil { @@ -207,7 +207,7 @@ func (a *InstallConfig) platformValidation(ctx context.Context) error { if err != nil { return err } - return icazure.Validate(client, a.Config) + return icazure.Validate(client, a.Azure, a.Config) } if a.Config.Platform.GCP != nil { client, err := icgcp.NewClient(ctx, a.Config.GCP.Endpoint) diff --git a/pkg/asset/machines/azure/azuremachines.go b/pkg/asset/machines/azure/azuremachines.go index 6c7f09ff112..a0d4434dda8 100644 --- a/pkg/asset/machines/azure/azuremachines.go +++ b/pkg/asset/machines/azure/azuremachines.go @@ -26,16 +26,17 @@ const ( // MachineInput defines the inputs needed to generate a machine asset. type MachineInput struct { - Subnet string - Role string - UserDataSecret string - HyperVGen string - StorageSuffix string - UseImageGallery bool - Private bool - UserTags map[string]string - Platform *aztypes.Platform - Pool *types.MachinePool + Subnet string + Role string + UserDataSecret string + HyperVGen string + StorageSuffix string + Environment aztypes.CloudEnvironment + Private bool + UserTags map[string]string + Platform *aztypes.Platform + Pool *types.MachinePool + RHCOS string } // GenerateMachines returns manifests and runtime objects to provision the control plane (including bootstrap, if applicable) nodes using CAPI. @@ -59,38 +60,7 @@ func GenerateMachines(clusterID, resourceGroup, subscriptionID string, session * if err != nil { return nil, fmt.Errorf("failed to create machineapi.TagSpecifications from UserTags: %w", err) } - - var image *capz.Image - osImage := mpool.OSImage - galleryName := strings.ReplaceAll(clusterID, "-", "_") - - switch { - case osImage.Publisher != "": - image = &capz.Image{ - Marketplace: &capz.AzureMarketplaceImage{ - ImagePlan: capz.ImagePlan{ - Publisher: osImage.Publisher, - Offer: osImage.Offer, - SKU: osImage.SKU, - }, - Version: osImage.Version, - ThirdPartyImage: osImage.Plan != aztypes.ImageNoPurchasePlan, - }, - } - case in.UseImageGallery: - // image gallery names cannot have dashes - imageID := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/galleries/gallery_%s/images/%s", subscriptionID, resourceGroup, galleryName, clusterID) - if in.HyperVGen == "V2" && in.Platform.CloudName != aztypes.StackCloud { - imageID += genV2Suffix - } - image = &capz.Image{ID: &imageID} - default: - // AzureStack is the only use for managed images & supports only Gen1 VMs: - // https://learn.microsoft.com/en-us/azure-stack/user/azure-stack-vm-considerations?view=azs-2501&tabs=az1%2Caz2#vm-differences - imageID := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/images/%s", subscriptionID, resourceGroup, clusterID) - image = &capz.Image{ID: &imageID} - } - + image := capzImage(mpool.OSImage, in.Environment, in.HyperVGen, resourceGroup, subscriptionID, clusterID, in.RHCOS) // Set up OSDisk osDisk := capz.OSDisk{ OSType: "Linux", @@ -383,3 +353,46 @@ func bootDiagStorageURIBuilder(diag *aztypes.BootDiagnostics, storageEndpointSuf } return "" } + +func capzImage(osImage aztypes.OSImage, azEnv aztypes.CloudEnvironment, gen, rg, sub, infraID, rhcosImg string) *capz.Image { + switch { + case osImage.Publisher != "": + return &capz.Image{ + Marketplace: &capz.AzureMarketplaceImage{ + ImagePlan: capz.ImagePlan{ + Publisher: osImage.Publisher, + Offer: osImage.Offer, + SKU: osImage.SKU, + }, + Version: osImage.Version, + ThirdPartyImage: osImage.Plan != aztypes.ImageNoPurchasePlan, + }, + } + case azEnv == aztypes.StackCloud: + // AzureStack is the only use for managed images & supports only Gen1 VMs: + // https://learn.microsoft.com/en-us/azure-stack/user/azure-stack-vm-considerations?view=azs-2501&tabs=az1%2Caz2#vm-differences + imageID := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/images/%s", sub, rg, infraID) + return &capz.Image{ID: &imageID} + case strings.Count(rhcosImg, ":") == 3: // Marketplace Image URN contains 3 colons + rhcosMktImg := strings.Split(rhcosImg, ":") + return &capz.Image{ + Marketplace: &capz.AzureMarketplaceImage{ + ImagePlan: capz.ImagePlan{ + Publisher: rhcosMktImg[0], + Offer: rhcosMktImg[1], + SKU: rhcosMktImg[2], + }, + Version: rhcosMktImg[3], + ThirdPartyImage: false, + }, + } + default: // Installer-created image gallery, should only be OKD. + // image gallery names cannot have dashes + galleryName := strings.ReplaceAll(infraID, "-", "_") + imageID := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/galleries/gallery_%s/images/%s", sub, rg, galleryName, infraID) + if gen == "V2" { + imageID += genV2Suffix + } + return &capz.Image{ID: &imageID} + } +} diff --git a/pkg/asset/machines/azure/machines.go b/pkg/asset/machines/azure/machines.go index 9cd8476b0fa..51159f9002b 100644 --- a/pkg/asset/machines/azure/machines.go +++ b/pkg/asset/machines/azure/machines.go @@ -27,7 +27,7 @@ const ( ) // Machines returns a list of machines for a machinepool. -func Machines(clusterID string, config *types.InstallConfig, pool *types.MachinePool, osImage, role, userDataSecret string, capabilities map[string]string, useImageGallery bool, session *icazure.Session) ([]machineapi.Machine, *machinev1.ControlPlaneMachineSet, error) { +func Machines(clusterID string, config *types.InstallConfig, pool *types.MachinePool, osImage, role, userDataSecret string, capabilities map[string]string, session *icazure.Session) ([]machineapi.Machine, *machinev1.ControlPlaneMachineSet, error) { if configPlatform := config.Platform.Name(); configPlatform != azure.Name { return nil, nil, fmt.Errorf("non-Azure configuration: %q", configPlatform) } @@ -62,7 +62,7 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine azIndex = int(idx) % len(azs) } subnetIndex := int(idx) % len(subnets) - provider, err := provider(platform, mpool, osImage, userDataSecret, clusterID, role, &azIndex, capabilities, useImageGallery, session, networkResourceGroup, virtualNetworkName, subnets[subnetIndex]) + provider, err := provider(platform, mpool, osImage, userDataSecret, clusterID, role, &azIndex, capabilities, session, networkResourceGroup, virtualNetworkName, subnets[subnetIndex]) if err != nil { return nil, nil, errors.Wrap(err, "failed to create provider") } @@ -159,7 +159,7 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine return machines, controlPlaneMachineSet, nil } -func provider(platform *azure.Platform, mpool *azure.MachinePool, osImage string, userDataSecret string, clusterID string, role string, azIdx *int, capabilities map[string]string, useImageGallery bool, session *icazure.Session, networkResourceGroup, virtualNetwork, subnet string) (*machineapi.AzureMachineProviderSpec, error) { +func provider(platform *azure.Platform, mpool *azure.MachinePool, osImage string, userDataSecret string, clusterID string, role string, azIdx *int, capabilities map[string]string, session *icazure.Session, networkResourceGroup, virtualNetwork, subnet string) (*machineapi.AzureMachineProviderSpec, error) { var az string if len(mpool.Zones) > 0 && azIdx != nil { az = mpool.Zones[*azIdx] @@ -179,32 +179,7 @@ func provider(platform *azure.Platform, mpool *azure.MachinePool, osImage string } rg := platform.ClusterResourceGroupName(clusterID) - var image machineapi.Image - if mpool.OSImage.Publisher != "" { - image.Type = machineapi.AzureImageTypeMarketplaceWithPlan - if mpool.OSImage.Plan == azure.ImageNoPurchasePlan { - image.Type = machineapi.AzureImageTypeMarketplaceNoPlan - } - image.Publisher = mpool.OSImage.Publisher - image.Offer = mpool.OSImage.Offer - image.SKU = mpool.OSImage.SKU - image.Version = mpool.OSImage.Version - } else if useImageGallery { - // image gallery names cannot have dashes - galleryName := strings.ReplaceAll(clusterID, "-", "_") - id := clusterID - if hyperVGen == "V2" { - id += "-gen2" - } - imageID := fmt.Sprintf("/resourceGroups/%s/providers/Microsoft.Compute/galleries/gallery_%s/images/%s/versions/latest", rg, galleryName, id) - image.ResourceID = imageID - } else { - imageID := fmt.Sprintf("/resourceGroups/%s/providers/Microsoft.Compute/images/%s", rg, clusterID) - if hyperVGen == "V2" && platform.CloudName != azure.StackCloud { - imageID += "-gen2" - } - image.ResourceID = imageID - } + image := mapiImage(mpool.OSImage, platform.CloudName, hyperVGen, rg, session.Credentials.SubscriptionID, clusterID, osImage) if mpool.OSDisk.DiskType == "" { mpool.OSDisk.DiskType = "Premium_LRS" @@ -454,3 +429,30 @@ func generateSecurityProfile(mpool *azure.MachinePool) *machineapi.SecurityProfi return securityProfile } + +func mapiImage(osImage azure.OSImage, azEnv azure.CloudEnvironment, gen, rg, sub, infraID, rhcosImg string) machineapi.Image { + cImg := capzImage(osImage, azEnv, gen, rg, sub, infraID, rhcosImg) + mImg := machineapi.Image{} + + if cImg.ID != nil { + mImg.ResourceID = trimSubscriptionPrefix(*cImg.ID) + } else if cImg.Marketplace != nil { + mImg.Publisher = cImg.Marketplace.Publisher + mImg.Offer = cImg.Marketplace.Offer + mImg.SKU = cImg.Marketplace.SKU + mImg.Version = cImg.Marketplace.Version + mImg.Type = machineapi.AzureImageTypeMarketplaceNoPlan + if cImg.Marketplace.ThirdPartyImage { + mImg.Type = machineapi.AzureImageTypeMarketplaceWithPlan + } + } + return mImg +} + +// trimSubscriptionPrefix takes an image id string +// formatted for CAPZ and returns a string formatted +// for MAPI, by removing the /subspcription/ prefix. +func trimSubscriptionPrefix(image string) string { + rgIndex := strings.Index(image, "/resourceGroups/") + return image[rgIndex:] +} diff --git a/pkg/asset/machines/azure/machinesets.go b/pkg/asset/machines/azure/machinesets.go index 90f18cbd7ea..1e8dbbcf92d 100644 --- a/pkg/asset/machines/azure/machinesets.go +++ b/pkg/asset/machines/azure/machinesets.go @@ -18,7 +18,7 @@ import ( ) // MachineSets returns a list of machinesets for a machinepool. -func MachineSets(clusterID string, ic *installconfig.InstallConfig, pool *types.MachinePool, osImage, role, userDataSecret string, capabilities map[string]string, useImageGallery bool, subnetZones []string, session *icazure.Session) ([]*clusterapi.MachineSet, error) { +func MachineSets(clusterID string, ic *installconfig.InstallConfig, pool *types.MachinePool, osImage, role, userDataSecret string, capabilities map[string]string, subnetZones []string, session *icazure.Session) ([]*clusterapi.MachineSet, error) { config := ic.Config if configPlatform := config.Platform.Name(); configPlatform != azure.Name { return nil, fmt.Errorf("non-azure configuration: %q", configPlatform) @@ -63,7 +63,6 @@ func MachineSets(clusterID string, ic *installconfig.InstallConfig, pool *types. clusterID: clusterID, role: role, capabilities: capabilities, - useImageGallery: useImageGallery, session: session, subnetSpec: config.Azure.Subnets, replicas: total, @@ -78,7 +77,7 @@ func MachineSets(clusterID string, ic *installconfig.InstallConfig, pool *types. replicas++ } subnetIndex = (subnetIndex + 1) % len(subnets) - provider, err := provider(platform, mpool, osImage, userDataSecret, clusterID, role, &idx, capabilities, useImageGallery, session, networkResourceGroup, virtualNetworkName, subnets[subnetIndex]) + provider, err := provider(platform, mpool, osImage, userDataSecret, clusterID, role, &idx, capabilities, session, networkResourceGroup, virtualNetworkName, subnets[subnetIndex]) if err != nil { return nil, errors.Wrap(err, "failed to create provider") } @@ -137,7 +136,6 @@ type multiZoneMachineSetInput struct { clusterID string role string capabilities map[string]string - useImageGallery bool session *icazure.Session virtualNetworkName string subnetSpec []azure.SubnetSpec @@ -197,7 +195,7 @@ func getMultiZoneMachineSets(in multiZoneMachineSetInput) ([]*clusterapi.Machine currentReplica++ remainder-- } - provider, err := provider(in.platform, in.mpool, in.osImage, in.userDataSecret, in.clusterID, in.role, &idx, in.capabilities, in.useImageGallery, in.session, in.networkResourceGroup, in.virtualNetworkName, subnet) + provider, err := provider(in.platform, in.mpool, in.osImage, in.userDataSecret, in.clusterID, in.role, &idx, in.capabilities, in.session, in.networkResourceGroup, in.virtualNetworkName, subnet) if err != nil { return nil, errors.Wrap(err, "failed to create provider") } diff --git a/pkg/asset/machines/clusterapi.go b/pkg/asset/machines/clusterapi.go index 0ea49fde970..697ee96327e 100644 --- a/pkg/asset/machines/clusterapi.go +++ b/pkg/asset/machines/clusterapi.go @@ -261,7 +261,7 @@ func (c *ClusterAPI) Generate(ctx context.Context, dependencies asset.Parents) e mpool.OSImage.Publisher = *img.Plan.Publisher } } - capabilities, err := client.GetVMCapabilities(ctx, mpool.InstanceType, installConfig.Config.Platform.Azure.Region) + capabilities, err := installConfig.Azure.ControlPlaneCapabilities() if err != nil { return err } @@ -296,16 +296,17 @@ func (c *ClusterAPI) Generate(ctx context.Context, dependencies asset.Parents) e session.Credentials.SubscriptionID, session, &azure.MachineInput{ - Subnet: subnet, - Role: "master", - UserDataSecret: "master-user-data", - HyperVGen: hyperVGen, - UseImageGallery: installConfig.Azure.CloudName != azuretypes.StackCloud, - Private: installConfig.Config.Publish == types.InternalPublishingStrategy, - UserTags: installConfig.Config.Platform.Azure.UserTags, - Platform: installConfig.Config.Platform.Azure, - Pool: &pool, - StorageSuffix: session.Environment.StorageEndpointSuffix, + Subnet: subnet, + Role: "master", + UserDataSecret: "master-user-data", + HyperVGen: hyperVGen, + Environment: installConfig.Azure.CloudName, + Private: installConfig.Config.Publish == types.InternalPublishingStrategy, + UserTags: installConfig.Config.Platform.Azure.UserTags, + Platform: installConfig.Config.Platform.Azure, + Pool: &pool, + StorageSuffix: session.Environment.StorageEndpointSuffix, + RHCOS: rhcosImage.ControlPlane, }, ) if err != nil { diff --git a/pkg/asset/machines/master.go b/pkg/asset/machines/master.go index 52fa726a770..d27f0c1c7b3 100644 --- a/pkg/asset/machines/master.go +++ b/pkg/asset/machines/master.go @@ -397,7 +397,7 @@ func (m *Master) Generate(ctx context.Context, dependencies asset.Parents) error } pool.Platform.Azure = &mpool - capabilities, err := client.GetVMCapabilities(ctx, mpool.InstanceType, installConfig.Config.Platform.Azure.Region) + capabilities, err := installConfig.Azure.ControlPlaneCapabilities() if err != nil { return err } @@ -405,8 +405,7 @@ func (m *Master) Generate(ctx context.Context, dependencies asset.Parents) error if err != nil { return err } - useImageGallery := installConfig.Azure.CloudName != azuretypes.StackCloud - machines, controlPlaneMachineSet, err = azure.Machines(clusterID.InfraID, ic, &pool, rhcosImage.ControlPlane, "master", masterUserDataSecretName, capabilities, useImageGallery, session) + machines, controlPlaneMachineSet, err = azure.Machines(clusterID.InfraID, ic, &pool, rhcosImage.ControlPlane, "master", masterUserDataSecretName, capabilities, session) if err != nil { return errors.Wrap(err, "failed to create master machine objects") } diff --git a/pkg/asset/machines/worker.go b/pkg/asset/machines/worker.go index a90dbdc4ee8..d45fa469855 100644 --- a/pkg/asset/machines/worker.go +++ b/pkg/asset/machines/worker.go @@ -610,13 +610,12 @@ func (w *Worker) Generate(ctx context.Context, dependencies asset.Parents) error } pool.Platform.Azure = &mpool - capabilities, err := client.GetVMCapabilities(ctx, mpool.InstanceType, installConfig.Config.Platform.Azure.Region) + capabilities, err := installConfig.Azure.ComputeCapabilities() if err != nil { return err } - useImageGallery := ic.Platform.Azure.CloudName != azuretypes.StackCloud - sets, err := azure.MachineSets(clusterID.InfraID, installConfig, &pool, rhcosImage.Compute, "worker", workerUserDataSecretName, capabilities, useImageGallery, subnetZones, session) + sets, err := azure.MachineSets(clusterID.InfraID, installConfig, &pool, rhcosImage.Compute, "worker", workerUserDataSecretName, capabilities, subnetZones, session) if err != nil { return errors.Wrap(err, "failed to create worker machine objects") } diff --git a/pkg/asset/rhcos/image.go b/pkg/asset/rhcos/image.go index d3e3a501ae7..aeef055a389 100644 --- a/pkg/asset/rhcos/image.go +++ b/pkg/asset/rhcos/image.go @@ -13,6 +13,7 @@ import ( "github.com/openshift/installer/pkg/asset" "github.com/openshift/installer/pkg/asset/installconfig" + icazure "github.com/openshift/installer/pkg/asset/installconfig/azure" "github.com/openshift/installer/pkg/rhcos" "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/aws" @@ -63,15 +64,17 @@ func (i *Image) Generate(ctx context.Context, p asset.Parents) error { ic := &installconfig.InstallConfig{} p.Get(ic) config := ic.Config - osimageControlPlane, err := osImage(ctx, config, config.ControlPlane.Architecture) + osimageControlPlane, err := osImage(ctx, ic, config.ControlPlane) if err != nil { return err } - arch := config.ControlPlane.Architecture + var computePool *types.MachinePool if len(config.Compute) > 0 { - arch = config.Compute[0].Architecture + computePool = &config.Compute[0] + } else { + computePool = config.ControlPlane } - osimageCompute, err := osImage(ctx, config, arch) + osimageCompute, err := osImage(ctx, ic, computePool) if err != nil { return err } @@ -80,10 +83,11 @@ func (i *Image) Generate(ctx context.Context, p asset.Parents) error { } //nolint:gocyclo -func osImage(ctx context.Context, config *types.InstallConfig, nodeArch types.Architecture) (string, error) { +func osImage(ctx context.Context, ic *installconfig.InstallConfig, machinePool *types.MachinePool) (string, error) { ctx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() + nodeArch := machinePool.Architecture archName := arch.RpmArch(string(nodeArch)) st, err := rhcos.FetchCoreOSBuild(ctx) @@ -94,9 +98,11 @@ func osImage(ctx context.Context, config *types.InstallConfig, nodeArch types.Ar if err != nil { return "", err } - switch config.Platform.Name() { + streamArchPrefix := st.FormatPrefix(archName) + platform := ic.Config.Platform + switch platform.Name() { case aws.Name: - region := config.Platform.AWS.Region + region := platform.AWS.Region if !rhcos.AMIRegions(nodeArch).Has(region) { const globalResourceRegion = "us-east-1" logrus.Debugf("No AMI found in %s. Using AMI from %s.", region, globalResourceRegion) @@ -106,7 +112,7 @@ func osImage(ctx context.Context, config *types.InstallConfig, nodeArch types.Ar if err != nil { return "", err } - if region != config.Platform.AWS.Region { + if region != platform.AWS.Region { osimage = fmt.Sprintf("%s,%s", osimage, region) } return osimage, nil @@ -115,14 +121,14 @@ func osImage(ctx context.Context, config *types.InstallConfig, nodeArch types.Ar img := streamArch.Images.Gcp return fmt.Sprintf("projects/%s/global/images/%s", img.Project, img.Name), nil } - return "", fmt.Errorf("%s: No GCP build found", st.FormatPrefix(archName)) + return "", fmt.Errorf("%s: No GCP build found", streamArchPrefix) case ibmcloud.Name: if a, ok := streamArch.Artifacts["ibmcloud"]; ok { return rhcos.FindArtifactURL(a) } - return "", fmt.Errorf("%s: No ibmcloud build found", st.FormatPrefix(archName)) + return "", fmt.Errorf("%s: No ibmcloud build found", streamArchPrefix) case ovirt.Name, openstack.Name, powervc.Name: - op := config.Platform.OpenStack + op := platform.OpenStack if op != nil { if oi := op.ClusterOSImage; oi != "" { return oi, nil @@ -131,31 +137,48 @@ func osImage(ctx context.Context, config *types.InstallConfig, nodeArch types.Ar if a, ok := streamArch.Artifacts["openstack"]; ok { return rhcos.FindArtifactURL(a) } - return "", fmt.Errorf("%s: No openstack build found", st.FormatPrefix(archName)) + return "", fmt.Errorf("%s: No openstack build found", streamArchPrefix) case azure.Name: ext := streamArch.RHELCoreOSExtensions - if config.Platform.Azure.CloudName == azure.StackCloud { - return config.Platform.Azure.ClusterOSImage, nil + if platform.Azure.CloudName == azure.StackCloud { + return platform.Azure.ClusterOSImage, nil } if ext == nil { - return "", fmt.Errorf("%s: No azure build found", st.FormatPrefix(archName)) + return "", fmt.Errorf("%s: No extensions found in stream", streamArchPrefix) } - azd := ext.AzureDisk - if azd == nil { - return "", fmt.Errorf("%s: No azure build found", st.FormatPrefix(archName)) + if ext.AzureDisk == nil { + return "", fmt.Errorf("%s: No azure build found", streamArchPrefix) } - return azd.URL, nil + azi := ext.AzureDisk.URL + azMP := machinePool.Platform.Azure + confidentialVM := azMP != nil && azMP.Settings != nil && azMP.Settings.SecurityType != "" + if mkt := ext.Marketplace; mkt == nil || mkt.Azure == nil || mkt.Azure.NoPurchasePlan == nil || mkt.Azure.NoPurchasePlan.Gen2 == nil { + logrus.Warnf("%s: No default Azure marketplace image was found in stream", streamArchPrefix) + } else if !confidentialVM { // Marketplace images don't suppot confidential VMs, so stick with managed image. + gen, err := getHyperVGeneration(ic.Azure, machinePool.Name) + if err != nil { + return "", fmt.Errorf("failed to get hyperVGeneration: %w", err) + } + azi = ext.Marketplace.Azure.NoPurchasePlan.Gen2.URN() + if gen == "V1" { + if mkt.Azure.NoPurchasePlan.Gen1 == nil { + return "", fmt.Errorf("a HyperVGeneration 1 instance was selected but no Gen1 marketplace image is available") + } + azi = ext.Marketplace.Azure.NoPurchasePlan.Gen1.URN() + } + } + return azi, nil case baremetal.Name: // Check for image URL override - if oi := config.Platform.BareMetal.ClusterOSImage; oi != "" { + if oi := platform.BareMetal.ClusterOSImage; oi != "" { return oi, nil } // Use image from release payload return "", nil case vsphere.Name: // Check for image URL override - if config.Platform.VSphere.ClusterOSImage != "" { - return config.Platform.VSphere.ClusterOSImage, nil + if platform.VSphere.ClusterOSImage != "" { + return platform.VSphere.ClusterOSImage, nil } if a, ok := streamArch.Artifacts["vmware"]; ok { @@ -177,11 +200,11 @@ func osImage(ctx context.Context, config *types.InstallConfig, nodeArch types.Ar return u.String(), nil } - return "", fmt.Errorf("%s: No vmware build found", st.FormatPrefix(archName)) + return "", fmt.Errorf("%s: No vmware build found", streamArchPrefix) case powervs.Name: // Check for image URL override - if config.Platform.PowerVS.ClusterOSImage != "" { - return config.Platform.PowerVS.ClusterOSImage, nil + if platform.PowerVS.ClusterOSImage != "" { + return platform.PowerVS.ClusterOSImage, nil } if streamArch.Images.PowerVS != nil { @@ -189,35 +212,35 @@ func osImage(ctx context.Context, config *types.InstallConfig, nodeArch types.Ar vpcRegion string err error ) - if config.Platform.PowerVS.VPCRegion != "" { - vpcRegion = config.Platform.PowerVS.VPCRegion + if platform.PowerVS.VPCRegion != "" { + vpcRegion = platform.PowerVS.VPCRegion } else { - vpcRegion = powervs.Regions[config.Platform.PowerVS.Region].VPCRegion + vpcRegion = powervs.Regions[platform.PowerVS.Region].VPCRegion } vpcRegion, err = powervs.COSRegionForVPCRegion(vpcRegion) if err != nil { - return "", fmt.Errorf("%s: No Power COS region found", st.FormatPrefix(archName)) + return "", fmt.Errorf("%s: No Power COS region found", streamArchPrefix) } img := streamArch.Images.PowerVS.Regions[vpcRegion] logrus.Debug("Power VS using image ", img.Object) return fmt.Sprintf("%s/%s", img.Bucket, img.Object), nil } - return "", fmt.Errorf("%s: No Power VS build found", st.FormatPrefix(archName)) + return "", fmt.Errorf("%s: No Power VS build found", streamArchPrefix) case external.Name: return "", nil case none.Name: return "", nil case nutanix.Name: - if config.Platform.Nutanix != nil && config.Platform.Nutanix.ClusterOSImage != "" { - return config.Platform.Nutanix.ClusterOSImage, nil + if platform.Nutanix != nil && platform.Nutanix.ClusterOSImage != "" { + return platform.Nutanix.ClusterOSImage, nil } if a, ok := streamArch.Artifacts["nutanix"]; ok { return rhcos.FindArtifactURL(a) } - return "", fmt.Errorf("%s: No nutanix build found", st.FormatPrefix(archName)) + return "", fmt.Errorf("%s: No nutanix build found", streamArchPrefix) default: - return "", fmt.Errorf("invalid platform %v", config.Platform.Name()) + return "", fmt.Errorf("invalid platform %v", platform.Name()) } } @@ -228,3 +251,10 @@ func MakeAsset(osImage string) *Image { Compute: osImage, } } + +func getHyperVGeneration(metadata *icazure.Metadata, role string) (string, error) { + if role == types.MachinePoolControlPlaneRoleName { + return metadata.ControlPlaneHyperVGeneration() + } + return metadata.ComputeHyperVGeneration() +} diff --git a/pkg/infrastructure/azure/azure.go b/pkg/infrastructure/azure/azure.go index 32b6fbaaacd..0539428122f 100644 --- a/pkg/infrastructure/azure/azure.go +++ b/pkg/infrastructure/azure/azure.go @@ -5,7 +5,6 @@ import ( "fmt" "math/rand" "net/http" - "os" "strings" "time" @@ -96,8 +95,6 @@ func (p Provider) ProvisionTimeout() time.Duration { func (*Provider) PublicGatherEndpoint() clusterapi.GatherEndpoint { return clusterapi.APILoadBalancer } // InfraReady is called once the installer infrastructure is ready. -// -//nolint:gocyclo //TODO(padillon): forthcoming marketplace image support should help reduce complexity here. func (p *Provider) InfraReady(ctx context.Context, in clusterapi.InfraReadyInput) error { session, err := in.InstallConfig.Azure.Session() if err != nil { @@ -257,9 +254,9 @@ func (p *Provider) InfraReady(ctx context.Context, in clusterapi.InfraReadyInput logrus.Debugf("StorageAccount.ID=%s", *storageAccount.ID) } - // Upload the image to the container - _, skipImageUpload := os.LookupEnv("OPENSHIFT_INSTALL_SKIP_IMAGE_UPLOAD") - if !(skipImageUpload || platform.CloudName == aztypes.StackCloud) { + // Create a managed image, which is used for OKD or confidential VMs on OCP. + hasConfidentialVM := getMachinePoolSecurityType(installConfig) != "" + if (hasConfidentialVM || installConfig.IsOKD()) && platform.CloudName != aztypes.StackCloud { // Create vhd blob storage container publicAccess := armstorage.PublicAccessNone createBlobContainerOutput, err := CreateBlobContainer(ctx, &CreateBlobContainerInput{ @@ -331,10 +328,7 @@ func (p *Provider) InfraReady(ctx context.Context, in clusterapi.InfraReadyInput // If Control Plane Security Type is provided, then pass that along // during Gen V2 Gallery Image creation. It will be added as a // supported feature of the image. - securityType, err := getMachinePoolSecurityType(in) - if err != nil { - return err - } + securityType := getMachinePoolSecurityType(installConfig) _, err = CreateGalleryImage(ctx, &CreateGalleryImageInput{ ResourceGroupName: resourceGroupName, @@ -823,16 +817,16 @@ func (p Provider) Ignition(ctx context.Context, in clusterapi.IgnitionInput) ([] return ignSecrets, nil } -func getMachinePoolSecurityType(in clusterapi.InfraReadyInput) (string, error) { +func getMachinePoolSecurityType(installConfig *types.InstallConfig) string { var securityType aztypes.SecurityTypes - if in.InstallConfig.Config.ControlPlane != nil && in.InstallConfig.Config.ControlPlane.Platform.Azure != nil { - pool := in.InstallConfig.Config.ControlPlane.Platform.Azure + if installConfig.ControlPlane != nil && installConfig.ControlPlane.Platform.Azure != nil { + pool := installConfig.ControlPlane.Platform.Azure if pool.Settings != nil { securityType = pool.Settings.SecurityType } } - if securityType == "" && in.InstallConfig.Config.Compute != nil { - for _, compute := range in.InstallConfig.Config.Compute { + if securityType == "" && installConfig.Compute != nil { + for _, compute := range installConfig.Compute { if compute.Platform.Azure != nil { pool := compute.Platform.Azure if pool.Settings != nil { @@ -842,17 +836,17 @@ func getMachinePoolSecurityType(in clusterapi.InfraReadyInput) (string, error) { } } } - if securityType == "" && in.InstallConfig.Config.Platform.Azure.DefaultMachinePlatform != nil { - pool := in.InstallConfig.Config.Platform.Azure.DefaultMachinePlatform + if securityType == "" && installConfig.Platform.Azure.DefaultMachinePlatform != nil { + pool := installConfig.Platform.Azure.DefaultMachinePlatform if pool.Settings != nil { securityType = pool.Settings.SecurityType } } switch securityType { case aztypes.SecurityTypesTrustedLaunch: - return trustedLaunchST, nil + return trustedLaunchST case aztypes.SecurityTypesConfidentialVM: - return confidentialVMST, nil + return confidentialVMST } - return "", nil + return "" } diff --git a/pkg/types/azure/defaults/platform.go b/pkg/types/azure/defaults/platform.go index 086162f8d79..6cd660a9929 100644 --- a/pkg/types/azure/defaults/platform.go +++ b/pkg/types/azure/defaults/platform.go @@ -24,6 +24,17 @@ func SetPlatformDefaults(p *azure.Platform) { } } +// Apply sets values from the default machine platform to the machinepool. +func Apply(defaultMachinePlatform, machinePool *azure.MachinePool) { + // Construct a temporary machine pool so we can set the + // defaults first, without overwriting the pool-sepcific values, + // which have precedence. + tempMP := &azure.MachinePool{} + tempMP.Set(defaultMachinePlatform) + tempMP.Set(machinePool) + machinePool.Set(tempMP) +} + // getInstanceClass returns the instance "class" we should use for a given region. func getInstanceClass(region string) string { if class, ok := defaultMachineClass[region]; ok { diff --git a/pkg/types/defaults/machinepools.go b/pkg/types/defaults/machinepools.go index e8d0ac88177..943ca2998ba 100644 --- a/pkg/types/defaults/machinepools.go +++ b/pkg/types/defaults/machinepools.go @@ -2,6 +2,8 @@ package defaults import ( "github.com/openshift/installer/pkg/types" + "github.com/openshift/installer/pkg/types/azure" + azuredefaults "github.com/openshift/installer/pkg/types/azure/defaults" "github.com/openshift/installer/pkg/types/gcp" gcpdefaults "github.com/openshift/installer/pkg/types/gcp/defaults" "github.com/openshift/installer/pkg/version" @@ -24,6 +26,11 @@ func SetMachinePoolDefaults(p *types.MachinePool, platform *types.Platform) { } switch platform.Name() { + case azure.Name: + if p.Platform.Azure == nil && platform.Azure.DefaultMachinePlatform != nil { + p.Platform.Azure = &azure.MachinePool{} + } + azuredefaults.Apply(platform.Azure.DefaultMachinePlatform, p.Platform.Azure) case gcp.Name: gcpdefaults.SetMachinePoolDefaults(platform, p.Platform.GCP) default: