Skip to content

Commit

Permalink
Merge pull request #4419 from jstuever/cors1549
Browse files Browse the repository at this point in the history 
asset/installconfig/Azure: Validate install-config instance types
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot authored Dec 3, 2020
2 parents dfb55c3 + 91402a2 commit b9701c5
Show file tree
Hide file tree
Showing 4 changed files with 203 additions and 0 deletions.
32 changes: 32 additions & 0 deletions pkg/asset/installconfig/azure/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type API interface {
GetControlPlaneSubnet(ctx context.Context, resourceGroupName, virtualNetwork, subnet string) (*aznetwork.Subnet, error)
ListLocations(ctx context.Context) (*[]azsubs.Location, error)
GetResourcesProvider(ctx context.Context, resourceProviderNamespace string) (*azres.Provider, error)
GetVirtualMachineSku(ctx context.Context, name, region string) (*azsku.ResourceSku, error)
GetDiskSkus(ctx context.Context, region string) ([]azsku.ResourceSku, error)
GetGroup(ctx context.Context, groupName string) (*azres.Group, error)
ListResourceIDsByGroup(ctx context.Context, groupName string) ([]string, error)
Expand Down Expand Up @@ -211,3 +212,34 @@ func (c *Client) ListResourceIDsByGroup(ctx context.Context, groupName string) (
}
return res, nil
}

// GetVirtualMachineSku retrieves the resource SKU of a specified virtual machine SKU in the specified region.
func (c *Client) GetVirtualMachineSku(ctx context.Context, name, region string) (*azsku.ResourceSku, error) {
client := azsku.NewResourceSkusClientWithBaseURI(c.ssn.Environment.ResourceManagerEndpoint, c.ssn.Credentials.SubscriptionID)
client.Authorizer = c.ssn.Authorizer
ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
defer cancel()

for page, err := client.List(ctx); page.NotDone(); err = page.NextWithContext(ctx) {
if err != nil {
return nil, errors.Wrap(err, "error fetching SKU pages")
}
for _, sku := range page.Values() {
// Filter out resources that are not virtualMachines
if !strings.EqualFold("virtualMachines", *sku.ResourceType) {
continue
}
// Filter out resources that do not match the provided name
if !strings.EqualFold(name, *sku.Name) {
continue
}
// Return the resource from the provided region
for _, location := range to.StringSlice(sku.Locations) {
if strings.EqualFold(location, region) {
return &sku, nil
}
}
}
}
return nil, nil
}
15 changes: 15 additions & 0 deletions pkg/asset/installconfig/azure/mock/azureclient_generated.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

86 changes: 86 additions & 0 deletions pkg/asset/installconfig/azure/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"net"
"sort"
"strconv"
"strings"

azdns "github.com/Azure/azure-sdk-for-go/profiles/latest/dns/mgmt/dns"
Expand All @@ -17,15 +18,100 @@ import (
aztypes "github.com/openshift/installer/pkg/types/azure"
)

type resourceRequirements struct {
minimumVCpus int64
minimumMemory int64
}

var controlPlaneReq = resourceRequirements{
minimumVCpus: 4,
minimumMemory: 16,
}

var computeReq = resourceRequirements{
minimumVCpus: 2,
minimumMemory: 8,
}

// Validate executes platform-specific validation.
func Validate(client API, ic *types.InstallConfig) error {
allErrs := field.ErrorList{}

allErrs = append(allErrs, validateNetworks(client, ic.Azure, ic.Networking.MachineNetwork, field.NewPath("platform").Child("azure"))...)
allErrs = append(allErrs, validateRegion(client, field.NewPath("platform").Child("azure").Child("region"), ic.Azure)...)
allErrs = append(allErrs, validateInstanceTypes(client, ic)...)
return allErrs.ToAggregate()
}

