From 0935f20d66df7264d156e309595bc1f0f08b4d7f Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Mon, 28 Sep 2020 16:42:47 +0100 Subject: [PATCH] UPSTREAM: : openshift: Ensure the Virtual Machine provider state is set to Unknown when Failed --- go.mod | 2 +- go.sum | 4 +- .../apis/machine/v1beta1/machine_webhook.go | 292 ++++++------------ .../machine/v1beta1/machineset_webhook.go | 34 +- .../pkg/controller/machine/controller.go | 49 +++ vendor/modules.txt | 2 +- 6 files changed, 160 insertions(+), 223 deletions(-) diff --git a/go.mod b/go.mod index a2c1551a2e4..caebbb9a00b 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/onsi/ginkgo v1.12.1 github.com/onsi/gomega v1.10.1 github.com/openshift/api v0.0.0-20200901182017-7ac89ba6b971 - github.com/openshift/machine-api-operator v0.2.1-0.20200922150054-e0db6b65ba71 + github.com/openshift/machine-api-operator v0.2.1-0.20200926044412-b7d860f8074c github.com/spf13/cobra v1.0.0 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 diff --git a/go.sum b/go.sum index b13e711f5b7..9ae80b5b1d5 100644 --- a/go.sum +++ b/go.sum @@ -443,8 +443,8 @@ github.com/openshift/machine-api-operator v0.2.1-0.20200611014855-9a69f85c32dd/g github.com/openshift/machine-api-operator v0.2.1-0.20200701225707-950912b03628/go.mod h1:cxjy/RUzv5C2T5FNl1KKXUgtakWsezWQ642B/CD9VQA= github.com/openshift/machine-api-operator v0.2.1-0.20200722104429-f4f9b84df9b7 h1:jeLZ5Ng+Ri42dbZveN1ofrrRsUnYusauzeXPDTnXQfE= github.com/openshift/machine-api-operator v0.2.1-0.20200722104429-f4f9b84df9b7/go.mod h1:XDsNRAVEJtkI00e51SAZ/PnqNJl1zv0rHXSdl9L1oOY= -github.com/openshift/machine-api-operator v0.2.1-0.20200922150054-e0db6b65ba71 h1:w50MvkvZPOx30R3wMvSPRRJsPipxM82Ny9mY67DnpDk= -github.com/openshift/machine-api-operator v0.2.1-0.20200922150054-e0db6b65ba71/go.mod h1:SA92GU0BSLwmjwD9rL2OTXiRmcSMhgYPw5RXH4OuM4U= +github.com/openshift/machine-api-operator v0.2.1-0.20200926044412-b7d860f8074c h1:mjdL135+tVttQZ7OusXzV+HlAFAojZCE8oR1KpLci1I= +github.com/openshift/machine-api-operator v0.2.1-0.20200926044412-b7d860f8074c/go.mod h1:cp/wPVzxHZeLUjOLkNPNqrk4wyyW6HuHd3Kz9+hl5xw= github.com/operator-framework/operator-sdk v0.5.1-0.20190301204940-c2efe6f74e7b/go.mod h1:iVyukRkam5JZa8AnjYf+/G3rk7JI1+M6GsU0sq0B9NA= github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= diff --git a/vendor/github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1/machine_webhook.go b/vendor/github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1/machine_webhook.go index 0040c2adc11..501c2c54cac 100644 --- a/vendor/github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1/machine_webhook.go +++ b/vendor/github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1/machine_webhook.go @@ -29,17 +29,6 @@ import ( ) var ( - // AWS Defaults - defaultAWSIAMInstanceProfile = func(clusterID string) *string { - return pointer.StringPtr(fmt.Sprintf("%s-worker-profile", clusterID)) - } - defaultAWSSecurityGroup = func(clusterID string) string { - return fmt.Sprintf("%s-worker-sg", clusterID) - } - defaultAWSSubnet = func(clusterID, az string) string { - return fmt.Sprintf("%s-private-%s", clusterID, az) - } - // Azure Defaults defaultAzureVnet = func(clusterID string) string { return fmt.Sprintf("%s-vnet", clusterID) @@ -70,16 +59,6 @@ var ( defaultGCPTags = func(clusterID string) []string { return []string{fmt.Sprintf("%s-worker", clusterID)} } - defaultGCPServiceAccounts = func(clusterID, projectID string) []gcp.GCPServiceAccount { - if clusterID == "" || projectID == "" { - return []gcp.GCPServiceAccount{} - } - - return []gcp.GCPServiceAccount{{ - Email: fmt.Sprintf("%s-w@%s.iam.gserviceaccount.com", clusterID, projectID), - Scopes: []string{"https://www.googleapis.com/auth/cloud-platform"}, - }} - } ) const ( @@ -105,6 +84,7 @@ const ( defaultAzureCredentialsSecret = "azure-cloud-credentials" defaultAzureOSDiskOSType = "Linux" defaultAzureOSDiskStorageType = "Premium_LRS" + azureMaxDiskSizeGB = 32768 // GCP Defaults defaultGCPMachineType = "n1-standard-4" @@ -148,7 +128,7 @@ func getInfra() (*osconfigv1.Infrastructure, error) { return infra, nil } -type machineAdmissionFn func(m *Machine, clusterID string) (bool, utilerrors.Aggregate) +type machineAdmissionFn func(m *Machine, clusterID string) (bool, []string, utilerrors.Aggregate) type admissionHandler struct { clusterID string @@ -207,8 +187,8 @@ func getMachineValidatorOperation(platform osconfigv1.PlatformType) machineAdmis return validateVSphere default: // just no-op - return func(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { - return true, nil + return func(m *Machine, clusterID string) (bool, []string, utilerrors.Aggregate) { + return true, []string{}, nil } } } @@ -243,17 +223,13 @@ func getMachineDefaulterOperation(platformStatus *osconfigv1.PlatformStatus) mac case osconfigv1.AzurePlatformType: return defaultAzure case osconfigv1.GCPPlatformType: - projectID := "" - if platformStatus.GCP != nil { - projectID = platformStatus.GCP.ProjectID - } - return gcpDefaulter{projectID: projectID}.defaultGCP + return defaultGCP case osconfigv1.VSpherePlatformType: return defaultVSphere default: // just no-op - return func(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { - return true, nil + return func(m *Machine, clusterID string) (bool, []string, utilerrors.Aggregate) { + return true, []string{}, nil } } } @@ -426,6 +402,11 @@ func MachineSetMutatingWebhook() admissionregistrationv1.MutatingWebhook { } } +func responseWithWarnings(response admission.Response, warnings []string) admission.Response { + response.AdmissionResponse.Warnings = warnings + return response +} + // Handle handles HTTP requests for admission webhook servers. func (h *machineValidatorHandler) Handle(ctx context.Context, req admission.Request) admission.Response { m := &Machine{} @@ -436,11 +417,12 @@ func (h *machineValidatorHandler) Handle(ctx context.Context, req admission.Requ klog.V(3).Infof("Validate webhook called for Machine: %s", m.GetName()) - if ok, err := h.webhookOperations(m, h.clusterID); !ok { - return admission.Denied(err.Error()) + ok, warnings, errs := h.webhookOperations(m, h.clusterID) + if !ok { + return responseWithWarnings(admission.Denied(errs.Error()), warnings) } - return admission.Allowed("Machine valid") + return responseWithWarnings(admission.Allowed("Machine valid"), warnings) } // Handle handles HTTP requests for admission webhook servers. @@ -464,37 +446,36 @@ func (h *machineDefaulterHandler) Handle(ctx context.Context, req admission.Requ m.Labels[MachineClusterIDLabel] = h.clusterID } - if ok, err := h.webhookOperations(m, h.clusterID); !ok { - return admission.Denied(err.Error()) + ok, warnings, errs := h.webhookOperations(m, h.clusterID) + if !ok { + return responseWithWarnings(admission.Denied(errs.Error()), warnings) } marshaledMachine, err := json.Marshal(m) if err != nil { - return admission.Errored(http.StatusInternalServerError, err) + return responseWithWarnings(admission.Errored(http.StatusInternalServerError, err), warnings) } - return admission.PatchResponseFromRaw(req.Object.Raw, marshaledMachine) + return responseWithWarnings(admission.PatchResponseFromRaw(req.Object.Raw, marshaledMachine), warnings) } type awsDefaulter struct { region string } -func (a awsDefaulter) defaultAWS(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { +func (a awsDefaulter) defaultAWS(m *Machine, clusterID string) (bool, []string, utilerrors.Aggregate) { klog.V(3).Infof("Defaulting AWS providerSpec") var errs []error + var warnings []string providerSpec := new(aws.AWSMachineProviderConfig) if err := unmarshalInto(m, providerSpec); err != nil { errs = append(errs, err) - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } if providerSpec.InstanceType == "" { providerSpec.InstanceType = defaultAWSInstanceType } - if providerSpec.IAMInstanceProfile == nil { - providerSpec.IAMInstanceProfile = &aws.AWSResourceReference{ID: defaultAWSIAMInstanceProfile(clusterID)} - } if providerSpec.Placement.Region == "" { providerSpec.Placement.Region = a.region @@ -508,39 +489,17 @@ func (a awsDefaulter) defaultAWS(m *Machine, clusterID string) (bool, utilerrors providerSpec.CredentialsSecret = &corev1.LocalObjectReference{Name: defaultAWSCredentialsSecret} } - if providerSpec.SecurityGroups == nil { - providerSpec.SecurityGroups = []aws.AWSResourceReference{ - { - Filters: []aws.Filter{ - { - Name: "tag:Name", - Values: []string{defaultAWSSecurityGroup(clusterID)}, - }, - }, - }, - } - } - - if providerSpec.Subnet.ARN == nil && providerSpec.Subnet.ID == nil && providerSpec.Subnet.Filters == nil && providerSpec.Placement.AvailabilityZone != "" { - providerSpec.Subnet.Filters = []aws.Filter{ - { - Name: "tag:Name", - Values: []string{defaultAWSSubnet(clusterID, providerSpec.Placement.AvailabilityZone)}, - }, - } - } - rawBytes, err := json.Marshal(providerSpec) if err != nil { errs = append(errs, err) } if len(errs) > 0 { - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } m.Spec.ProviderSpec.Value = &runtime.RawExtension{Raw: rawBytes} - return true, nil + return true, warnings, nil } func unmarshalInto(m *Machine, providerSpec interface{}) error { @@ -554,14 +513,15 @@ func unmarshalInto(m *Machine, providerSpec interface{}) error { return nil } -func validateAWS(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { +func validateAWS(m *Machine, clusterID string) (bool, []string, utilerrors.Aggregate) { klog.V(3).Infof("Validating AWS providerSpec") var errs []error + var warnings []string providerSpec := new(aws.AWSMachineProviderConfig) if err := unmarshalInto(m, providerSpec); err != nil { errs = append(errs, err) - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } if providerSpec.AMI.ARN == nil && providerSpec.AMI.Filters == nil && providerSpec.AMI.ID == nil { @@ -594,16 +554,6 @@ func validateAWS(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { ) } - if providerSpec.IAMInstanceProfile == nil { - errs = append( - errs, - field.Required( - field.NewPath("providerSpec", "iamInstanceProfile"), - "expected providerSpec.iamInstanceProfile to be populated", - ), - ) - } - if providerSpec.UserDataSecret == nil { errs = append( errs, @@ -624,43 +574,31 @@ func validateAWS(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { ) } - if providerSpec.SecurityGroups == nil { - errs = append( - errs, - field.Required( - field.NewPath("providerSpec", "securityGroups"), - "expected providerSpec.securityGroups to be populated", - ), - ) - } - - if providerSpec.Subnet.ARN == nil && providerSpec.Subnet.ID == nil && providerSpec.Subnet.Filters == nil && providerSpec.Placement.AvailabilityZone == "" { - errs = append( - errs, - field.Required( - field.NewPath("providerSpec", "subnet"), - "expected either providerSpec.subnet.arn or providerSpec.subnet.id or providerSpec.subnet.filters or providerSpec.placement.availabilityZone to be populated", - ), + if providerSpec.Subnet.ARN == nil && providerSpec.Subnet.ID == nil && providerSpec.Subnet.Filters == nil { + warnings = append( + warnings, + "providerSpec.subnet: No subnet has been provided. Instances may be created in an unexpected subnet and may not join the cluster.", ) } // TODO(alberto): Validate providerSpec.BlockDevices. // https://github.com/openshift/cluster-api-provider-aws/pull/299#discussion_r433920532 if len(errs) > 0 { - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } - return true, nil + return true, warnings, nil } -func defaultAzure(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { +func defaultAzure(m *Machine, clusterID string) (bool, []string, utilerrors.Aggregate) { klog.V(3).Infof("Defaulting Azure providerSpec") var errs []error + var warnings []string providerSpec := new(azure.AzureMachineProviderSpec) if err := unmarshalInto(m, providerSpec); err != nil { errs = append(errs, err) - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } if providerSpec.VMSize == "" { @@ -671,26 +609,12 @@ func defaultAzure(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { if providerSpec.Vnet == "" && providerSpec.Subnet == "" { providerSpec.Vnet = defaultAzureVnet(clusterID) providerSpec.Subnet = defaultAzureSubnet(clusterID) - - // NetworkResourceGroup can be set by the user without Vnet and Subnet, - // only override if they didn't set it - if providerSpec.NetworkResourceGroup == "" { - providerSpec.NetworkResourceGroup = defaultAzureNetworkResourceGroup(clusterID) - } } if providerSpec.Image == (azure.Image{}) { providerSpec.Image.ResourceID = defaultAzureImageResourceID(clusterID) } - if providerSpec.ManagedIdentity == "" { - providerSpec.ManagedIdentity = defaultAzureManagedIdentiy(clusterID) - } - - if providerSpec.ResourceGroup == "" { - providerSpec.ResourceGroup = defaultAzureResourceGroup(clusterID) - } - if providerSpec.UserDataSecret == nil { providerSpec.UserDataSecret = &corev1.SecretReference{Name: defaultUserDataSecret} } else if providerSpec.UserDataSecret.Name == "" { @@ -708,39 +632,28 @@ func defaultAzure(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { } } - if providerSpec.OSDisk.OSType == "" { - providerSpec.OSDisk.OSType = defaultAzureOSDiskOSType - } - - if providerSpec.OSDisk.ManagedDisk.StorageAccountType == "" { - providerSpec.OSDisk.ManagedDisk.StorageAccountType = defaultAzureOSDiskStorageType - } - rawBytes, err := json.Marshal(providerSpec) if err != nil { errs = append(errs, err) } if len(errs) > 0 { - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } m.Spec.ProviderSpec.Value = &runtime.RawExtension{Raw: rawBytes} - return true, nil + return true, warnings, nil } -func validateAzure(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { +func validateAzure(m *Machine, clusterID string) (bool, []string, utilerrors.Aggregate) { klog.V(3).Infof("Validating Azure providerSpec") var errs []error + var warnings []string providerSpec := new(azure.AzureMachineProviderSpec) if err := unmarshalInto(m, providerSpec); err != nil { errs = append(errs, err) - return false, utilerrors.NewAggregate(errs) - } - - if providerSpec.Location == "" { - errs = append(errs, field.Required(field.NewPath("providerSpec", "location"), "location should be set to one of the supported Azure regions")) + return false, warnings, utilerrors.NewAggregate(errs) } if providerSpec.VMSize == "" { @@ -757,21 +670,8 @@ func validateAzure(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { errs = append(errs, field.Required(field.NewPath("providerSpec", "vnet"), "must provide a virtual network when supplying subnets")) } - // Vnet + Subnet requires NetworkResourceGroup - if (providerSpec.Vnet != "" || providerSpec.Subnet != "") && providerSpec.NetworkResourceGroup == "" { - errs = append(errs, field.Required(field.NewPath("providerSpec", "networkResourceGroup"), "must provide a network resource group when a virtual network or subnet is specified")) - } - errs = append(errs, validateAzureImage(providerSpec.Image)...) - if providerSpec.ManagedIdentity == "" { - errs = append(errs, field.Required(field.NewPath("providerSpec", "managedIdentity"), "managedIdentity must be provided")) - } - - if providerSpec.ResourceGroup == "" { - errs = append(errs, field.Required(field.NewPath("providerSpec", "resourceGroup"), "resourceGroup must be provided")) - } - if providerSpec.UserDataSecret == nil { errs = append(errs, field.Required(field.NewPath("providerSpec", "userDataSecret"), "userDataSecret must be provided")) } else if providerSpec.UserDataSecret.Name == "" { @@ -789,21 +689,14 @@ func validateAzure(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { } } - if providerSpec.OSDisk.DiskSizeGB <= 0 { - errs = append(errs, field.Invalid(field.NewPath("providerSpec", "osDisk", "diskSizeGB"), providerSpec.OSDisk.DiskSizeGB, "diskSizeGB must be greater than zero")) - } - - if providerSpec.OSDisk.OSType == "" { - errs = append(errs, field.Required(field.NewPath("providerSpec", "osDisk", "osType"), "osType must be provided")) - } - if providerSpec.OSDisk.ManagedDisk.StorageAccountType == "" { - errs = append(errs, field.Required(field.NewPath("providerSpec", "osDisk", "managedDisk", "storageAccountType"), "storageAccountType must be provided")) + if providerSpec.OSDisk.DiskSizeGB <= 0 || providerSpec.OSDisk.DiskSizeGB >= azureMaxDiskSizeGB { + errs = append(errs, field.Invalid(field.NewPath("providerSpec", "osDisk", "diskSizeGB"), providerSpec.OSDisk.DiskSizeGB, "diskSizeGB must be greater than zero and less than 32768")) } if len(errs) > 0 { - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } - return true, nil + return true, warnings, nil } func validateAzureImage(image azure.Image) []error { @@ -836,18 +729,15 @@ func validateAzureImage(image azure.Image) []error { return errors } -type gcpDefaulter struct { - projectID string -} - -func (g gcpDefaulter) defaultGCP(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { +func defaultGCP(m *Machine, clusterID string) (bool, []string, utilerrors.Aggregate) { klog.V(3).Infof("Defaulting GCP providerSpec") var errs []error + var warnings []string providerSpec := new(gcp.GCPMachineProviderSpec) if err := unmarshalInto(m, providerSpec); err != nil { errs = append(errs, err) - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } if providerSpec.MachineType == "" { @@ -875,21 +765,17 @@ func (g gcpDefaulter) defaultGCP(m *Machine, clusterID string) (bool, utilerrors providerSpec.CredentialsSecret = &corev1.LocalObjectReference{Name: defaultGCPCredentialsSecret} } - if len(providerSpec.ServiceAccounts) == 0 { - providerSpec.ServiceAccounts = defaultGCPServiceAccounts(clusterID, g.projectID) - } - rawBytes, err := json.Marshal(providerSpec) if err != nil { errs = append(errs, err) } if len(errs) > 0 { - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } m.Spec.ProviderSpec.Value = &runtime.RawExtension{Raw: rawBytes} - return true, nil + return true, warnings, nil } func defaultGCPDisks(disks []*gcp.GCPDisk, clusterID string) []*gcp.GCPDisk { @@ -918,14 +804,15 @@ func defaultGCPDisks(disks []*gcp.GCPDisk, clusterID string) []*gcp.GCPDisk { return disks } -func validateGCP(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { +func validateGCP(m *Machine, clusterID string) (bool, []string, utilerrors.Aggregate) { klog.V(3).Infof("Validating GCP providerSpec") var errs []error + var warnings []string providerSpec := new(gcp.GCPMachineProviderSpec) if err := unmarshalInto(m, providerSpec); err != nil { errs = append(errs, err) - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } if providerSpec.Region == "" { @@ -942,7 +829,12 @@ func validateGCP(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { errs = append(errs, validateGCPNetworkInterfaces(providerSpec.NetworkInterfaces, field.NewPath("providerSpec", "networkInterfaces"))...) errs = append(errs, validateGCPDisks(providerSpec.Disks, field.NewPath("providerSpec", "disks"))...) - errs = append(errs, validateGCPServiceAccounts(providerSpec.ServiceAccounts, field.NewPath("providerSpec", "serviceAccounts"))...) + + if len(providerSpec.ServiceAccounts) == 0 { + warnings = append(warnings, "providerSpec.serviceAccounts: no service account provided: nodes may be unable to join the cluster") + } else { + errs = append(errs, validateGCPServiceAccounts(providerSpec.ServiceAccounts, field.NewPath("providerSpec", "serviceAccounts"))...) + } if providerSpec.UserDataSecret == nil { errs = append(errs, field.Required(field.NewPath("providerSpec", "userDataSecret"), "userDataSecret must be provided")) @@ -961,9 +853,9 @@ func validateGCP(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { } if len(errs) > 0 { - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } - return true, nil + return true, warnings, nil } func validateGCPNetworkInterfaces(networkInterfaces []*gcp.GCPNetworkInterface, parentPath *field.Path) []error { @@ -1035,14 +927,15 @@ func validateGCPServiceAccounts(serviceAccounts []gcp.GCPServiceAccount, parentP return errs } -func defaultVSphere(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { +func defaultVSphere(m *Machine, clusterID string) (bool, []string, utilerrors.Aggregate) { klog.V(3).Infof("Defaulting vSphere providerSpec") var errs []error + var warnings []string providerSpec := new(vsphere.VSphereMachineProviderSpec) if err := unmarshalInto(m, providerSpec); err != nil { errs = append(errs, err) - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } if providerSpec.UserDataSecret == nil { @@ -1053,58 +946,48 @@ func defaultVSphere(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { providerSpec.CredentialsSecret = &corev1.LocalObjectReference{Name: defaultVSphereCredentialsSecret} } - // Default values for number of cpu, memory and disk size come from installer - // https://github.com/openshift/installer/blob/0ceffc5c737b49ab59441e2fd02f51f997d54a53/pkg/asset/machines/worker.go#L134 - if providerSpec.NumCPUs == 0 { - providerSpec.NumCPUs = minVSphereCPU - } - - if providerSpec.MemoryMiB == 0 { - providerSpec.MemoryMiB = minVSphereMemoryMiB - } - - if providerSpec.DiskGiB == 0 { - providerSpec.DiskGiB = minVSphereDiskGiB - } - rawBytes, err := json.Marshal(providerSpec) if err != nil { errs = append(errs, err) } if len(errs) > 0 { - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } m.Spec.ProviderSpec.Value = &runtime.RawExtension{Raw: rawBytes} - return true, nil + return true, warnings, nil } -func validateVSphere(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { +func validateVSphere(m *Machine, clusterID string) (bool, []string, utilerrors.Aggregate) { klog.V(3).Infof("Validating vSphere providerSpec") var errs []error + var warnings []string providerSpec := new(vsphere.VSphereMachineProviderSpec) if err := unmarshalInto(m, providerSpec); err != nil { errs = append(errs, err) - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } if providerSpec.Template == "" { errs = append(errs, field.Required(field.NewPath("providerSpec", "template"), "template must be provided")) } - errs = append(errs, validateVSphereWorkspace(providerSpec.Workspace, field.NewPath("providerSpec", "workspace"))...) + workspaceWarnings, workspaceErrors := validateVSphereWorkspace(providerSpec.Workspace, field.NewPath("providerSpec", "workspace")) + warnings = append(warnings, workspaceWarnings...) + errs = append(errs, workspaceErrors...) + errs = append(errs, validateVSphereNetwork(providerSpec.Network, field.NewPath("providerSpec", "network"))...) - if providerSpec.NumCPUs != 0 && providerSpec.NumCPUs < minVSphereCPU { - errs = append(errs, field.Invalid(field.NewPath("providerSpec", "numCPUs"), providerSpec.NumCPUs, fmt.Sprintf("numCPUs is below minimum value (%d)", minVSphereCPU))) + if providerSpec.NumCPUs < minVSphereCPU { + warnings = append(warnings, fmt.Sprintf("providerSpec.numCPUs: %d is less than the minimum value (%d): the minimum value will be used instead", providerSpec.NumCPUs, minVSphereCPU)) } - if providerSpec.MemoryMiB != 0 && providerSpec.MemoryMiB < minVSphereMemoryMiB { - errs = append(errs, field.Invalid(field.NewPath("providerSpec", "memoryMiB"), providerSpec.MemoryMiB, fmt.Sprintf("memoryMiB is below minimum value (%d)", minVSphereMemoryMiB))) + if providerSpec.MemoryMiB < minVSphereMemoryMiB { + warnings = append(warnings, fmt.Sprintf("providerSpec.memoryMiB: %d is less than the recommended minimum value (%d): nodes may not boot correctly", providerSpec.MemoryMiB, minVSphereMemoryMiB)) } - if providerSpec.DiskGiB != 0 && providerSpec.DiskGiB < minVSphereDiskGiB { - errs = append(errs, field.Invalid(field.NewPath("providerSpec", "diskGiB"), providerSpec.DiskGiB, fmt.Sprintf("diskGiB is below minimum value (%d)", minVSphereDiskGiB))) + if providerSpec.DiskGiB < minVSphereDiskGiB { + warnings = append(warnings, fmt.Sprintf("providerSpec.diskGiB: %d is less than the recommended minimum (%d): nodes may fail to start if disk size is too low", providerSpec.DiskGiB, minVSphereDiskGiB)) } if providerSpec.UserDataSecret == nil { @@ -1124,22 +1007,23 @@ func validateVSphere(m *Machine, clusterID string) (bool, utilerrors.Aggregate) } if len(errs) > 0 { - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } - return true, nil + return true, warnings, nil } -func validateVSphereWorkspace(workspace *vsphere.Workspace, parentPath *field.Path) []error { +func validateVSphereWorkspace(workspace *vsphere.Workspace, parentPath *field.Path) ([]string, []error) { if workspace == nil { - return []error{field.Required(parentPath, "workspace must be provided")} + return []string{}, []error{field.Required(parentPath, "workspace must be provided")} } var errs []error + var warnings []string if workspace.Server == "" { errs = append(errs, field.Required(parentPath.Child("server"), "server must be provided")) } if workspace.Datacenter == "" { - errs = append(errs, field.Required(parentPath.Child("datacenter"), "datacenter must be provided")) + warnings = append(warnings, fmt.Sprintf("%s: datacenter is unset: if more than one datacenter is present, VMs cannot be created", parentPath.Child("datacenter"))) } if workspace.Folder != "" { expectedPrefix := fmt.Sprintf("/%s/vm/", workspace.Datacenter) @@ -1149,7 +1033,7 @@ func validateVSphereWorkspace(workspace *vsphere.Workspace, parentPath *field.Pa } } - return errs + return warnings, errs } func validateVSphereNetwork(network vsphere.NetworkSpec, parentPath *field.Path) []error { diff --git a/vendor/github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1/machineset_webhook.go b/vendor/github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1/machineset_webhook.go index 6dd89985518..2c313c4af67 100644 --- a/vendor/github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1/machineset_webhook.go +++ b/vendor/github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1/machineset_webhook.go @@ -73,11 +73,12 @@ func (h *machineSetValidatorHandler) Handle(ctx context.Context, req admission.R klog.V(3).Infof("Validate webhook called for MachineSet: %s", ms.GetName()) - if ok, err := h.validateMachineSet(ms); !ok { - return admission.Denied(err.Error()) + ok, warnings, errs := h.validateMachineSet(ms) + if !ok { + return responseWithWarnings(admission.Denied(errs.Error()), warnings) } - return admission.Allowed("MachineSet valid") + return responseWithWarnings(admission.Allowed("MachineSet valid"), warnings) } // Handle handles HTTP requests for admission webhook servers. @@ -90,40 +91,43 @@ func (h *machineSetDefaulterHandler) Handle(ctx context.Context, req admission.R klog.V(3).Infof("Mutate webhook called for MachineSet: %s", ms.GetName()) - if ok, err := h.defaultMachineSet(ms); !ok { - return admission.Denied(err.Error()) + ok, warnings, errs := h.defaultMachineSet(ms) + if !ok { + return responseWithWarnings(admission.Denied(errs.Error()), warnings) } marshaledMachineSet, err := json.Marshal(ms) if err != nil { - return admission.Errored(http.StatusInternalServerError, err) + return responseWithWarnings(admission.Errored(http.StatusInternalServerError, err), warnings) } - return admission.PatchResponseFromRaw(req.Object.Raw, marshaledMachineSet) + return responseWithWarnings(admission.PatchResponseFromRaw(req.Object.Raw, marshaledMachineSet), warnings) } -func (h *machineSetValidatorHandler) validateMachineSet(ms *MachineSet) (bool, utilerrors.Aggregate) { +func (h *machineSetValidatorHandler) validateMachineSet(ms *MachineSet) (bool, []string, utilerrors.Aggregate) { var errs []error // Create a Machine from the MachineSet and validate the Machine template m := &Machine{Spec: ms.Spec.Template.Spec} - if ok, err := h.webhookOperations(m, h.clusterID); !ok { + ok, warnings, err := h.webhookOperations(m, h.clusterID) + if !ok { errs = append(errs, err.Errors()...) } if len(errs) > 0 { - return false, utilerrors.NewAggregate(errs) + return false, warnings, utilerrors.NewAggregate(errs) } - return true, nil + return true, warnings, nil } -func (h *machineSetDefaulterHandler) defaultMachineSet(ms *MachineSet) (bool, utilerrors.Aggregate) { +func (h *machineSetDefaulterHandler) defaultMachineSet(ms *MachineSet) (bool, []string, utilerrors.Aggregate) { // Create a Machine from the MachineSet and default the Machine template m := &Machine{Spec: ms.Spec.Template.Spec} - if ok, err := h.webhookOperations(m, h.clusterID); !ok { - return false, utilerrors.NewAggregate(err.Errors()) + ok, warnings, err := h.webhookOperations(m, h.clusterID) + if !ok { + return false, warnings, utilerrors.NewAggregate(err.Errors()) } // Restore the defaulted template ms.Spec.Template.Spec = m.Spec - return true, nil + return true, warnings, nil } diff --git a/vendor/github.com/openshift/machine-api-operator/pkg/controller/machine/controller.go b/vendor/github.com/openshift/machine-api-operator/pkg/controller/machine/controller.go index 9283d5ce4cb..ed684ea8d94 100644 --- a/vendor/github.com/openshift/machine-api-operator/pkg/controller/machine/controller.go +++ b/vendor/github.com/openshift/machine-api-operator/pkg/controller/machine/controller.go @@ -27,6 +27,7 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -434,7 +435,16 @@ func (r *ReconcileMachine) setPhase(machine *machinev1.Machine, phase string, er } // Since we may have mutated the local copy of the machine above, we need to calculate baseToPatch here. + // Any updates to the status must be done after this point. baseToPatch := client.MergeFrom(machine.DeepCopy()) + + if phase == phaseFailed { + if err := r.overrideFailedMachineProviderStatusState(machine); err != nil { + klog.Errorf("Failed to update machine provider status %q: %v", machine.GetName(), err) + return err + } + } + machine.Status.Phase = &phase machine.Status.ErrorMessage = nil now := metav1.Now() @@ -462,6 +472,45 @@ func (r *ReconcileMachine) patchFailedMachineInstanceAnnotation(machine *machine return nil } +// overrideFailedMachineProviderStatusState patches the state of the VM in the provider status if it is set. +// Not all providers set a state, but AWS, Azure, GCP and vSphere do. +// If the machine has gone into the Failed phase, and the providerStatus has already been set, +// the VM is in an unknown state. This function overrides the state. +func (r *ReconcileMachine) overrideFailedMachineProviderStatusState(machine *machinev1.Machine) error { + if machine.Status.ProviderStatus == nil { + return nil + } + + // instanceState is used by AWS, GCP and vSphere; vmState is used by Azure. + const instanceStateField = "instanceState" + const vmStateField = "vmState" + + providerStatus, err := runtime.DefaultUnstructuredConverter.ToUnstructured(machine.Status.ProviderStatus) + if err != nil { + return fmt.Errorf("could not covert provider status to unstructured: %v", err) + } + + // if the instanceState is set already, update it to unknown + if _, found, err := unstructured.NestedString(providerStatus, instanceStateField); err == nil && found { + if err := unstructured.SetNestedField(providerStatus, unknownInstanceState, instanceStateField); err != nil { + return fmt.Errorf("could not set %s: %v", instanceStateField, err) + } + } + + // if the vmState is set already, update it to unknown + if _, found, err := unstructured.NestedString(providerStatus, vmStateField); err == nil && found { + if err := unstructured.SetNestedField(providerStatus, unknownInstanceState, vmStateField); err != nil { + return fmt.Errorf("could not set %s: %v", instanceStateField, err) + } + } + + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(providerStatus, machine.Status.ProviderStatus); err != nil { + return fmt.Errorf("could not convert provider status from unstructured: %v", err) + } + + return nil +} + func machineIsProvisioned(machine *machinev1.Machine) bool { return len(machine.Status.Addresses) > 0 || stringPointerDeref(machine.Spec.ProviderID) != "" } diff --git a/vendor/modules.txt b/vendor/modules.txt index b5a655165e9..4529cd24c5f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -176,7 +176,7 @@ github.com/openshift/client-go/config/clientset/versioned/scheme github.com/openshift/client-go/config/clientset/versioned/typed/config/v1 # github.com/openshift/cluster-api-provider-gcp v0.0.1-0.20200901173901-9056dbc8c9b9 github.com/openshift/cluster-api-provider-gcp/pkg/apis/gcpprovider/v1beta1 -# github.com/openshift/machine-api-operator v0.2.1-0.20200922150054-e0db6b65ba71 +# github.com/openshift/machine-api-operator v0.2.1-0.20200926044412-b7d860f8074c github.com/openshift/machine-api-operator/pkg/apis/machine github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1 github.com/openshift/machine-api-operator/pkg/apis/vsphereprovider/v1beta1