From b6c9bb93797987cfbf5853aa92a955e0eafd1b3d Mon Sep 17 00:00:00 2001 From: Enxebre Date: Wed, 22 Jul 2020 12:59:58 +0200 Subject: [PATCH] UPSTREAM: : openshift: Revendor mao to bring https://github.com/openshift/machine-api-operator/pull/644 This is to bring https://github.com/openshift/machine-api-operator/pull/644 and so restoring the previous backend behaviour in the machine controller https://github.com/openshift/machine-api-operator/pull/644/commits/261c3373e317fe3c9736d48c1deb96b87be8f492 --- go.mod | 3 +- go.sum | 4 + .../pkg/apis/machine/v1beta1/machine_types.go | 9 +- .../apis/machine/v1beta1/machine_webhook.go | 220 +++++++++++++++++- .../machine/v1beta1/machineset_webhook.go | 14 ++ .../pkg/controller/machine/controller.go | 27 --- vendor/modules.txt | 4 +- 7 files changed, 246 insertions(+), 35 deletions(-) diff --git a/go.mod b/go.mod index 0c04293ae5e..5c72e9bf5b7 100644 --- a/go.mod +++ b/go.mod @@ -13,10 +13,9 @@ require ( github.com/onsi/ginkgo v1.12.0 github.com/onsi/gomega v1.8.1 github.com/openshift/api v0.0.0-20200424083944-0422dc17083e - github.com/openshift/machine-api-operator v0.2.1-0.20200701225707-950912b03628 + github.com/openshift/machine-api-operator v0.2.1-0.20200722104429-f4f9b84df9b7 github.com/spf13/cobra v0.0.5 golang.org/x/crypto v0.0.0-20200220183623-bac4c82f6975 - golang.org/x/text v0.3.3 // indirect // kube 1.18 k8s.io/api v0.18.2 diff --git a/go.sum b/go.sum index 98213c288f0..72fdef98506 100644 --- a/go.sum +++ b/go.sum @@ -352,11 +352,15 @@ github.com/openshift/cluster-api-provider-aws v0.2.1-0.20200618031251-e16dd65fdd github.com/openshift/cluster-api-provider-azure v0.1.0-alpha.3.0.20200618001858-af08a66b92de/go.mod h1:XTKOAoCAzFtgljQ9HUs4KeTyczeFUoXa0S9+0R2WCmk= github.com/openshift/cluster-api-provider-gcp v0.0.1-0.20200701112720-3a7d727c9a10 h1:AZczulAAkBuMm058BGtk5id9lhupszVQXvZFdKmSHEE= github.com/openshift/cluster-api-provider-gcp v0.0.1-0.20200701112720-3a7d727c9a10/go.mod h1:wgkZrOlcIMWTzo8khB4Js2PoDJDlIUUdzCBm7BuDdqw= +github.com/openshift/cluster-api-provider-gcp v0.0.1-0.20200713133651-5c8a640669ac h1:j4kWMuCD5Yvwa3LDXy0tN0ys24jDbQnQbCl0oRTBb6I= +github.com/openshift/cluster-api-provider-gcp v0.0.1-0.20200713133651-5c8a640669ac/go.mod h1:XVYX9JE339nKbDDa/W481XD+1GTeqeaBm8bDPr7WE7I= github.com/openshift/library-go v0.0.0-20200512120242-21a1ff978534/go.mod h1:2kWwXTkpoQJUN3jZ3QW88EIY1hdRMqxgRs2hheEW/pg= github.com/openshift/machine-api-operator v0.2.1-0.20200527204437-14e5e0c7d862/go.mod h1:YKEQMHjXzrzm4fQGTyHBafFfQ/Yq/FrV+1YcGdPCp+0= github.com/openshift/machine-api-operator v0.2.1-0.20200611014855-9a69f85c32dd/go.mod h1:6vMi+R3xqznBdq5rgeal9N3ak3sOpy50t0fdRCcQXjE= github.com/openshift/machine-api-operator v0.2.1-0.20200701225707-950912b03628 h1:8ErZADoEg3XJBqY8tM9eUEJ9szb7jQHNHSlw2j9Gr5Y= 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/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_types.go b/vendor/github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1/machine_types.go index 5cf1fc29687..ff690375605 100644 --- a/vendor/github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1/machine_types.go +++ b/vendor/github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1/machine_types.go @@ -17,6 +17,8 @@ limitations under the License. package v1beta1 import ( + "fmt" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -194,8 +196,13 @@ type LastOperation struct { func (m *Machine) Validate() field.ErrorList { errors := field.ErrorList{} - // validate provider config is set + // validate spec.labels fldPath := field.NewPath("spec") + if m.Labels[MachineClusterIDLabel] == "" { + errors = append(errors, field.Invalid(fldPath.Child("labels"), m.Labels, fmt.Sprintf("missing %v label.", MachineClusterIDLabel))) + } + + // validate provider config is set if m.Spec.ProviderSpec.Value == nil { errors = append(errors, field.Invalid(fldPath.Child("spec").Child("providerspec"), m.Spec.ProviderSpec, "value field must be set")) } 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 d5a9b2d6e24..4dab8e2abed 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 @@ -10,7 +10,9 @@ import ( osconfigv1 "github.com/openshift/api/config/v1" osclientset "github.com/openshift/client-go/config/clientset/versioned" gcp "github.com/openshift/cluster-api-provider-gcp/pkg/apis/gcpprovider/v1beta1" + "github.com/openshift/machine-api-operator/pkg/apis/machine" vsphere "github.com/openshift/machine-api-operator/pkg/apis/vsphereprovider/v1beta1" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -84,6 +86,15 @@ var ( ) const ( + DefaultMachineMutatingHookPath = "/mutate-machine-openshift-io-v1beta1-machine" + DefaultMachineValidatingHookPath = "/validate-machine-openshift-io-v1beta1-machine" + DefaultMachineSetMutatingHookPath = "/mutate-machine-openshift-io-v1beta1-machineset" + DefaultMachineSetValidatingHookPath = "/validate-machine-openshift-io-v1beta1-machineset" + + defaultWebhookConfigurationName = "machine-api" + defaultWebhookServiceName = "machine-api-operator-webhook" + defaultWebhookServiceNamespace = "openshift-machine-api" + defaultUserDataSecret = "worker-user-data" defaultSecretNamespace = "openshift-machine-api" @@ -110,6 +121,13 @@ const ( minVSphereMemoryMiB = 2048 ) +var ( + // webhookFailurePolicy is ignore so we don't want to block machine lifecycle on the webhook operational aspects. + // This would be particularly problematic for chicken egg issues when bootstrapping a cluster. + webhookFailurePolicy = admissionregistrationv1.Ignore + webhookSideEffects = admissionregistrationv1.SideEffectClassNone +) + func getInfra() (*osconfigv1.Infrastructure, error) { cfg, err := ctrl.GetConfig() if err != nil { @@ -213,7 +231,11 @@ func createMachineDefaulter(platformStatus *osconfigv1.PlatformStatus, clusterID func getMachineDefaulterOperation(platformStatus *osconfigv1.PlatformStatus) machineAdmissionFn { switch platformStatus.Type { case osconfigv1.AWSPlatformType: - return defaultAWS + region := "" + if platformStatus.AWS != nil { + region = platformStatus.AWS.Region + } + return awsDefaulter{region: region}.defaultAWS case osconfigv1.AzurePlatformType: return defaultAzure case osconfigv1.GCPPlatformType: @@ -232,6 +254,170 @@ func getMachineDefaulterOperation(platformStatus *osconfigv1.PlatformStatus) mac } } +// NewValidatingWebhookConfiguration creates a validation webhook configuration with configured Machine and MachineSet webhooks +func NewValidatingWebhookConfiguration() *admissionregistrationv1.ValidatingWebhookConfiguration { + validatingWebhookConfiguration := &admissionregistrationv1.ValidatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultWebhookConfigurationName, + Annotations: map[string]string{ + "service.beta.openshift.io/inject-cabundle": "true", + }, + }, + Webhooks: []admissionregistrationv1.ValidatingWebhook{ + MachineValidatingWebhook(), + MachineSetValidatingWebhook(), + }, + } + + // Setting group version is required for testEnv to create unstructured objects, as the new structure sets it on empty strings + // Usual way to populate those values, is to create the resource in the cluster first, which we can't yet do. + validatingWebhookConfiguration.SetGroupVersionKind(admissionregistrationv1.SchemeGroupVersion.WithKind("ValidatingWebhookConfiguration")) + return validatingWebhookConfiguration +} + +// MachineValidatingWebhook returns validating webhooks for machine to populate the configuration +func MachineValidatingWebhook() admissionregistrationv1.ValidatingWebhook { + serviceReference := admissionregistrationv1.ServiceReference{ + Namespace: defaultWebhookServiceNamespace, + Name: defaultWebhookServiceName, + Path: pointer.StringPtr(DefaultMachineValidatingHookPath), + } + return admissionregistrationv1.ValidatingWebhook{ + AdmissionReviewVersions: []string{"v1beta1"}, + Name: "validation.machine.machine.openshift.io", + FailurePolicy: &webhookFailurePolicy, + SideEffects: &webhookSideEffects, + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + Service: &serviceReference, + }, + Rules: []admissionregistrationv1.RuleWithOperations{ + { + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{machine.GroupName}, + APIVersions: []string{SchemeGroupVersion.Version}, + Resources: []string{"machines"}, + }, + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.Create, + admissionregistrationv1.Update, + }, + }, + }, + } +} + +// MachineSetValidatingWebhook returns validating webhooks for machineSet to populate the configuration +func MachineSetValidatingWebhook() admissionregistrationv1.ValidatingWebhook { + machinesetServiceReference := admissionregistrationv1.ServiceReference{ + Namespace: defaultWebhookServiceNamespace, + Name: defaultWebhookServiceName, + Path: pointer.StringPtr(DefaultMachineSetValidatingHookPath), + } + return admissionregistrationv1.ValidatingWebhook{ + AdmissionReviewVersions: []string{"v1beta1"}, + Name: "validation.machineset.machine.openshift.io", + FailurePolicy: &webhookFailurePolicy, + SideEffects: &webhookSideEffects, + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + Service: &machinesetServiceReference, + }, + Rules: []admissionregistrationv1.RuleWithOperations{ + { + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{machine.GroupName}, + APIVersions: []string{SchemeGroupVersion.Version}, + Resources: []string{"machinesets"}, + }, + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.Create, + admissionregistrationv1.Update, + }, + }, + }, + } +} + +// NewMutatingWebhookConfiguration creates a mutating webhook configuration with configured Machine and MachineSet webhooks +func NewMutatingWebhookConfiguration() *admissionregistrationv1.MutatingWebhookConfiguration { + mutatingWebhookConfiguration := &admissionregistrationv1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultWebhookConfigurationName, + Annotations: map[string]string{ + "service.beta.openshift.io/inject-cabundle": "true", + }, + }, + Webhooks: []admissionregistrationv1.MutatingWebhook{ + MachineMutatingWebhook(), + MachineSetMutatingWebhook(), + }, + } + + // Setting group version is required for testEnv to create unstructured objects, as the new structure sets it on empty strings + // Usual way to populate those values, is to create the resource in the cluster first, which we can't yet do. + mutatingWebhookConfiguration.SetGroupVersionKind(admissionregistrationv1.SchemeGroupVersion.WithKind("MutatingWebhookConfiguration")) + return mutatingWebhookConfiguration +} + +// MachineMutatingWebhook returns mutating webhooks for machine to apply in configuration +func MachineMutatingWebhook() admissionregistrationv1.MutatingWebhook { + machineServiceReference := admissionregistrationv1.ServiceReference{ + Namespace: defaultWebhookServiceNamespace, + Name: defaultWebhookServiceName, + Path: pointer.StringPtr(DefaultMachineMutatingHookPath), + } + return admissionregistrationv1.MutatingWebhook{ + AdmissionReviewVersions: []string{"v1beta1"}, + Name: "default.machine.machine.openshift.io", + FailurePolicy: &webhookFailurePolicy, + SideEffects: &webhookSideEffects, + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + Service: &machineServiceReference, + }, + Rules: []admissionregistrationv1.RuleWithOperations{ + { + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{machine.GroupName}, + APIVersions: []string{SchemeGroupVersion.Version}, + Resources: []string{"machines"}, + }, + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.Create, + }, + }, + }, + } +} + +// MachineSetMutatingWebhook returns mutating webhook for machineSet to apply in configuration +func MachineSetMutatingWebhook() admissionregistrationv1.MutatingWebhook { + machineSetServiceReference := admissionregistrationv1.ServiceReference{ + Namespace: defaultWebhookServiceNamespace, + Name: defaultWebhookServiceName, + Path: pointer.StringPtr(DefaultMachineSetMutatingHookPath), + } + return admissionregistrationv1.MutatingWebhook{ + AdmissionReviewVersions: []string{"v1beta1"}, + Name: "default.machineset.machine.openshift.io", + FailurePolicy: &webhookFailurePolicy, + SideEffects: &webhookSideEffects, + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + Service: &machineSetServiceReference, + }, + Rules: []admissionregistrationv1.RuleWithOperations{ + { + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{machine.GroupName}, + APIVersions: []string{SchemeGroupVersion.Version}, + Resources: []string{"machinesets"}, + }, + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.Create, + }, + }, + }, + } +} + // Handle handles HTTP requests for admission webhook servers. func (h *machineValidatorHandler) Handle(ctx context.Context, req admission.Request) admission.Response { m := &Machine{} @@ -259,6 +445,15 @@ func (h *machineDefaulterHandler) Handle(ctx context.Context, req admission.Requ klog.V(3).Infof("Mutate webhook called for Machine: %s", m.GetName()) + // Enforce that the same clusterID is set for machineSet Selector and machine labels. + // Otherwise a discrepancy on the value would leave the machine orphan + // and would trigger a new machine creation by the machineSet. + // https://bugzilla.redhat.com/show_bug.cgi?id=1857175 + if m.Labels == nil { + m.Labels = make(map[string]string) + } + m.Labels[MachineClusterIDLabel] = h.clusterID + if ok, err := h.webhookOperations(m, h.clusterID); !ok { return admission.Denied(err.Error()) } @@ -270,7 +465,11 @@ func (h *machineDefaulterHandler) Handle(ctx context.Context, req admission.Requ return admission.PatchResponseFromRaw(req.Object.Raw, marshaledMachine) } -func defaultAWS(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { +type awsDefaulter struct { + region string +} + +func (a awsDefaulter) defaultAWS(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { klog.V(3).Infof("Defaulting AWS providerSpec") var errs []error @@ -286,6 +485,11 @@ func defaultAWS(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { if providerSpec.IAMInstanceProfile == nil { providerSpec.IAMInstanceProfile = &aws.AWSResourceReference{ID: defaultAWSIAMInstanceProfile(clusterID)} } + + if providerSpec.Placement.Region == "" { + providerSpec.Placement.Region = a.region + } + if providerSpec.UserDataSecret == nil { providerSpec.UserDataSecret = &corev1.LocalObjectReference{Name: defaultUserDataSecret} } @@ -307,7 +511,7 @@ func defaultAWS(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { } } - if providerSpec.Subnet.ARN == nil && providerSpec.Subnet.ID == nil && providerSpec.Subnet.Filters == nil { + if providerSpec.Subnet.ARN == nil && providerSpec.Subnet.ID == nil && providerSpec.Subnet.Filters == nil && providerSpec.Placement.AvailabilityZone != "" { providerSpec.Subnet.Filters = []aws.Filter{ { Name: "tag:Name", @@ -360,6 +564,16 @@ func validateAWS(m *Machine, clusterID string) (bool, utilerrors.Aggregate) { ) } + if providerSpec.Placement.Region == "" { + errs = append( + errs, + field.Required( + field.NewPath("providerSpec", "placement", "region"), + "expected providerSpec.placement.region to be populated", + ), + ) + } + if providerSpec.InstanceType == "" { errs = append( errs, 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 595f406028a..6e3bd90cfd8 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 @@ -124,6 +124,20 @@ func (h *machineSetDefaulterHandler) defaultMachineSet(ms *MachineSet) (bool, ut if ok, err := h.webhookOperations(m, h.clusterID); !ok { errs = append(errs, err.Errors()...) } else { + // Enforce that the same clusterID is set for machineSet Selector and machine labels. + // Otherwise a discrepancy on the value would leave the machine orphan + // and would trigger a new machine creation by the machineSet. + // https://bugzilla.redhat.com/show_bug.cgi?id=1857175 + if ms.Spec.Selector.MatchLabels == nil { + ms.Spec.Selector.MatchLabels = make(map[string]string) + } + ms.Spec.Selector.MatchLabels[MachineClusterIDLabel] = h.clusterID + + if ms.Spec.Template.Labels == nil { + ms.Spec.Template.Labels = make(map[string]string) + } + ms.Spec.Template.Labels[MachineClusterIDLabel] = h.clusterID + // Restore the defaulted template ms.Spec.Template.Spec = m.Spec } 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 22b4768685f..8b84b7e0aba 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 @@ -22,7 +22,6 @@ import ( "fmt" "time" - configv1 "github.com/openshift/api/config/v1" machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" "github.com/openshift/machine-api-operator/pkg/util" corev1 "k8s.io/api/core/v1" @@ -92,8 +91,6 @@ const ( unknownInstanceState = "Unknown" skipWaitForDeleteTimeoutSeconds = 60 * 5 - - globalInfrastuctureName = "cluster" ) var DefaultActuator Actuator @@ -178,11 +175,6 @@ func (r *ReconcileMachine) Reconcile(request reconcile.Request) (reconcile.Resul return reconcile.Result{}, err } - // Add clusterID label - if err := r.setClusterIDLabel(ctx, m); err != nil { - return reconcile.Result{}, err - } - // If object hasn't been deleted and doesn't have a finalizer, add one // Add a finalizer to newly created objects. if m.ObjectMeta.DeletionTimestamp.IsZero() { @@ -470,25 +462,6 @@ func (r *ReconcileMachine) patchFailedMachineInstanceAnnotation(machine *machine return nil } -func (r *ReconcileMachine) setClusterIDLabel(ctx context.Context, m *machinev1.Machine) error { - infra := &configv1.Infrastructure{} - infraName := client.ObjectKey{Name: globalInfrastuctureName} - - if err := r.Client.Get(ctx, infraName, infra); err != nil { - return err - } - - clusterID := infra.Status.InfrastructureName - - if m.Labels == nil { - m.Labels = make(map[string]string) - } - - m.Labels[machinev1.MachineClusterIDLabel] = clusterID - - 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 238d6c22d9c..23cdbd344d7 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -170,9 +170,9 @@ github.com/openshift/api/config/v1 github.com/openshift/client-go/config/clientset/versioned 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.20200701112720-3a7d727c9a10 +# github.com/openshift/cluster-api-provider-gcp v0.0.1-0.20200713133651-5c8a640669ac github.com/openshift/cluster-api-provider-gcp/pkg/apis/gcpprovider/v1beta1 -# github.com/openshift/machine-api-operator v0.2.1-0.20200701225707-950912b03628 +# github.com/openshift/machine-api-operator v0.2.1-0.20200722104429-f4f9b84df9b7 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