From b5abd8cf46fae777a6ab4d08cd8ef3829cf724f7 Mon Sep 17 00:00:00 2001 From: Janis Kemper Date: Tue, 28 May 2024 10:00:00 +0100 Subject: [PATCH] :seedling: Refactor HelmChartStatus Refactor HelmChartStatus so that it is more user-friendly and always shows the appropriate phase we are in. Signed-off-by: janiskemper --- api/v1alpha1/clusteraddon_types.go | 56 ++++++++++++++++--- api/v1alpha1/zz_generated.deepcopy.go | 25 +++++++-- .../clusterstack.x-k8s.io_clusteraddons.yaml | 26 ++++++--- .../controller/clusteraddon_controller.go | 52 ++++++++++------- .../2/docker/ferrol/1-27/clusteraddon.yaml | 2 + .../2/docker/ferrol/1-27/csmctl.yaml | 2 +- 6 files changed, 118 insertions(+), 45 deletions(-) diff --git a/api/v1alpha1/clusteraddon_types.go b/api/v1alpha1/clusteraddon_types.go index db20ba2f7..2b09c5737 100644 --- a/api/v1alpha1/clusteraddon_types.go +++ b/api/v1alpha1/clusteraddon_types.go @@ -17,6 +17,7 @@ limitations under the License. package v1alpha1 import ( + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/clusteraddon" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -27,17 +28,33 @@ const ( ClusterAddonFinalizer = "clusteraddon.clusterstack.x-k8s.io" ) -// HelmChartStatusConditions defines the status of helm chart in the cluster addon. -type HelmChartStatusConditions string +// StagePhase defines the status of helm chart in the cluster addon. +type StagePhase string var ( - None = HelmChartStatusConditions("") - WaitingForPreCondition = HelmChartStatusConditions("waitingForPreCondition") - ApplyingOrDeleting = HelmChartStatusConditions("applyingOrDeleting") - WaitingForPostCondition = HelmChartStatusConditions("waitingForPostCondition") - Done = HelmChartStatusConditions("done") + None = StagePhase("") + Pending = StagePhase("Pending") + WaitingForPreCondition = StagePhase("waitingForPreCondition") + ApplyingOrDeleting = StagePhase("applyingOrDeleting") + WaitingForPostCondition = StagePhase("waitingForPostCondition") + Done = StagePhase("done") ) +// StageStatus represents the helm charts of the hook and it's phases. +type StageStatus struct { + // Name represent name of the helm chart + // +optional + Name string `json:"helmChartName"` + + // Action is the action of the helm chart. e.g. - apply and delete. + // +optional + Action clusteraddon.Action `json:"action,omitempty"` + + // Phase is the current phase of the helm chart. + // +optional + Phase StagePhase `json:"phase"` +} + // ClusterAddonSpec defines the desired state of a ClusterAddon object. type ClusterAddonSpec struct { // ClusterStack is the full string --- that will be filled with the cluster stack that @@ -64,9 +81,9 @@ type ClusterAddonStatus struct { // +optional Resources []*Resource `json:"resources,omitempty"` - // HelmChartStatus defines the status of helm chart in the cluster addon. + // Stages shows the state of all stages in the current running hook. // +optional - HelmChartStatus map[string]HelmChartStatusConditions `json:"helmChartStatus,omitempty"` + Stages []StageStatus `json:"stages,omitempty"` // +optional // +kubebuilder:default:=false @@ -95,6 +112,27 @@ type ClusterAddon struct { Status ClusterAddonStatus `json:"status,omitempty"` } +// GetStagePhase returns helm chart status for the helm chart. +func (r *ClusterAddon) GetStagePhase(helmChartName string, action clusteraddon.Action) StagePhase { + for _, stage := range r.Status.Stages { + if stage.Name == helmChartName && stage.Action == action { + return stage.Phase + } + } + + // This cannot occur as we populate phase value with "pending". + return None +} + +// SetStagePhase sets the helm chart status phase. +func (r *ClusterAddon) SetStagePhase(helmChartName string, action clusteraddon.Action, phase StagePhase) { + for i, _ := range r.Status.Stages { + if r.Status.Stages[i].Name == helmChartName && r.Status.Stages[i].Action == action { + r.Status.Stages[i].Phase = phase + } + } +} + // GetConditions returns the observations of the operational state of the ClusterAddon resource. func (r *ClusterAddon) GetConditions() clusterv1.Conditions { return r.Status.Conditions diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index d7b51c561..b1805d3c3 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -120,12 +120,10 @@ func (in *ClusterAddonStatus) DeepCopyInto(out *ClusterAddonStatus) { } } } - if in.HelmChartStatus != nil { - in, out := &in.HelmChartStatus, &out.HelmChartStatus - *out = make(map[string]HelmChartStatusConditions, len(*in)) - for key, val := range *in { - (*out)[key] = val - } + if in.Stages != nil { + in, out := &in.Stages, &out.Stages + *out = make([]StageStatus, len(*in)) + copy(*out, *in) } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions @@ -398,3 +396,18 @@ func (in *Resource) DeepCopy() *Resource { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *StageStatus) DeepCopyInto(out *StageStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StageStatus. +func (in *StageStatus) DeepCopy() *StageStatus { + if in == nil { + return nil + } + out := new(StageStatus) + in.DeepCopyInto(out) + return out +} diff --git a/config/crd/bases/clusterstack.x-k8s.io_clusteraddons.yaml b/config/crd/bases/clusterstack.x-k8s.io_clusteraddons.yaml index 6b6bec1ac..83e21bb24 100644 --- a/config/crd/bases/clusterstack.x-k8s.io_clusteraddons.yaml +++ b/config/crd/bases/clusterstack.x-k8s.io_clusteraddons.yaml @@ -159,14 +159,6 @@ spec: - type type: object type: array - helmChartStatus: - additionalProperties: - description: HelmChartStatusConditions defines the status of helm - chart in the cluster addon. - type: string - description: HelmChartStatus defines the status of helm chart in the - cluster addon. - type: object ready: default: false type: boolean @@ -201,6 +193,24 @@ spec: type: string type: object type: array + stages: + description: Stages shows the state of all stages in the current running + hook. + items: + description: StageStatus represents the helm charts of the hook + and it's phases. + properties: + action: + description: Action is the action of the helm chart. e.g. - + apply and delete. + helmChartName: + description: Name represent name of the helm chart + type: string + phase: + description: Phase is the current phase of the helm chart. + type: string + type: object + type: array type: object type: object served: true diff --git a/internal/controller/clusteraddon_controller.go b/internal/controller/clusteraddon_controller.go index faa96671c..65ef6fb1c 100644 --- a/internal/controller/clusteraddon_controller.go +++ b/internal/controller/clusteraddon_controller.go @@ -280,6 +280,12 @@ func (r *ClusterAddonReconciler) Reconcile(ctx context.Context, req reconcile.Re return reconcile.Result{}, fmt.Errorf("failed to get addon stages input: %w", err) } + // clusteraddon.yaml in the release. + clusterAddonConfig, err := clusteraddon.ParseClusterAddonConfig(in.clusterAddonConfigPath) + if err != nil { + return reconcile.Result{}, fmt.Errorf("failed to parse clusteraddon.yaml config: %w", err) + } + var ( oldRelease release.Release requeue bool @@ -307,11 +313,13 @@ func (r *ClusterAddonReconciler) Reconcile(ctx context.Context, req reconcile.Re // if a hook is set, it is expected that HelmChartAppliedCondition is removed if clusterAddon.Spec.Hook != "" { // if the clusterAddon was ready before, it means this hook is fresh and we have to reset the status - if clusterAddon.Status.Ready { - clusterAddon.Status.HelmChartStatus = make(map[string]csov1alpha1.HelmChartStatusConditions) - } - if clusterAddon.Status.HelmChartStatus == nil { - clusterAddon.Status.HelmChartStatus = make(map[string]csov1alpha1.HelmChartStatusConditions) + if clusterAddon.Status.Ready || len(clusterAddon.Status.Stages) == 0 { + clusterAddon.Status.Stages = make([]csov1alpha1.StageStatus, len(clusterAddonConfig.AddonStages[clusterAddon.Spec.Hook])) + for i, stage := range clusterAddonConfig.AddonStages[clusterAddon.Spec.Hook] { + clusterAddon.Status.Stages[i].Name = stage.HelmChartName + clusterAddon.Status.Stages[i].Action = stage.Action + clusterAddon.Status.Stages[i].Phase = csov1alpha1.Pending + } } clusterAddon.Status.Ready = false } @@ -320,7 +328,14 @@ func (r *ClusterAddonReconciler) Reconcile(ctx context.Context, req reconcile.Re // Therefore, we have to check whether the ClusterStack is upgraded and if that is the case, the ClusterAddons have to be upgraded as well. if clusterAddon.Spec.ClusterStack != cluster.Spec.Topology.Class && oldRelease.Meta.Versions.Kubernetes == releaseAsset.Meta.Versions.Kubernetes { if clusterAddon.Spec.Version != releaseAsset.Meta.Versions.Components.ClusterAddon { - clusterAddon.Status.HelmChartStatus = make(map[string]csov1alpha1.HelmChartStatusConditions) + if clusterAddon.Status.Ready || len(clusterAddon.Status.Stages) == 0 { + clusterAddon.Status.Stages = make([]csov1alpha1.StageStatus, len(clusterAddonConfig.AddonStages["BeforeClusterUpgrade"])) + for i, stage := range clusterAddonConfig.AddonStages["BeforeClusterUpgrade"] { + clusterAddon.Status.Stages[i].Name = stage.HelmChartName + clusterAddon.Status.Stages[i].Action = stage.Action + clusterAddon.Status.Stages[i].Phase = csov1alpha1.Pending + } + } clusterAddon.Status.Ready = false conditions.Delete(clusterAddon, csov1alpha1.HelmChartAppliedCondition) } else { @@ -336,12 +351,6 @@ func (r *ClusterAddonReconciler) Reconcile(ctx context.Context, req reconcile.Re return reconcile.Result{}, nil } - // clusteraddon.yaml in the release. - clusterAddonConfig, err := clusteraddon.ParseClusterAddonConfig(in.clusterAddonConfigPath) - if err != nil { - return reconcile.Result{}, fmt.Errorf("failed to parse clusteraddon.yaml config: %w", err) - } - // In case the Kubernetes version stayed the same during an upgrade, the hook server does not trigger and // we just take the Helm charts that are supposed to be installed in the BeforeClusterUpgrade hook and apply them. if oldRelease.Meta.Versions.Kubernetes == releaseAsset.Meta.Versions.Kubernetes { @@ -363,7 +372,7 @@ func (r *ClusterAddonReconciler) Reconcile(ctx context.Context, req reconcile.Re clusterAddon.Status.Resources = make([]*csov1alpha1.Resource, 0) // remove the helm chart status from the status. - clusterAddon.Status.HelmChartStatus = make(map[string]csov1alpha1.HelmChartStatusConditions) + clusterAddon.Status.Stages = make([]csov1alpha1.StageStatus, 0) // update the latest cluster class clusterAddon.Spec.ClusterStack = cluster.Spec.Topology.Class @@ -397,7 +406,7 @@ func (r *ClusterAddonReconciler) Reconcile(ctx context.Context, req reconcile.Re conditions.MarkTrue(clusterAddon, csov1alpha1.HelmChartAppliedCondition) // remove the helm chart status from the status. - clusterAddon.Status.HelmChartStatus = make(map[string]csov1alpha1.HelmChartStatusConditions) + clusterAddon.Status.Stages = make([]csov1alpha1.StageStatus, 0) // remove the status resource if hook is finished clusterAddon.Status.Resources = make([]*csov1alpha1.Resource, 0) @@ -534,8 +543,8 @@ func (r *ClusterAddonReconciler) executeStage(ctx context.Context, stage cluster } check: - switch in.clusterAddon.Status.HelmChartStatus[stage.HelmChartName] { - case csov1alpha1.None, csov1alpha1.WaitingForPreCondition: + switch in.clusterAddon.GetStagePhase(stage.HelmChartName, stage.Action) { + case csov1alpha1.Pending, csov1alpha1.WaitingForPreCondition: // If WaitForPreCondition is mentioned. if !reflect.DeepEqual(stage.WaitForPreCondition, clusteraddon.WaitForCondition{}) { // Evaluate the condition. @@ -550,7 +559,7 @@ check: "failed to successfully evaluate pre condition: %q: %s", stage.HelmChartName, err.Error(), ) - in.clusterAddon.Status.HelmChartStatus[stage.HelmChartName] = csov1alpha1.WaitingForPreCondition + in.clusterAddon.SetStagePhase(stage.HelmChartName, stage.Action, csov1alpha1.WaitingForPreCondition) return true, nil } @@ -558,7 +567,8 @@ check: } logger.V(1).Info("finished evaluating pre condition", "clusterStack", in.clusterAddon.Spec.ClusterStack, "helm chart", stage.HelmChartName, "hook", in.clusterAddon.Spec.Hook) } - in.clusterAddon.Status.HelmChartStatus[stage.HelmChartName] = csov1alpha1.ApplyingOrDeleting + + in.clusterAddon.SetStagePhase(stage.HelmChartName, stage.Action, csov1alpha1.ApplyingOrDeleting) goto check case csov1alpha1.ApplyingOrDeleting: @@ -584,7 +594,7 @@ check: // remove status resource if applied successfully in.clusterAddon.Status.Resources = make([]*csov1alpha1.Resource, 0) - in.clusterAddon.Status.HelmChartStatus[stage.HelmChartName] = csov1alpha1.WaitingForPostCondition + in.clusterAddon.SetStagePhase(stage.HelmChartName, stage.Action, csov1alpha1.WaitingForPostCondition) goto check } else { @@ -610,7 +620,7 @@ check: // remove status resource if deleted successfully in.clusterAddon.Status.Resources = make([]*csov1alpha1.Resource, 0) - in.clusterAddon.Status.HelmChartStatus[stage.HelmChartName] = csov1alpha1.WaitingForPostCondition + in.clusterAddon.SetStagePhase(stage.HelmChartName, stage.Action, csov1alpha1.WaitingForPostCondition) goto check } @@ -636,7 +646,7 @@ check: logger.V(1).Info("finished evaluating post condition", "clusterStack", in.clusterAddon.Spec.ClusterStack, "helm chart", stage.HelmChartName, "hook", in.clusterAddon.Spec.Hook) } - in.clusterAddon.Status.HelmChartStatus[stage.HelmChartName] = csov1alpha1.Done + in.clusterAddon.SetStagePhase(stage.HelmChartName, stage.Action, csov1alpha1.Done) } return false, nil diff --git a/test/cluster-stacks/2/docker/ferrol/1-27/clusteraddon.yaml b/test/cluster-stacks/2/docker/ferrol/1-27/clusteraddon.yaml index 8f5753a04..4b39814f8 100644 --- a/test/cluster-stacks/2/docker/ferrol/1-27/clusteraddon.yaml +++ b/test/cluster-stacks/2/docker/ferrol/1-27/clusteraddon.yaml @@ -9,6 +9,8 @@ addonStages: BeforeClusterUpgrade: - helmChartName: metrics-server action: apply + - helmChartName: metrics-server + action: delete - helmChartName: ngnix action: apply waitForPostCondition: diff --git a/test/cluster-stacks/2/docker/ferrol/1-27/csmctl.yaml b/test/cluster-stacks/2/docker/ferrol/1-27/csmctl.yaml index 326cf844d..6b20cf03b 100644 --- a/test/cluster-stacks/2/docker/ferrol/1-27/csmctl.yaml +++ b/test/cluster-stacks/2/docker/ferrol/1-27/csmctl.yaml @@ -1,6 +1,6 @@ apiVersion: csmctl.clusterstack.x-k8s.io/v1alpha1 config: - kubernetesVersion: v1.27.7 + kubernetesVersion: v1.27.6 clusterStackName: ferrol provider: type: docker