diff --git a/config/crd/bases/controlplane.cluster.x-k8s.io_rosacontrolplanes.yaml b/config/crd/bases/controlplane.cluster.x-k8s.io_rosacontrolplanes.yaml index d67cf97022..b38308d4f9 100644 --- a/config/crd/bases/controlplane.cluster.x-k8s.io_rosacontrolplanes.yaml +++ b/config/crd/bases/controlplane.cluster.x-k8s.io_rosacontrolplanes.yaml @@ -341,9 +341,6 @@ spec: version: description: OpenShift semantic version, for example "4.14.5". type: string - x-kubernetes-validations: - - message: version must be a valid semantic version - rule: self.matches('^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)$') workerRoleARN: type: string required: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_rosamachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_rosamachinepools.yaml index d94ef3e30d..0cc4441e88 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_rosamachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_rosamachinepools.yaml @@ -94,14 +94,48 @@ spec: type: array subnet: type: string + x-kubernetes-validations: + - message: subnet is immutable + rule: self == oldSelf + taints: + description: Taints specifies the taints to apply to the nodes of + the machine pool + items: + properties: + effect: + description: The effect of the taint on pods that do not tolerate + the taint. Valid effects are NoSchedule, PreferNoSchedule + and NoExecute. + enum: + - NoSchedule + - PreferNoSchedule + - NoExecute + type: string + key: + description: The taint key to be applied to a node. + type: string + value: + description: The taint value corresponding to the taint key. + pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$ + type: string + required: + - effect + - key + type: object + type: array + tuningConfigs: + description: TuningConfigs specifies the names of the tuning configs + to be applied to this MachinePool. Tuning configs must already exist. + items: + type: string + type: array version: - description: Version specifies the penshift version of the nodes associated - with this machinepool. ROSAControlPlane version is used if not set. + description: Version specifies the OpenShift version of the nodes + associated with this machinepool. ROSAControlPlane version is used + if not set. type: string - x-kubernetes-validations: - - message: version must be a valid semantic version - rule: self.matches('^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)$') required: + - instanceType - nodePoolName type: object status: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 33946b3057..9aa4801808 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -175,6 +175,12 @@ rules: - get - list - watch +- apiGroups: + - controlplane.cluster.x-k8s.io + resources: + - rosacontrolplanes/finalizers + verbs: + - update - apiGroups: - controlplane.cluster.x-k8s.io resources: @@ -409,6 +415,12 @@ rules: - patch - update - watch +- apiGroups: + - infrastructure.cluster.x-k8s.io + resources: + - rosamachinepools/finalizers + verbs: + - update - apiGroups: - infrastructure.cluster.x-k8s.io resources: diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 9dd74404c2..5eebfae968 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -201,6 +201,28 @@ webhooks: resources: - awsmanagedmachinepools sideEffects: None +- admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-infrastructure-cluster-x-k8s-io-v1beta2-rosamachinepool + failurePolicy: Fail + matchPolicy: Equivalent + name: default.rosamachinepool.infrastructure.cluster.x-k8s.io + rules: + - apiGroups: + - infrastructure.cluster.x-k8s.io + apiVersions: + - v1beta2 + operations: + - CREATE + - UPDATE + resources: + - rosamachinepools + sideEffects: None - admissionReviewVersions: - v1 - v1beta1 @@ -267,6 +289,28 @@ webhooks: resources: - awsmanagedcontrolplanes sideEffects: None +- admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-controlplane-cluster-x-k8s-io-v1beta2-rosacontrolplane + failurePolicy: Fail + matchPolicy: Equivalent + name: default.rosacontrolplanes.controlplane.cluster.x-k8s.io + rules: + - apiGroups: + - controlplane.cluster.x-k8s.io + apiVersions: + - v1beta2 + operations: + - CREATE + - UPDATE + resources: + - rosacontrolplanes + sideEffects: None --- apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration @@ -493,6 +537,28 @@ webhooks: resources: - awsmanagedmachinepools sideEffects: None +- admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-infrastructure-cluster-x-k8s-io-v1beta2-rosamachinepool + failurePolicy: Fail + matchPolicy: Equivalent + name: validation.rosamachinepool.infrastructure.cluster.x-k8s.io + rules: + - apiGroups: + - infrastructure.cluster.x-k8s.io + apiVersions: + - v1beta2 + operations: + - CREATE + - UPDATE + resources: + - rosamachinepools + sideEffects: None - admissionReviewVersions: - v1 - v1beta1 @@ -559,3 +625,25 @@ webhooks: resources: - awsmanagedcontrolplanes sideEffects: None +- admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-controlplane-cluster-x-k8s-io-v1beta2-rosacontrolplane + failurePolicy: Fail + matchPolicy: Equivalent + name: validation.rosacontrolplanes.controlplane.cluster.x-k8s.io + rules: + - apiGroups: + - controlplane.cluster.x-k8s.io + apiVersions: + - v1beta2 + operations: + - CREATE + - UPDATE + resources: + - rosacontrolplanes + sideEffects: None diff --git a/controlplane/rosa/api/v1beta2/conditions_consts.go b/controlplane/rosa/api/v1beta2/conditions_consts.go index 797e04a0a5..a80e534c61 100644 --- a/controlplane/rosa/api/v1beta2/conditions_consts.go +++ b/controlplane/rosa/api/v1beta2/conditions_consts.go @@ -22,6 +22,15 @@ const ( // ROSAControlPlaneReadyCondition condition reports on the successful reconciliation of ROSAControlPlane. ROSAControlPlaneReadyCondition clusterv1.ConditionType = "ROSAControlPlaneReady" + // ROSAControlPlaneValidCondition condition reports whether ROSAControlPlane configuration is valid. + ROSAControlPlaneValidCondition clusterv1.ConditionType = "ROSAControlPlaneValid" + // ROSAControlPlaneUpgradingCondition condition reports whether ROSAControlPlane is upgrading or not. ROSAControlPlaneUpgradingCondition clusterv1.ConditionType = "ROSAControlPlaneUpgrading" + + // ROSAControlPlaneReconciliationFailedReason used to report failures while reconciling ROSAControlPlane. + ROSAControlPlaneReconciliationFailedReason = "ReconciliationFailed" + + // ROSAControlPlaneInvalidConfigurationReason used to report invalid user input. + ROSAControlPlaneInvalidConfigurationReason = "InvalidConfiguration" ) diff --git a/controlplane/rosa/api/v1beta2/rosacontrolplane_types.go b/controlplane/rosa/api/v1beta2/rosacontrolplane_types.go index 65ecd9279b..27e1c27348 100644 --- a/controlplane/rosa/api/v1beta2/rosacontrolplane_types.go +++ b/controlplane/rosa/api/v1beta2/rosacontrolplane_types.go @@ -30,10 +30,10 @@ type RosaControlPlaneSpec struct { //nolint: maligned // characters or '-', start with an alphabetic character, end with an alphanumeric character // and have a max length of 15 characters. // - // +immutable // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="rosaClusterName is immutable" // +kubebuilder:validation:MaxLength:=15 // +kubebuilder:validation:Pattern:=`^[a-z]([-a-z0-9]*[a-z0-9])?$` + // +immutable RosaClusterName string `json:"rosaClusterName"` // The Subnet IDs to use when installing the cluster. @@ -45,35 +45,31 @@ type RosaControlPlaneSpec struct { //nolint: maligned AvailabilityZones []string `json:"availabilityZones"` // The AWS Region the cluster lives in. - Region *string `json:"region"` + Region string `json:"region"` // OpenShift semantic version, for example "4.14.5". - // +kubebuilder:validation:XValidation:rule=`self.matches('^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)$')`, message="version must be a valid semantic version" Version string `json:"version"` - // ControlPlaneEndpoint represents the endpoint used to communicate with the control plane. - // +optional - ControlPlaneEndpoint clusterv1.APIEndpoint `json:"controlPlaneEndpoint"` - // AWS IAM roles used to perform credential requests by the openshift operators. RolesRef AWSRolesRef `json:"rolesRef"` // The ID of the OpenID Connect Provider. - OIDCID *string `json:"oidcID"` + OIDCID string `json:"oidcID"` // TODO: these are to satisfy ocm sdk. Explore how to drop them. InstallerRoleARN *string `json:"installerRoleARN"` SupportRoleARN *string `json:"supportRoleARN"` WorkerRoleARN *string `json:"workerRoleARN"` - // +immutable - // +kubebuilder:validation:Optional - // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="billingAccount is immutable" - // +kubebuilder:validation:XValidation:rule="self.matches('^[0-9]{12}$')", message="billingAccount must be a valid AWS account ID" - // BillingAccount is an optional AWS account to use for billing the subscription fees for ROSA clusters. // The cost of running each ROSA cluster will be billed to the infrastructure account in which the cluster // is running. + // + // +kubebuilder:validation:Optional + // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="billingAccount is immutable" + // +kubebuilder:validation:XValidation:rule="self.matches('^[0-9]{12}$')", message="billingAccount must be a valid AWS account ID" + // +immutable + // +optional BillingAccount string `json:"billingAccount,omitempty"` // CredentialsSecretRef references a secret with necessary credentials to connect to the OCM API. @@ -83,13 +79,14 @@ type RosaControlPlaneSpec struct { //nolint: maligned // +optional CredentialsSecretRef *corev1.LocalObjectReference `json:"credentialsSecretRef,omitempty"` - // +optional - // IdentityRef is a reference to an identity to be used when reconciling the managed control plane. // If no identity is specified, the default identity for this controller will be used. + // + // +optional IdentityRef *infrav1.AWSIdentityReference `json:"identityRef,omitempty"` // Network config for the ROSA HCP cluster. + // +optional Network *NetworkSpec `json:"network,omitempty"` // The instance type to use, for example `r5.xlarge`. Instance type ref; https://aws.amazon.com/ec2/instance-types/ @@ -99,12 +96,17 @@ type RosaControlPlaneSpec struct { //nolint: maligned // Autoscaling specifies auto scaling behaviour for the MachinePools. // +optional Autoscaling *expinfrav1.RosaMachinePoolAutoScaling `json:"autoscaling,omitempty"` + + // ControlPlaneEndpoint represents the endpoint used to communicate with the control plane. + // +optional + ControlPlaneEndpoint clusterv1.APIEndpoint `json:"controlPlaneEndpoint"` } // NetworkSpec for ROSA-HCP. type NetworkSpec struct { // IP addresses block used by OpenShift while installing the cluster, for example "10.0.0.0/16". // +kubebuilder:validation:Format=cidr + // +optional MachineCIDR string `json:"machineCIDR,omitempty"` // IP address block from which to assign pod IP addresses, for example `10.128.0.0/14`. @@ -125,6 +127,7 @@ type NetworkSpec struct { // The CNI network type default is OVNKubernetes. // +kubebuilder:validation:Enum=OVNKubernetes;Other // +kubebuilder:default=OVNKubernetes + // +optional NetworkType string `json:"networkType,omitempty"` } @@ -533,7 +536,7 @@ type RosaControlPlaneStatus struct { Conditions clusterv1.Conditions `json:"conditions,omitempty"` // ID is the cluster ID given by ROSA. - ID *string `json:"id,omitempty"` + ID string `json:"id,omitempty"` // ConsoleURL is the url for the openshift console. ConsoleURL string `json:"consoleURL,omitempty"` // OIDCEndpointURL is the endpoint url for the managed OIDC porvider. diff --git a/controlplane/rosa/api/v1beta2/rosacontrolplane_webhook.go b/controlplane/rosa/api/v1beta2/rosacontrolplane_webhook.go new file mode 100644 index 0000000000..b13edf43d1 --- /dev/null +++ b/controlplane/rosa/api/v1beta2/rosacontrolplane_webhook.go @@ -0,0 +1,119 @@ +package v1beta2 + +import ( + "net" + + "github.com/blang/semver" + apierrors "k8s.io/apimachinery/pkg/api/errors" + runtime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// SetupWebhookWithManager will setup the webhooks for the ROSAControlPlane. +func (r *ROSAControlPlane) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(r). + Complete() +} + +// +kubebuilder:webhook:verbs=create;update,path=/validate-controlplane-cluster-x-k8s-io-v1beta2-rosacontrolplane,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=controlplane.cluster.x-k8s.io,resources=rosacontrolplanes,versions=v1beta2,name=validation.rosacontrolplanes.controlplane.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 +// +kubebuilder:webhook:verbs=create;update,path=/mutate-controlplane-cluster-x-k8s-io-v1beta2-rosacontrolplane,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=controlplane.cluster.x-k8s.io,resources=rosacontrolplanes,versions=v1beta2,name=default.rosacontrolplanes.controlplane.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 + +var _ webhook.Defaulter = &ROSAControlPlane{} +var _ webhook.Validator = &ROSAControlPlane{} + +// ValidateCreate implements admission.Validator. +func (r *ROSAControlPlane) ValidateCreate() (warnings admission.Warnings, err error) { + var allErrs field.ErrorList + + if err := r.validateVersion(); err != nil { + allErrs = append(allErrs, err) + } + + allErrs = append(allErrs, r.validateNetwork()...) + + if len(allErrs) == 0 { + return nil, nil + } + + return nil, apierrors.NewInvalid( + r.GroupVersionKind().GroupKind(), + r.Name, + allErrs, + ) +} + +// ValidateUpdate implements admission.Validator. +func (r *ROSAControlPlane) ValidateUpdate(old runtime.Object) (warnings admission.Warnings, err error) { + var allErrs field.ErrorList + + if err := r.validateVersion(); err != nil { + allErrs = append(allErrs, err) + } + + allErrs = append(allErrs, r.validateNetwork()...) + + if len(allErrs) == 0 { + return nil, nil + } + + return nil, apierrors.NewInvalid( + r.GroupVersionKind().GroupKind(), + r.Name, + allErrs, + ) +} + +// ValidateDelete implements admission.Validator. +func (r *ROSAControlPlane) ValidateDelete() (warnings admission.Warnings, err error) { + return nil, nil +} + +func (r *ROSAControlPlane) validateVersion() *field.Error { + _, err := semver.Parse(r.Spec.Version) + if err != nil { + return field.Invalid(field.NewPath("spec.version"), r.Spec.Version, "must be a valid semantic version") + } + + return nil +} + +func (r *ROSAControlPlane) validateNetwork() field.ErrorList { + var allErrs field.ErrorList + if r.Spec.Network == nil { + return allErrs + } + + rootPath := field.NewPath("spec", "network") + + if r.Spec.Network.MachineCIDR != "" { + _, _, err := net.ParseCIDR(r.Spec.Network.MachineCIDR) + if err != nil { + allErrs = append(allErrs, field.Invalid(rootPath.Child("machineCIDR"), r.Spec.Network.MachineCIDR, "must be valid CIDR block")) + } + } + + if r.Spec.Network.PodCIDR != "" { + _, _, err := net.ParseCIDR(r.Spec.Network.PodCIDR) + if err != nil { + allErrs = append(allErrs, field.Invalid(rootPath.Child("podCIDR"), r.Spec.Network.PodCIDR, "must be valid CIDR block")) + } + } + + if r.Spec.Network.ServiceCIDR != "" { + _, _, err := net.ParseCIDR(r.Spec.Network.ServiceCIDR) + if err != nil { + allErrs = append(allErrs, field.Invalid(rootPath.Child("serviceCIDR"), r.Spec.Network.ServiceCIDR, "must be valid CIDR block")) + } + } + + return allErrs +} + +// Default implements admission.Defaulter. +func (r *ROSAControlPlane) Default() { + SetObjectDefaults_ROSAControlPlane(r) +} diff --git a/controlplane/rosa/api/v1beta2/zz_generated.deepcopy.go b/controlplane/rosa/api/v1beta2/zz_generated.deepcopy.go index 41dac04354..7a972c59fe 100644 --- a/controlplane/rosa/api/v1beta2/zz_generated.deepcopy.go +++ b/controlplane/rosa/api/v1beta2/zz_generated.deepcopy.go @@ -22,7 +22,7 @@ package v1beta2 import ( "k8s.io/api/core/v1" - runtime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime" apiv1beta2 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2" expapiv1beta2 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2" "sigs.k8s.io/cluster-api/api/v1beta1" @@ -130,18 +130,7 @@ func (in *RosaControlPlaneSpec) DeepCopyInto(out *RosaControlPlaneSpec) { *out = make([]string, len(*in)) copy(*out, *in) } - if in.Region != nil { - in, out := &in.Region, &out.Region - *out = new(string) - **out = **in - } - out.ControlPlaneEndpoint = in.ControlPlaneEndpoint out.RolesRef = in.RolesRef - if in.OIDCID != nil { - in, out := &in.OIDCID, &out.OIDCID - *out = new(string) - **out = **in - } if in.InstallerRoleARN != nil { in, out := &in.InstallerRoleARN, &out.InstallerRoleARN *out = new(string) @@ -177,6 +166,7 @@ func (in *RosaControlPlaneSpec) DeepCopyInto(out *RosaControlPlaneSpec) { *out = new(expapiv1beta2.RosaMachinePoolAutoScaling) **out = **in } + out.ControlPlaneEndpoint = in.ControlPlaneEndpoint } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RosaControlPlaneSpec. @@ -209,11 +199,6 @@ func (in *RosaControlPlaneStatus) DeepCopyInto(out *RosaControlPlaneStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.ID != nil { - in, out := &in.ID, &out.ID - *out = new(string) - **out = **in - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RosaControlPlaneStatus. diff --git a/controlplane/rosa/controllers/rosacontrolplane_controller.go b/controlplane/rosa/controllers/rosacontrolplane_controller.go index 69e6e31c8a..aecf06ab2f 100644 --- a/controlplane/rosa/controllers/rosacontrolplane_controller.go +++ b/controlplane/rosa/controllers/rosacontrolplane_controller.go @@ -115,6 +115,7 @@ func (r *ROSAControlPlaneReconciler) SetupWithManager(ctx context.Context, mgr c // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinepools,verbs=get;list;watch // +kubebuilder:rbac:groups=controlplane.cluster.x-k8s.io,resources=rosacontrolplanes,verbs=get;list;watch;update;patch;delete // +kubebuilder:rbac:groups=controlplane.cluster.x-k8s.io,resources=rosacontrolplanes/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=controlplane.cluster.x-k8s.io,resources=rosacontrolplanes/finalizers,verbs=update // Reconcile will reconcile RosaControlPlane Resources. func (r *ROSAControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.Result, reterr error) { @@ -199,23 +200,29 @@ func (r *ROSAControlPlaneReconciler) reconcileNormal(ctx context.Context, rosaSc return ctrl.Result{}, fmt.Errorf("failed to transform caller identity to creator: %w", err) } - if validationMessage, validationError := validateControlPlaneSpec(ocmClient, rosaScope); validationError != nil { - return ctrl.Result{}, fmt.Errorf("validate ROSAControlPlane.spec: %w", validationError) - } else if validationMessage != "" { - rosaScope.ControlPlane.Status.FailureMessage = ptr.To(validationMessage) + validationMessage, err := validateControlPlaneSpec(ocmClient, rosaScope) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to validate ROSAControlPlane.spec: %w", err) + } + + conditions.MarkTrue(rosaScope.ControlPlane, rosacontrolplanev1.ROSAControlPlaneValidCondition) + if validationMessage != "" { + conditions.MarkFalse(rosaScope.ControlPlane, + rosacontrolplanev1.ROSAControlPlaneValidCondition, + rosacontrolplanev1.ROSAControlPlaneInvalidConfigurationReason, + clusterv1.ConditionSeverityError, + validationMessage) // dont' requeue because input is invalid and manual intervention is needed. return ctrl.Result{}, nil - } else { - rosaScope.ControlPlane.Status.FailureMessage = nil } cluster, err := ocmClient.GetCluster(rosaScope.ControlPlane.Spec.RosaClusterName, creator) - if weberr.GetType(err) != weberr.NotFound && err != nil { + if err != nil && weberr.GetType(err) != weberr.NotFound { return ctrl.Result{}, err } if cluster != nil { - rosaScope.ControlPlane.Status.ID = ptr.To(cluster.ID()) + rosaScope.ControlPlane.Status.ID = cluster.ID() rosaScope.ControlPlane.Status.ConsoleURL = cluster.Console().URL() rosaScope.ControlPlane.Status.OIDCEndpointURL = cluster.AWS().STS().OIDCEndpointURL() rosaScope.ControlPlane.Status.Ready = false @@ -267,13 +274,13 @@ func (r *ROSAControlPlaneReconciler) reconcileNormal(ctx context.Context, rosaSc billingAccount = rosaScope.ControlPlane.Spec.BillingAccount } - spec := ocm.Spec{ + ocmClusterSpec := ocm.Spec{ DryRun: ptr.To(false), Name: rosaScope.RosaClusterName(), - Region: *rosaScope.ControlPlane.Spec.Region, + Region: rosaScope.ControlPlane.Spec.Region, MultiAZ: true, Version: ocm.CreateVersionID(rosaScope.ControlPlane.Spec.Version, ocm.DefaultChannelGroup), - ChannelGroup: "stable", + ChannelGroup: ocm.DefaultChannelGroup, Expiration: time.Now().Add(1 * time.Hour), DisableWorkloadMonitoring: ptr.To(true), DefaultIngress: ocm.NewDefaultIngressSpec(), // n.b. this is a no-op when it's set to the default value @@ -281,13 +288,12 @@ func (r *ROSAControlPlaneReconciler) reconcileNormal(ctx context.Context, rosaSc SubnetIds: rosaScope.ControlPlane.Spec.Subnets, AvailabilityZones: rosaScope.ControlPlane.Spec.AvailabilityZones, - NetworkType: rosaScope.ControlPlane.Spec.Network.NetworkType, IsSTS: true, RoleARN: *rosaScope.ControlPlane.Spec.InstallerRoleARN, SupportRoleARN: *rosaScope.ControlPlane.Spec.SupportRoleARN, - OperatorIAMRoles: getOperatorIAMRole(*rosaScope.ControlPlane), WorkerRoleARN: *rosaScope.ControlPlane.Spec.WorkerRoleARN, - OidcConfigId: *rosaScope.ControlPlane.Spec.OIDCID, + OperatorIAMRoles: operatorIAMRoles(rosaScope.ControlPlane.Spec.RolesRef), + OidcConfigId: rosaScope.ControlPlane.Spec.OIDCID, Mode: "auto", Hypershift: ocm.Hypershift{ Enabled: true, @@ -296,97 +302,98 @@ func (r *ROSAControlPlaneReconciler) reconcileNormal(ctx context.Context, rosaSc AWSCreator: creator, } - _, machineCIDR, err := net.ParseCIDR(rosaScope.ControlPlane.Spec.Network.MachineCIDR) - if err == nil { - spec.MachineCIDR = *machineCIDR - } else { - // TODO: expose in status - rosaScope.Error(err, "rosacontrolplane.spec.network.machineCIDR invalid", rosaScope.ControlPlane.Spec.Network.MachineCIDR) - return ctrl.Result{}, nil - } + if networkSpec := rosaScope.ControlPlane.Spec.Network; networkSpec != nil { + if networkSpec.MachineCIDR != "" { + _, machineCIDR, err := net.ParseCIDR(networkSpec.MachineCIDR) + if err != nil { + return ctrl.Result{}, err + } + ocmClusterSpec.MachineCIDR = *machineCIDR + } - if rosaScope.ControlPlane.Spec.Network.PodCIDR != "" { - _, podCIDR, err := net.ParseCIDR(rosaScope.ControlPlane.Spec.Network.PodCIDR) - if err == nil { - spec.PodCIDR = *podCIDR - } else { - // TODO: expose in status. - rosaScope.Error(err, "rosacontrolplane.spec.network.podCIDR invalid", rosaScope.ControlPlane.Spec.Network.PodCIDR) - return ctrl.Result{}, nil + if networkSpec.PodCIDR != "" { + _, podCIDR, err := net.ParseCIDR(networkSpec.PodCIDR) + if err != nil { + return ctrl.Result{}, err + } + ocmClusterSpec.PodCIDR = *podCIDR } - } - if rosaScope.ControlPlane.Spec.Network.ServiceCIDR != "" { - _, serviceCIDR, err := net.ParseCIDR(rosaScope.ControlPlane.Spec.Network.ServiceCIDR) - if err == nil { - spec.ServiceCIDR = *serviceCIDR - } else { - // TODO: expose in status. - rosaScope.Error(err, "rosacontrolplane.spec.network.serviceCIDR invalid", rosaScope.ControlPlane.Spec.Network.ServiceCIDR) - return ctrl.Result{}, nil + if networkSpec.ServiceCIDR != "" { + _, serviceCIDR, err := net.ParseCIDR(networkSpec.ServiceCIDR) + if err != nil { + return ctrl.Result{}, err + } + ocmClusterSpec.ServiceCIDR = *serviceCIDR } + + ocmClusterSpec.HostPrefix = networkSpec.HostPrefix + ocmClusterSpec.NetworkType = networkSpec.NetworkType } // Set autoscale replica if rosaScope.ControlPlane.Spec.Autoscaling != nil { - spec.MaxReplicas = rosaScope.ControlPlane.Spec.Autoscaling.MaxReplicas - spec.MinReplicas = rosaScope.ControlPlane.Spec.Autoscaling.MinReplicas + ocmClusterSpec.MaxReplicas = rosaScope.ControlPlane.Spec.Autoscaling.MaxReplicas + ocmClusterSpec.MinReplicas = rosaScope.ControlPlane.Spec.Autoscaling.MinReplicas } - cluster, err = ocmClient.CreateCluster(spec) + cluster, err = ocmClient.CreateCluster(ocmClusterSpec) if err != nil { - // TODO: need to expose in status, as likely the spec is invalid + conditions.MarkFalse(rosaScope.ControlPlane, + rosacontrolplanev1.ROSAControlPlaneReadyCondition, + rosacontrolplanev1.ROSAControlPlaneReconciliationFailedReason, + clusterv1.ConditionSeverityError, + err.Error()) return ctrl.Result{}, fmt.Errorf("failed to create OCM cluster: %w", err) } rosaScope.Info("cluster created", "state", cluster.Status().State()) - clusterID := cluster.ID() - rosaScope.ControlPlane.Status.ID = &clusterID + rosaScope.ControlPlane.Status.ID = cluster.ID() return ctrl.Result{}, nil } -func getOperatorIAMRole(rosaControlPlane rosacontrolplanev1.ROSAControlPlane) []ocm.OperatorIAMRole { +func operatorIAMRoles(rolesRef rosacontrolplanev1.AWSRolesRef) []ocm.OperatorIAMRole { return []ocm.OperatorIAMRole{ { Name: "cloud-credentials", Namespace: "openshift-ingress-operator", - RoleARN: rosaControlPlane.Spec.RolesRef.IngressARN, + RoleARN: rolesRef.IngressARN, }, { Name: "installer-cloud-credentials", Namespace: "openshift-image-registry", - RoleARN: rosaControlPlane.Spec.RolesRef.ImageRegistryARN, + RoleARN: rolesRef.ImageRegistryARN, }, { Name: "ebs-cloud-credentials", Namespace: "openshift-cluster-csi-drivers", - RoleARN: rosaControlPlane.Spec.RolesRef.StorageARN, + RoleARN: rolesRef.StorageARN, }, { Name: "cloud-credentials", Namespace: "openshift-cloud-network-config-controller", - RoleARN: rosaControlPlane.Spec.RolesRef.NetworkARN, + RoleARN: rolesRef.NetworkARN, }, { Name: "kube-controller-manager", Namespace: "kube-system", - RoleARN: rosaControlPlane.Spec.RolesRef.KubeCloudControllerARN, + RoleARN: rolesRef.KubeCloudControllerARN, }, { Name: "kms-provider", Namespace: "kube-system", - RoleARN: rosaControlPlane.Spec.RolesRef.KMSProviderARN, + RoleARN: rolesRef.KMSProviderARN, }, { Name: "control-plane-operator", Namespace: "kube-system", - RoleARN: rosaControlPlane.Spec.RolesRef.ControlPlaneOperatorARN, + RoleARN: rolesRef.ControlPlaneOperatorARN, }, { Name: "capa-controller-manager", Namespace: "kube-system", - RoleARN: rosaControlPlane.Spec.RolesRef.NodePoolManagementARN, + RoleARN: rolesRef.NodePoolManagementARN, }, } } @@ -406,7 +413,7 @@ func (r *ROSAControlPlaneReconciler) reconcileDelete(ctx context.Context, rosaSc } cluster, err := ocmClient.GetCluster(rosaScope.ControlPlane.Spec.RosaClusterName, creator) - if weberr.GetType(err) != weberr.NotFound && err != nil { + if err != nil && weberr.GetType(err) != weberr.NotFound { return ctrl.Result{}, err } if cluster == nil { @@ -580,7 +587,7 @@ func (r *ROSAControlPlaneReconciler) reconcileClusterAdminPassword(ctx context.C func validateControlPlaneSpec(ocmClient *ocm.Client, rosaScope *scope.ROSAControlPlaneScope) (string, error) { version := rosaScope.ControlPlane.Spec.Version - valid, err := ocmClient.ValidateHypershiftVersion(version, "stable") + valid, err := ocmClient.ValidateHypershiftVersion(version, ocm.DefaultChannelGroup) if err != nil { return "", fmt.Errorf("failed to check if version is valid: %w", err) } diff --git a/exp/api/v1beta2/rosamachinepool_types.go b/exp/api/v1beta2/rosamachinepool_types.go index 9ac13ac7ce..8335eaacec 100644 --- a/exp/api/v1beta2/rosamachinepool_types.go +++ b/exp/api/v1beta2/rosamachinepool_types.go @@ -17,6 +17,7 @@ limitations under the License. package v1beta2 import ( + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -33,11 +34,10 @@ type RosaMachinePoolSpec struct { // +kubebuilder:validation:Pattern:=`^[a-z]([-a-z0-9]*[a-z0-9])?$` NodePoolName string `json:"nodePoolName"` - // Version specifies the penshift version of the nodes associated with this machinepool. + // Version specifies the OpenShift version of the nodes associated with this machinepool. // ROSAControlPlane version is used if not set. // // +optional - // +kubebuilder:validation:XValidation:rule=`self.matches('^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)$')`, message="version must be a valid semantic version" Version string `json:"version,omitempty"` // AvailabilityZone is an optinal field specifying the availability zone where instances of this machine pool should run @@ -45,6 +45,8 @@ type RosaMachinePoolSpec struct { // +optional AvailabilityZone string `json:"availabilityZone,omitempty"` + // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="subnet is immutable" + // +immutable // +optional Subnet string `json:"subnet,omitempty"` @@ -52,30 +54,54 @@ type RosaMachinePoolSpec struct { // +optional Labels map[string]string `json:"labels,omitempty"` + // Taints specifies the taints to apply to the nodes of the machine pool + // +optional + Taints []RosaTaint `json:"taints,omitempty"` + // AutoRepair specifies whether health checks should be enabled for machines // in the NodePool. The default is false. - // +optional // +kubebuilder:default=false + // +optional AutoRepair bool `json:"autoRepair,omitempty"` // InstanceType specifies the AWS instance type - InstanceType string `json:"instanceType,omitempty"` + // + // +kubebuilder:validation:Required + InstanceType string `json:"instanceType"` // Autoscaling specifies auto scaling behaviour for this MachinePool. // required if Replicas is not configured // +optional Autoscaling *RosaMachinePoolAutoScaling `json:"autoscaling,omitempty"` - // TODO(alberto): Enable and propagate this API input. - // Taints []*Taint `json:"taints,omitempty"` - // TuningConfigs []string `json:"tuningConfigs,omitempty"` - // Version *Version `json:"version,omitempty"` + // TuningConfigs specifies the names of the tuning configs to be applied to this MachinePool. + // Tuning configs must already exist. + // +optional + TuningConfigs []string `json:"tuningConfigs,omitempty"` // ProviderIDList contain a ProviderID for each machine instance that's currently managed by this machine pool. // +optional ProviderIDList []string `json:"providerIDList,omitempty"` } +type RosaTaint struct { + // The taint key to be applied to a node. + // + // +kubebuilder:validation:Required + Key string `json:"key"` + // The taint value corresponding to the taint key. + // + // +kubebuilder:validation:Pattern:=`^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$` + // +optional + Value string `json:"value,omitempty"` + // The effect of the taint on pods that do not tolerate the taint. + // Valid effects are NoSchedule, PreferNoSchedule and NoExecute. + // + // +kubebuilder:validation:Required + // +kubebuilder:validation:Enum=NoSchedule;PreferNoSchedule;NoExecute + Effect corev1.TaintEffect `json:"effect"` +} + // RosaMachinePoolAutoScaling specifies scaling options. type RosaMachinePoolAutoScaling struct { // +kubebuilder:validation:Minimum=1 diff --git a/exp/api/v1beta2/rosamachinepool_webhook.go b/exp/api/v1beta2/rosamachinepool_webhook.go new file mode 100644 index 0000000000..b0660c031f --- /dev/null +++ b/exp/api/v1beta2/rosamachinepool_webhook.go @@ -0,0 +1,83 @@ +package v1beta2 + +import ( + "github.com/blang/semver" + apierrors "k8s.io/apimachinery/pkg/api/errors" + runtime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// SetupWebhookWithManager will setup the webhooks for the ROSAMachinePool. +func (r *ROSAMachinePool) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(r). + Complete() +} + +// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta2-rosamachinepool,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=rosamachinepools,versions=v1beta2,name=validation.rosamachinepool.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 +// +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1beta2-rosamachinepool,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=rosamachinepools,versions=v1beta2,name=default.rosamachinepool.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 + +var _ webhook.Defaulter = &ROSAMachinePool{} +var _ webhook.Validator = &ROSAMachinePool{} + +// ValidateCreate implements admission.Validator. +func (r *ROSAMachinePool) ValidateCreate() (warnings admission.Warnings, err error) { + var allErrs field.ErrorList + + if err := r.validateVersion(); err != nil { + allErrs = append(allErrs, err) + } + + if len(allErrs) == 0 { + return nil, nil + } + + return nil, apierrors.NewInvalid( + r.GroupVersionKind().GroupKind(), + r.Name, + allErrs, + ) +} + +// ValidateUpdate implements admission.Validator. +func (r *ROSAMachinePool) ValidateUpdate(old runtime.Object) (warnings admission.Warnings, err error) { + var allErrs field.ErrorList + + if err := r.validateVersion(); err != nil { + allErrs = append(allErrs, err) + } + + if len(allErrs) == 0 { + return nil, nil + } + + return nil, apierrors.NewInvalid( + r.GroupVersionKind().GroupKind(), + r.Name, + allErrs, + ) +} + +// ValidateDelete implements admission.Validator. +func (r *ROSAMachinePool) ValidateDelete() (warnings admission.Warnings, err error) { + return nil, nil +} + +func (r *ROSAMachinePool) validateVersion() *field.Error { + if r.Spec.Version == "" { + return nil + } + _, err := semver.Parse(r.Spec.Version) + if err != nil { + return field.Invalid(field.NewPath("spec.version"), r.Spec.Version, "must be a valid semantic version") + } + + return nil +} + +// Default implements admission.Defaulter. +func (r *ROSAMachinePool) Default() { +} diff --git a/exp/api/v1beta2/zz_generated.deepcopy.go b/exp/api/v1beta2/zz_generated.deepcopy.go index 867324e441..9a4a96b7ff 100644 --- a/exp/api/v1beta2/zz_generated.deepcopy.go +++ b/exp/api/v1beta2/zz_generated.deepcopy.go @@ -1090,11 +1090,21 @@ func (in *RosaMachinePoolSpec) DeepCopyInto(out *RosaMachinePoolSpec) { (*out)[key] = val } } + if in.Taints != nil { + in, out := &in.Taints, &out.Taints + *out = make([]RosaTaint, len(*in)) + copy(*out, *in) + } if in.Autoscaling != nil { in, out := &in.Autoscaling, &out.Autoscaling *out = new(RosaMachinePoolAutoScaling) **out = **in } + if in.TuningConfigs != nil { + in, out := &in.TuningConfigs, &out.TuningConfigs + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.ProviderIDList != nil { in, out := &in.ProviderIDList, &out.ProviderIDList *out = make([]string, len(*in)) @@ -1139,6 +1149,21 @@ func (in *RosaMachinePoolStatus) DeepCopy() *RosaMachinePoolStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RosaTaint) DeepCopyInto(out *RosaTaint) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RosaTaint. +func (in *RosaTaint) DeepCopy() *RosaTaint { + if in == nil { + return nil + } + out := new(RosaTaint) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SuspendProcessesTypes) DeepCopyInto(out *SuspendProcessesTypes) { *out = *in diff --git a/exp/controllers/rosamachinepool_controller.go b/exp/controllers/rosamachinepool_controller.go index 6e837b257a..b05a0fccf5 100644 --- a/exp/controllers/rosamachinepool_controller.go +++ b/exp/controllers/rosamachinepool_controller.go @@ -6,6 +6,7 @@ import ( "time" "github.com/blang/semver" + "github.com/google/go-cmp/cmp" cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" "github.com/openshift/rosa/pkg/ocm" "github.com/pkg/errors" @@ -35,7 +36,7 @@ import ( "sigs.k8s.io/cluster-api/util/predicates" ) -// ROSAMachinePoolReconciler reconciles a RosaMachinePool object. +// ROSAMachinePoolReconciler reconciles a ROSAMachinePool object. type ROSAMachinePoolReconciler struct { client.Client Recorder record.EventRecorder @@ -49,7 +50,7 @@ func (r *ROSAMachinePoolReconciler) SetupWithManager(ctx context.Context, mgr ct gvk, err := apiutil.GVKForObject(new(expinfrav1.ROSAMachinePool), mgr.GetScheme()) if err != nil { - return errors.Wrapf(err, "failed to find GVK for RosaMachinePool") + return errors.Wrapf(err, "failed to find GVK for ROSAMachinePool") } rosaControlPlaneToRosaMachinePoolMap := rosaControlPlaneToRosaMachinePoolMapFunc(r.Client, gvk, log) return ctrl.NewControllerManagedBy(mgr). @@ -72,8 +73,9 @@ func (r *ROSAMachinePoolReconciler) SetupWithManager(ctx context.Context, mgr ct // +kubebuilder:rbac:groups=controlplane.cluster.x-k8s.io,resources=rosacontrolplanes;rosacontrolplanes/status,verbs=get;list;watch // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=rosamachinepools,verbs=get;list;watch;update;patch;delete // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=rosamachinepools/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=rosamachinepools/finalizers,verbs=update -// Reconcile reconciles RosaMachinePool. +// Reconcile reconciles ROSAMachinePool. func (r *ROSAMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) { log := logger.FromContext(ctx) @@ -138,6 +140,7 @@ func (r *ROSAMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Requ Cluster: cluster, ControlPlane: controlPlane, ControllerName: "rosaControlPlane", + Endpoints: r.Endpoints, }) if err != nil { return ctrl.Result{}, errors.Wrap(err, "failed to create control plane scope") @@ -168,7 +171,7 @@ func (r *ROSAMachinePoolReconciler) reconcileNormal(ctx context.Context, machinePoolScope *scope.RosaMachinePoolScope, rosaControlPlaneScope *scope.ROSAControlPlaneScope, ) (ctrl.Result, error) { - machinePoolScope.Info("Reconciling RosaMachinePool") + machinePoolScope.Info("Reconciling ROSAMachinePool") if controllerutil.AddFinalizer(machinePoolScope.RosaMachinePool, expinfrav1.RosaMachinePoolFinalizer) { if err := machinePoolScope.PatchObject(); err != nil { @@ -195,21 +198,25 @@ func (r *ROSAMachinePoolReconciler) reconcileNormal(ctx context.Context, } rosaMachinePool := machinePoolScope.RosaMachinePool - machinePool := machinePoolScope.MachinePool - controlPlane := machinePoolScope.ControlPlane - createdNodePool, found, err := ocmClient.GetNodePool(*controlPlane.Status.ID, rosaMachinePool.Spec.NodePoolName) + nodePool, found, err := ocmClient.GetNodePool(machinePoolScope.ControlPlane.Status.ID, rosaMachinePool.Spec.NodePoolName) if err != nil { return ctrl.Result{}, err } + if found { + nodePool, err := r.updateNodePool(machinePoolScope, ocmClient, nodePool) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to ensure rosaMachinePool: %w", err) + } + // TODO (alberto): discover and store providerIDs from aws so the CAPI controller can match then to Nodes and report readiness. - rosaMachinePool.Status.Replicas = int32(createdNodePool.Status().CurrentReplicas()) - if createdNodePool.Replicas() == createdNodePool.Status().CurrentReplicas() && createdNodePool.Status().Message() == "" { + rosaMachinePool.Status.Replicas = int32(nodePool.Status().CurrentReplicas()) + if nodePool.Replicas() == nodePool.Status().CurrentReplicas() && nodePool.Status().Message() == "" { conditions.MarkTrue(rosaMachinePool, expinfrav1.RosaMachinePoolReadyCondition) rosaMachinePool.Status.Ready = true - if err := r.reconcileMachinePoolVersion(machinePoolScope, ocmClient, createdNodePool); err != nil { + if err := r.reconcileMachinePoolVersion(machinePoolScope, ocmClient, nodePool); err != nil { return ctrl.Result{}, err } @@ -218,54 +225,27 @@ func (r *ROSAMachinePoolReconciler) reconcileNormal(ctx context.Context, conditions.MarkFalse(rosaMachinePool, expinfrav1.RosaMachinePoolReadyCondition, - createdNodePool.Status().Message(), + nodePool.Status().Message(), clusterv1.ConditionSeverityInfo, "") - machinePoolScope.Info("waiting for NodePool to become ready", "state", createdNodePool.Status().Message()) + machinePoolScope.Info("waiting for NodePool to become ready", "state", nodePool.Status().Message()) // Requeue so that status.ready is set to true when the nodepool is fully created. return ctrl.Result{RequeueAfter: time.Second * 60}, nil } - npBuilder := cmv1.NewNodePool() - npBuilder.ID(rosaMachinePool.Spec.NodePoolName). - Labels(rosaMachinePool.Spec.Labels). - AutoRepair(rosaMachinePool.Spec.AutoRepair) - - if rosaMachinePool.Spec.Autoscaling != nil { - npBuilder = npBuilder.Autoscaling( - cmv1.NewNodePoolAutoscaling(). - MinReplica(rosaMachinePool.Spec.Autoscaling.MinReplicas). - MaxReplica(rosaMachinePool.Spec.Autoscaling.MaxReplicas)) - } else { - replicas := 1 - if machinePool.Spec.Replicas != nil { - replicas = int(*machinePool.Spec.Replicas) - } - npBuilder = npBuilder.Replicas(replicas) - } - - if rosaMachinePool.Spec.Subnet != "" { - npBuilder.Subnet(rosaMachinePool.Spec.Subnet) - } - - npBuilder.AWSNodePool(cmv1.NewAWSNodePool().InstanceType(rosaMachinePool.Spec.InstanceType)) - if rosaMachinePool.Spec.Version != "" { - npBuilder.Version(cmv1.NewVersion().ID(ocm.CreateVersionID(rosaMachinePool.Spec.Version, ocm.DefaultChannelGroup))) - } - + npBuilder := nodePoolBuilder(rosaMachinePool.Spec, machinePoolScope.MachinePool.Spec) nodePoolSpec, err := npBuilder.Build() if err != nil { return ctrl.Result{}, fmt.Errorf("failed to build rosa nodepool: %w", err) } - createdNodePool, err = ocmClient.CreateNodePool(*controlPlane.Status.ID, nodePoolSpec) + nodePool, err = ocmClient.CreateNodePool(machinePoolScope.ControlPlane.Status.ID, nodePoolSpec) if err != nil { return ctrl.Result{}, fmt.Errorf("failed to create nodepool: %w", err) } - machinePoolScope.RosaMachinePool.Status.ID = createdNodePool.ID() - + machinePoolScope.RosaMachinePool.Status.ID = nodePool.ID() return ctrl.Result{}, nil } @@ -281,12 +261,12 @@ func (r *ROSAMachinePoolReconciler) reconcileDelete( return fmt.Errorf("failed to create OCM client: %w", err) } - nodePool, found, err := ocmClient.GetNodePool(*machinePoolScope.ControlPlane.Status.ID, machinePoolScope.NodePoolName()) + nodePool, found, err := ocmClient.GetNodePool(machinePoolScope.ControlPlane.Status.ID, machinePoolScope.NodePoolName()) if err != nil { return err } if found { - if err := ocmClient.DeleteNodePool(*machinePoolScope.ControlPlane.Status.ID, nodePool.ID()); err != nil { + if err := ocmClient.DeleteNodePool(machinePoolScope.ControlPlane.Status.ID, nodePool.ID()); err != nil { return err } } @@ -307,7 +287,7 @@ func (r *ROSAMachinePoolReconciler) reconcileMachinePoolVersion(machinePoolScope return nil } - clusterID := *machinePoolScope.ControlPlane.Status.ID + clusterID := machinePoolScope.ControlPlane.Status.ID _, scheduledUpgrade, err := ocmClient.GetHypershiftNodePoolUpgrade(clusterID, machinePoolScope.ControlPlane.Spec.RosaClusterName, nodePool.ID()) if err != nil { return fmt.Errorf("failed to get existing scheduled upgrades: %w", err) @@ -349,6 +329,34 @@ func (r *ROSAMachinePoolReconciler) reconcileMachinePoolVersion(machinePoolScope return nil } +func (r *ROSAMachinePoolReconciler) updateNodePool(machinePoolScope *scope.RosaMachinePoolScope, ocmClient *ocm.Client, nodePool *cmv1.NodePool) (*cmv1.NodePool, error) { + desiredSpec := machinePoolScope.RosaMachinePool.Spec.DeepCopy() + + currentSpec := nodePoolToRosaMachinePoolSpec(nodePool) + currentSpec.ProviderIDList = desiredSpec.ProviderIDList // providerIDList is set by the controller and shouldn't be compared here. + currentSpec.Version = desiredSpec.Version // Version changed are reconciled separately and shouldn't be compared here. + + if cmp.Equal(desiredSpec, currentSpec) { + // no changes detected. + return nodePool, nil + } + + npBuilder := nodePoolBuilder(*desiredSpec, machinePoolScope.MachinePool.Spec) + npBuilder.Version(nil) // eunsure version is unset. + + nodePoolSpec, err := npBuilder.Build() + if err != nil { + return nil, fmt.Errorf("failed to build nodePool spec: %w", err) + } + + updatedNodePool, err := ocmClient.UpdateNodePool(machinePoolScope.ControlPlane.Status.ID, nodePoolSpec) + if err != nil { + return nil, fmt.Errorf("failed to update nodePool: %w", err) + } + + return updatedNodePool, nil +} + func validateMachinePoolSpec(machinePoolScope *scope.RosaMachinePoolScope) (*string, error) { if machinePoolScope.RosaMachinePool.Spec.Version == "" { return nil, nil @@ -372,6 +380,79 @@ func validateMachinePoolSpec(machinePoolScope *scope.RosaMachinePoolScope) (*str return nil, nil } +func nodePoolBuilder(rosaMachinePoolSpec expinfrav1.RosaMachinePoolSpec, machinePoolSpec expclusterv1.MachinePoolSpec) *cmv1.NodePoolBuilder { + npBuilder := cmv1.NewNodePool().ID(rosaMachinePoolSpec.NodePoolName). + Labels(rosaMachinePoolSpec.Labels). + AutoRepair(rosaMachinePoolSpec.AutoRepair). + TuningConfigs(rosaMachinePoolSpec.TuningConfigs...) + + if len(rosaMachinePoolSpec.Taints) > 0 { + taintBuilders := []*cmv1.TaintBuilder{} + for _, taint := range rosaMachinePoolSpec.Taints { + newTaintBuilder := cmv1.NewTaint().Key(taint.Key).Value(taint.Value).Effect(string(taint.Effect)) + taintBuilders = append(taintBuilders, newTaintBuilder) + } + npBuilder = npBuilder.Taints(taintBuilders...) + } + + if rosaMachinePoolSpec.Autoscaling != nil { + npBuilder = npBuilder.Autoscaling( + cmv1.NewNodePoolAutoscaling(). + MinReplica(rosaMachinePoolSpec.Autoscaling.MinReplicas). + MaxReplica(rosaMachinePoolSpec.Autoscaling.MaxReplicas)) + } else { + replicas := 1 + if machinePoolSpec.Replicas != nil { + replicas = int(*machinePoolSpec.Replicas) + } + npBuilder = npBuilder.Replicas(replicas) + } + + if rosaMachinePoolSpec.Subnet != "" { + npBuilder.Subnet(rosaMachinePoolSpec.Subnet) + } + + npBuilder.AWSNodePool(cmv1.NewAWSNodePool().InstanceType(rosaMachinePoolSpec.InstanceType)) + if rosaMachinePoolSpec.Version != "" { + npBuilder.Version(cmv1.NewVersion().ID(ocm.CreateVersionID(rosaMachinePoolSpec.Version, ocm.DefaultChannelGroup))) + } + + return npBuilder +} + +func nodePoolToRosaMachinePoolSpec(nodePool *cmv1.NodePool) expinfrav1.RosaMachinePoolSpec { + spec := expinfrav1.RosaMachinePoolSpec{ + NodePoolName: nodePool.ID(), + Version: rosa.RawVersionID(nodePool.Version()), + AvailabilityZone: nodePool.AvailabilityZone(), + Subnet: nodePool.Subnet(), + Labels: nodePool.Labels(), + AutoRepair: nodePool.AutoRepair(), + InstanceType: nodePool.AWSNodePool().InstanceType(), + TuningConfigs: nodePool.TuningConfigs(), + } + + if nodePool.Autoscaling() != nil { + spec.Autoscaling = &expinfrav1.RosaMachinePoolAutoScaling{ + MinReplicas: nodePool.Autoscaling().MinReplica(), + MaxReplicas: nodePool.Autoscaling().MaxReplica(), + } + } + if nodePool.Taints() != nil { + rosaTaints := make([]expinfrav1.RosaTaint, len(nodePool.Taints())) + for _, taint := range nodePool.Taints() { + rosaTaints = append(rosaTaints, expinfrav1.RosaTaint{ + Key: taint.Key(), + Value: taint.Value(), + Effect: corev1.TaintEffect(taint.Effect()), + }) + } + spec.Taints = rosaTaints + } + + return spec +} + func rosaControlPlaneToRosaMachinePoolMapFunc(c client.Client, gvk schema.GroupVersionKind, log logger.Wrapper) handler.MapFunc { return func(ctx context.Context, o client.Object) []reconcile.Request { rosaControlPlane, ok := o.(*rosacontrolplanev1.ROSAControlPlane) diff --git a/exp/controllers/rosamachinepool_controller_test.go b/exp/controllers/rosamachinepool_controller_test.go new file mode 100644 index 0000000000..5df3df6f4b --- /dev/null +++ b/exp/controllers/rosamachinepool_controller_test.go @@ -0,0 +1,37 @@ +package controllers + +import ( + "testing" + + . "github.com/onsi/gomega" + "k8s.io/utils/ptr" + + expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2" + expclusterv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" +) + +func TestNodePoolToRosaMachinePoolSpec(t *testing.T) { + g := NewWithT(t) + + rosaMachinePoolSpec := expinfrav1.RosaMachinePoolSpec{ + NodePoolName: "test-nodepool", + Version: "4.14.5", + Subnet: "subnet-id", + AutoRepair: true, + InstanceType: "m5.large", + TuningConfigs: []string{"config1"}, + } + + machinePoolSpec := expclusterv1.MachinePoolSpec{ + Replicas: ptr.To[int32](2), + } + + nodePoolBuilder := nodePoolBuilder(rosaMachinePoolSpec, machinePoolSpec) + + nodePoolSpec, err := nodePoolBuilder.Build() + g.Expect(err).ToNot(HaveOccurred()) + + expectedSpec := nodePoolToRosaMachinePoolSpec(nodePoolSpec) + + g.Expect(expectedSpec).To(Equal(rosaMachinePoolSpec)) +} diff --git a/main.go b/main.go index 32954e6dfd..8f38b3f49f 100644 --- a/main.go +++ b/main.go @@ -254,6 +254,16 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "ROSAMachinePool") os.Exit(1) } + + if err := (&rosacontrolplanev1.ROSAControlPlane{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "ROSAControlPlane") + os.Exit(1) + } + + if err := (&expinfrav1.ROSAMachinePool{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "ROSAMachinePool") + os.Exit(1) + } } // +kubebuilder:scaffold:builder diff --git a/pkg/cloud/scope/rosacontrolplane.go b/pkg/cloud/scope/rosacontrolplane.go index da4c36cb13..81b0d1843c 100644 --- a/pkg/cloud/scope/rosacontrolplane.go +++ b/pkg/cloud/scope/rosacontrolplane.go @@ -67,7 +67,7 @@ func NewROSAControlPlaneScope(params ROSAControlPlaneScopeParams) (*ROSAControlP controllerName: params.ControllerName, } - session, serviceLimiters, err := sessionForClusterWithRegion(params.Client, managedScope, *params.ControlPlane.Spec.Region, params.Endpoints, params.Logger) + session, serviceLimiters, err := sessionForClusterWithRegion(params.Client, managedScope, params.ControlPlane.Spec.Region, params.Endpoints, params.Logger) if err != nil { return nil, errors.Errorf("failed to create aws session: %v", err) } @@ -183,6 +183,8 @@ func (s *ROSAControlPlaneScope) PatchObject() error { s.ControlPlane, patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{ rosacontrolplanev1.ROSAControlPlaneReadyCondition, + rosacontrolplanev1.ROSAControlPlaneValidCondition, + rosacontrolplanev1.ROSAControlPlaneUpgradingCondition, }}) }