// ValidateInstanceType ensures the instance type has sufficient Vcpu and Memory.
func ValidateInstanceType(client API, fieldPath *field.Path, region, instanceType string, req resourceRequirements) field.ErrorList {
allErrs := field.ErrorList{}

typeMeta, err := client.GetVirtualMachineSku(context.TODO(), instanceType, region)
if err != nil {
return append(allErrs, field.Invalid(fieldPath.Child("type"), instanceType, err.Error()))
}

if typeMeta == nil {
errMsg := fmt.Sprintf("not found in region %s", region)
return append(allErrs, field.Invalid(fieldPath.Child("type"), instanceType, errMsg))
}

for _, capability := range *typeMeta.Capabilities {

if strings.EqualFold(*capability.Name, "vCPUs") {
cpus, err := strconv.ParseInt(*capability.Value, 10, 0)
if err != nil {
return append(allErrs, field.InternalError(fieldPath, err))
}
if cpus < req.minimumVCpus {
errMsg := fmt.Sprintf("instance type does not meet minimum resource requirements of %d vCPUs", req.minimumVCpus)
allErrs = append(allErrs, field.Invalid(fieldPath.Child("type"), instanceType, errMsg))
}
} else if strings.EqualFold(*capability.Name, "MemoryGB") {
memory, err := strconv.ParseInt(*capability.Value, 10, 0)
if err != nil {
return append(allErrs, field.InternalError(fieldPath, err))
}
if memory < req.minimumMemory {
errMsg := fmt.Sprintf("instance type does not meet minimum resource requirements of %d GB Memory", req.minimumMemory)
allErrs = append(allErrs, field.Invalid(fieldPath.Child("type"), instanceType, errMsg))
}
}
}

return allErrs
}

// validateInstanceTypes checks that the user-provided instance types are valid.
func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList {
allErrs := field.ErrorList{}

// Default requirements need to be sufficient to support Control Plane instances.
defaultInstanceReq := controlPlaneReq

if ic.ControlPlane != nil && ic.ControlPlane.Platform.Azure != nil && ic.ControlPlane.Platform.Azure.InstanceType != "" {
// Default requirements can be relaxed when the controlPlane type is set explicitly.
defaultInstanceReq = computeReq

allErrs = append(allErrs, ValidateInstanceType(client, field.NewPath("controlPlane", "platform", "azure"), ic.Azure.Region, ic.ControlPlane.Platform.Azure.InstanceType, controlPlaneReq)...)
}

if ic.Platform.Azure.DefaultMachinePlatform != nil && ic.Platform.Azure.DefaultMachinePlatform.InstanceType != "" {
allErrs = append(allErrs, ValidateInstanceType(client, field.NewPath("platform", "azure", "defaultMachinePlatform"), ic.Azure.Region, ic.Platform.Azure.DefaultMachinePlatform.InstanceType, defaultInstanceReq)...)
}

for idx, compute := range ic.Compute {
fieldPath := field.NewPath("compute").Index(idx)
if compute.Platform.Azure != nil && compute.Platform.Azure.InstanceType != "" {
allErrs = append(allErrs, ValidateInstanceType(client, fieldPath.Child("platform", "azure"),
ic.Azure.Region, compute.Platform.Azure.InstanceType, computeReq)...)
}
}

return allErrs
}

