From 3e14ffd30582e2752ad61dba29d494008d075cae Mon Sep 17 00:00:00 2001 From: Ivo Petrov Date: Fri, 23 Aug 2024 16:32:49 +0300 Subject: [PATCH] Drain refactor to handle complex cluster types (#57) * Drain refactor to handle complex cluster types * Bring back old func name * Rework drain configuration to support only disables * Rework user configured drain option implementation * Use normal value for DisableDrain --- api/v1alpha1/upgradeplan_types.go | 11 +++-- api/v1alpha1/zz_generated.deepcopy.go | 26 +++--------- .../lifecycle.suse.com_upgradeplans.yaml | 7 ++-- internal/controller/reconcile_kubernetes.go | 2 +- internal/controller/reconcile_os.go | 2 +- internal/controller/upgradeplan_controller.go | 41 ++++++++++++++----- 6 files changed, 47 insertions(+), 42 deletions(-) diff --git a/api/v1alpha1/upgradeplan_types.go b/api/v1alpha1/upgradeplan_types.go index d4a0d58..9e9ef91 100644 --- a/api/v1alpha1/upgradeplan_types.go +++ b/api/v1alpha1/upgradeplan_types.go @@ -50,17 +50,16 @@ type UpgradePlanSpec struct { // ReleaseVersion specifies the target version for platform upgrade. // The version format is X.Y.Z, for example "3.0.2". ReleaseVersion string `json:"releaseVersion"` - // Drain specifies whether control-plane and worker nodes should be drained. - // If left unspecified, drain is done on both control-plane and worker nodes by default. + // DisableDrain specifies whether control-plane and worker nodes drain should be disabled. // +optional - Drain *Drain `json:"drain"` + DisableDrain DisableDrain `json:"disableDrain"` } -type Drain struct { +type DisableDrain struct { // +optional - ControlPlane *bool `json:"controlPlane"` + ControlPlane bool `json:"controlPlane"` // +optional - Worker *bool `json:"worker"` + Worker bool `json:"worker"` } // UpgradePlanStatus defines the observed state of UpgradePlan diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 9a39eaf..b263eb8 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -44,26 +44,16 @@ func (in *Components) DeepCopy() *Components { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Drain) DeepCopyInto(out *Drain) { +func (in *DisableDrain) DeepCopyInto(out *DisableDrain) { *out = *in - if in.ControlPlane != nil { - in, out := &in.ControlPlane, &out.ControlPlane - *out = new(bool) - **out = **in - } - if in.Worker != nil { - in, out := &in.Worker, &out.Worker - *out = new(bool) - **out = **in - } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Drain. -func (in *Drain) DeepCopy() *Drain { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DisableDrain. +func (in *DisableDrain) DeepCopy() *DisableDrain { if in == nil { return nil } - out := new(Drain) + out := new(DisableDrain) in.DeepCopyInto(out) return out } @@ -244,7 +234,7 @@ func (in *UpgradePlan) DeepCopyInto(out *UpgradePlan) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - in.Spec.DeepCopyInto(&out.Spec) + out.Spec = in.Spec in.Status.DeepCopyInto(&out.Status) } @@ -301,11 +291,7 @@ func (in *UpgradePlanList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *UpgradePlanSpec) DeepCopyInto(out *UpgradePlanSpec) { *out = *in - if in.Drain != nil { - in, out := &in.Drain, &out.Drain - *out = new(Drain) - (*in).DeepCopyInto(*out) - } + out.DisableDrain = in.DisableDrain } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UpgradePlanSpec. diff --git a/config/crd/bases/lifecycle.suse.com_upgradeplans.yaml b/config/crd/bases/lifecycle.suse.com_upgradeplans.yaml index e52add3..fbd15c2 100644 --- a/config/crd/bases/lifecycle.suse.com_upgradeplans.yaml +++ b/config/crd/bases/lifecycle.suse.com_upgradeplans.yaml @@ -39,10 +39,9 @@ spec: spec: description: UpgradePlanSpec defines the desired state of UpgradePlan properties: - drain: - description: |- - Drain specifies whether control-plane and worker nodes should be drained. - If left unspecified, drain is done on both control-plane and worker nodes by default. + disableDrain: + description: DisableDrain specifies whether control-plane and worker + nodes drain should be disabled. properties: controlPlane: type: boolean diff --git a/internal/controller/reconcile_kubernetes.go b/internal/controller/reconcile_kubernetes.go index af93d42..2861d13 100644 --- a/internal/controller/reconcile_kubernetes.go +++ b/internal/controller/reconcile_kubernetes.go @@ -28,7 +28,7 @@ func (r *UpgradePlanReconciler) reconcileKubernetes(ctx context.Context, upgrade } identifierAnnotations := upgrade.PlanIdentifierAnnotations(upgradePlan.Name, upgradePlan.Namespace) - drainControlPlane, drainWorker := parseDrainOptions(upgradePlan) + drainControlPlane, drainWorker := parseDrainOptions(nodeList, upgradePlan) controlPlanePlan := upgrade.KubernetesControlPlanePlan(kubernetesVersion, drainControlPlane, identifierAnnotations) if err = r.Get(ctx, client.ObjectKeyFromObject(controlPlanePlan), controlPlanePlan); err != nil { if !errors.IsNotFound(err) { diff --git a/internal/controller/reconcile_os.go b/internal/controller/reconcile_os.go index c323d2f..b762a4f 100644 --- a/internal/controller/reconcile_os.go +++ b/internal/controller/reconcile_os.go @@ -41,7 +41,7 @@ func (r *UpgradePlanReconciler) reconcileOS(ctx context.Context, upgradePlan *li return ctrl.Result{}, r.createObject(ctx, upgradePlan, secret) } - drainControlPlane, drainWorker := parseDrainOptions(upgradePlan) + drainControlPlane, drainWorker := parseDrainOptions(nodeList, upgradePlan) controlPlanePlan := upgrade.OSControlPlanePlan(releaseVersion, secret.Name, releaseOS, drainControlPlane, identifierAnnotations) if err = r.Get(ctx, client.ObjectKeyFromObject(controlPlanePlan), controlPlanePlan); err != nil { if !errors.IsNotFound(err) { diff --git a/internal/controller/upgradeplan_controller.go b/internal/controller/upgradeplan_controller.go index 5e2c0bf..ef32178 100644 --- a/internal/controller/upgradeplan_controller.go +++ b/internal/controller/upgradeplan_controller.go @@ -157,18 +157,39 @@ func isHelmUpgradeFinished(plan *lifecyclev1alpha1.UpgradePlan, conditionType st return false } -func parseDrainOptions(plan *lifecyclev1alpha1.UpgradePlan) (drainControlPlane bool, drainWorker bool) { - drainControlPlane = true - drainWorker = true - - if plan.Spec.Drain != nil { - if plan.Spec.Drain.ControlPlane != nil { - drainControlPlane = *plan.Spec.Drain.ControlPlane +func parseDrainOptions(nodeList *corev1.NodeList, plan *lifecyclev1alpha1.UpgradePlan) (drainControlPlane bool, drainWorker bool) { + var controlPlaneCounter, workerCounter int + for _, node := range nodeList.Items { + if node.Labels[upgrade.ControlPlaneLabel] != "true" { + workerCounter++ + } else { + controlPlaneCounter++ } + } - if plan.Spec.Drain.Worker != nil { - drainWorker = *plan.Spec.Drain.Worker - } + switch { + case controlPlaneCounter > 1 && workerCounter <= 1: + drainControlPlane = true + drainWorker = false + case controlPlaneCounter == 1 && workerCounter > 1: + drainControlPlane = false + drainWorker = true + case controlPlaneCounter <= 1 && workerCounter <= 1: + drainControlPlane = false + drainWorker = false + default: + drainControlPlane = true + drainWorker = true + } + + // If user has explicitly disabled control-plane drains + if plan.Spec.DisableDrain.ControlPlane { + drainControlPlane = false + } + + // If user has explicitly disabled worker drains + if plan.Spec.DisableDrain.Worker { + drainWorker = false } return drainControlPlane, drainWorker