// validateNetworks checks that the user-provided VNet and subnets are valid.
func validateNetworks(client API, p *aztypes.Platform, machineNetworks []types.MachineNetworkEntry, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
Expand Down
70 changes: 70 additions & 0 deletions pkg/asset/installconfig/azure/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net"
"testing"

azsku "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-10-01/compute"
aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-12-01/network"
azres "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-05-01/resources"
azsubs "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-06-01/subscriptions"
Expand Down Expand Up @@ -35,6 +36,34 @@ var (
validResourceGroupResourceType = "resourceGroups"
validResourceSkuRegions = "southeastasia"

instanceTypeSku = []*azsku.ResourceSku{
{Name: to.StringPtr("Standard_A1_v2"), Capabilities: &[]azsku.ResourceSkuCapabilities{{Name: to.StringPtr("vCPUs"), Value: to.StringPtr("1")}, {Name: to.StringPtr("MemoryGB"), Value: to.StringPtr("2")}}},
{Name: to.StringPtr("Standard_D2_v4"), Capabilities: &[]azsku.ResourceSkuCapabilities{{Name: to.StringPtr("vCPUs"), Value: to.StringPtr("2")}, {Name: to.StringPtr("MemoryGB"), Value: to.StringPtr("8")}}},
{Name: to.StringPtr("Standard_D4_v4"), Capabilities: &[]azsku.ResourceSkuCapabilities{{Name: to.StringPtr("vCPUs"), Value: to.StringPtr("4")}, {Name: to.StringPtr("MemoryGB"), Value: to.StringPtr("16")}}},
}

validInstanceTypes = func(ic *types.InstallConfig) {
ic.Platform.Azure.DefaultMachinePlatform.InstanceType = "Standard_D2_v4"
ic.ControlPlane.Platform.Azure.InstanceType = "Standard_D4_v4"
ic.Compute[0].Platform.Azure.InstanceType = "Standard_D2_v4"
}

invalidateDefaultInstanceTypes = func(ic *types.InstallConfig) {
ic.Platform.Azure.DefaultMachinePlatform.InstanceType = "Standard_A1_v2"
}

invalidateControlPlaneInstanceTypes = func(ic *types.InstallConfig) {
ic.ControlPlane.Platform.Azure.InstanceType = "Standard_A1_v2"
}

invalidateComputeInstanceTypes = func(ic *types.InstallConfig) {
ic.Compute[0].Platform.Azure.InstanceType = "Standard_A1_v2"
}

undefinedDefaultInstanceTypes = func(ic *types.InstallConfig) {
ic.Platform.Azure.DefaultMachinePlatform.InstanceType = "Dne_D2_v4"
}

invalidateMachineCIDR = func(ic *types.InstallConfig) {
_, newCidr, _ := net.ParseCIDR("192.168.111.0/24")
ic.MachineNetwork = []types.MachineNetworkEntry{
Expand Down Expand Up @@ -105,6 +134,16 @@ func validInstallConfig() *types.InstallConfig {
DefaultMachinePlatform: &azure.MachinePool{},
},
},
ControlPlane: &types.MachinePool{
Platform: types.MachinePoolPlatform{
Azure: &azure.MachinePool{},
},
},
Compute: []types.MachinePool{{
Platform: types.MachinePoolPlatform{
Azure: &azure.MachinePool{},
},
}},
}
}

Expand Down Expand Up @@ -149,6 +188,31 @@ func TestAzureInstallConfigValidation(t *testing.T) {
edits: editFunctions{invalidateControlPlaneSubnet, invalidateComputeSubnet},
errorMsg: "failed to retrieve compute subnet",
},
{
name: "Valid instance types",
edits: editFunctions{validInstanceTypes},
errorMsg: "",
},
{
name: "Invalid default machine type",
edits: editFunctions{invalidateDefaultInstanceTypes},
errorMsg: `\[platform.azure.defaultMachinePlatform.type: Invalid value: "Standard_A1_v2": instance type does not meet minimum resource requirements of 4 vCPUs, platform.azure.defaultMachinePlatform.type: Invalid value: "Standard_A1_v2": instance type does not meet minimum resource requirements of 16 GB Memory\]`,
},
{
name: "Invalid control plane instance types",
edits: editFunctions{invalidateControlPlaneInstanceTypes},
errorMsg: `[controlPlane.platform.azure.type: Invalid value: "n1\-standard\-1": instance type does not meet minimum resource requirements of 4 vCPUs, controlPlane.platform.azure.type: Invalid value: "n1\-standard\-1": instance type does not meet minimum resource requirements of 16 GB Memory]`,
},
{
name: "Invalid compute instance types",
edits: editFunctions{invalidateComputeInstanceTypes},
errorMsg: `\[compute\[0\].platform.azure.type: Invalid value: "Standard_A1_v2": instance type does not meet minimum resource requirements of 2 vCPUs, compute\[0\].platform.azure.type: Invalid value: "Standard_A1_v2": instance type does not meet minimum resource requirements of 8 GB Memory\]`,
},
{
name: "Undefined default instance types",
edits: editFunctions{undefinedDefaultInstanceTypes},
errorMsg: `platform.azure.defaultMachinePlatform.type: Invalid value: "Dne_D2_v4": not found in region`,
},
{
name: "Invalid region",
edits: editFunctions{invalidateRegion},
Expand All @@ -171,6 +235,12 @@ func TestAzureInstallConfigValidation(t *testing.T) {

azureClient := mock.NewMockAPI(mockCtrl)

// InstanceType
for _, value := range instanceTypeSku {
azureClient.EXPECT().GetVirtualMachineSku(gomock.Any(), to.String(value.Name), gomock.Any()).Return(value, nil).AnyTimes()
}
azureClient.EXPECT().GetVirtualMachineSku(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()

// VirtualNetwork
azureClient.EXPECT().GetVirtualNetwork(gomock.Any(), validNetworkResourceGroup, validVirtualNetwork).Return(virtualNetworkAPIResult, nil).AnyTimes()
azureClient.EXPECT().GetVirtualNetwork(gomock.Any(), gomock.Not(validNetworkResourceGroup), gomock.Not(validVirtualNetwork)).Return(&aznetwork.VirtualNetwork{}, fmt.Errorf("invalid network resource group")).AnyTimes()
Expand Down

0 comments on commit b9701c5

Please sign in to comment.