From 29f63eb38b14096ccad59454d7d8a32a04a0f1f2 Mon Sep 17 00:00:00 2001 From: Andrei Pavlov Date: Tue, 15 Oct 2024 12:16:59 +0700 Subject: [PATCH] Wait for templates creation in Release controller Also report current state in conditions. Signed-off-by: Andrei Pavlov --- api/v1alpha1/common.go | 14 +++ api/v1alpha1/managedcluster_types.go | 14 --- api/v1alpha1/release_types.go | 11 ++- api/v1alpha1/zz_generated.deepcopy.go | 1 - internal/controller/release_controller.go | 92 ++++++++++++------- .../crds/hmc.mirantis.com_releases.yaml | 20 +--- .../hmc/templates/rbac/controller/roles.yaml | 6 ++ 7 files changed, 92 insertions(+), 66 deletions(-) diff --git a/api/v1alpha1/common.go b/api/v1alpha1/common.go index 0a25b4a70..5ee4e32e0 100644 --- a/api/v1alpha1/common.go +++ b/api/v1alpha1/common.go @@ -21,6 +21,20 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +const ( + // SucceededReason indicates a condition or event observed a success, for example when declared desired state + // matches actual state, or a performed action succeeded. + SucceededReason string = "Succeeded" + + // FailedReason indicates a condition or event observed a failure, for example when declared state does not match + // actual state, or a performed action failed. + FailedReason string = "Failed" + + // ProgressingReason indicates a condition or event observed progression, for example when the reconciliation of a + // resource or an action has started. + ProgressingReason string = "Progressing" +) + type ( // Providers hold different types of CAPI providers. Providers struct { diff --git a/api/v1alpha1/managedcluster_types.go b/api/v1alpha1/managedcluster_types.go index befb3122a..89a7f9bf9 100644 --- a/api/v1alpha1/managedcluster_types.go +++ b/api/v1alpha1/managedcluster_types.go @@ -50,20 +50,6 @@ const ( ReadyCondition string = "Ready" ) -const ( - // SucceededReason indicates a condition or event observed a success, for example when declared desired state - // matches actual state, or a performed action succeeded. - SucceededReason string = "Succeeded" - - // FailedReason indicates a condition or event observed a failure, for example when declared state does not match - // actual state, or a performed action failed. - FailedReason string = "Failed" - - // ProgressingReason indicates a condition or event observed progression, for example when the reconciliation of a - // resource or an action has started. - ProgressingReason string = "Progressing" -) - // ManagedClusterSpec defines the desired state of ManagedCluster type ManagedClusterSpec struct { // Config allows to provide parameters for template customization. diff --git a/api/v1alpha1/release_types.go b/api/v1alpha1/release_types.go index 8acf8bdaf..a20f1dc73 100644 --- a/api/v1alpha1/release_types.go +++ b/api/v1alpha1/release_types.go @@ -18,6 +18,13 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + ReleaseKind = "Release" + + // TemplatesCreatedCondition indicates that all templates associated with the Release are created. + TemplatesCreatedCondition = "TemplatesCreated" +) + // ReleaseSpec defines the desired state of Release type ReleaseSpec struct { // Version of the HMC Release in the semver format. @@ -52,12 +59,12 @@ func (in *Release) ProviderTemplate(name string) string { // ReleaseStatus defines the observed state of Release type ReleaseStatus struct { - // Templates indicates the status of templates associated with the Release. - Templates ComponentStatus `json:"templates,omitempty"` // Conditions contains details for the current state of the Release Conditions []metav1.Condition `json:"conditions,omitempty"` // Ready indicates whether HMC is ready to be upgraded to this Release. Ready bool `json:"ready,omitempty"` + // ObservedGeneration is the last observed generation. + ObservedGeneration int64 `json:"observedGeneration,omitempty"` } // +kubebuilder:object:root=true diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index a0d3c518a..e14ce1474 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -997,7 +997,6 @@ func (in *ReleaseSpec) DeepCopy() *ReleaseSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ReleaseStatus) DeepCopyInto(out *ReleaseStatus) { *out = *in - out.Templates = in.Templates if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]metav1.Condition, len(*in)) diff --git a/internal/controller/release_controller.go b/internal/controller/release_controller.go index 825126e29..79ac14676 100644 --- a/internal/controller/release_controller.go +++ b/internal/controller/release_controller.go @@ -21,14 +21,18 @@ import ( "fmt" hcv2 "github.com/fluxcd/helm-controller/api/v2" + fluxmeta "github.com/fluxcd/pkg/apis/meta" + fluxconditions "github.com/fluxcd/pkg/runtime/conditions" sourcev1 "github.com/fluxcd/source-controller/api/v1" "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/storage/driver" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" @@ -61,18 +65,31 @@ type ReleaseReconciler struct { CreateTemplates bool } -func (r *ReleaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +func (r *ReleaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, err error) { l := ctrl.LoggerFrom(ctx).WithValues("controller", "ReleaseController") l.Info("Reconciling Release") defer l.Info("Release reconcile is finished") - err := r.reconcileHMCTemplates(ctx, req) + release := &hmc.Release{} + if req.Name != "" { + if err := r.Client.Get(ctx, req.NamespacedName, release); err != nil { + l.Error(err, "failed to get Release") + return ctrl.Result{}, err + } + defer func() { + release.Status.ObservedGeneration = release.Generation + err = errors.Join(err, r.Status().Update(ctx, release)) + }() + } + + err = r.reconcileHMCTemplates(ctx, release.Name, release.Spec.Version, release.UID) + r.updateTemplatesCondition(release, err) if err != nil { l.Error(err, "failed to reconcile HMC Templates") return ctrl.Result{}, err } - if initialReconcile(req) { + if release.Name == "" { if err := r.ensureManagement(ctx); err != nil { l.Error(err, "failed to get or create Management object") return ctrl.Result{}, err @@ -81,8 +98,23 @@ func (r *ReleaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct return ctrl.Result{}, nil } -func initialReconcile(req ctrl.Request) bool { - return req.Name == "" +func (r *ReleaseReconciler) updateTemplatesCondition(release *hmc.Release, err error) { + condition := metav1.Condition{ + Type: hmc.TemplatesCreatedCondition, + Status: metav1.ConditionTrue, + ObservedGeneration: release.Generation, + Reason: hmc.SucceededReason, + Message: "All templates have been created", + } + if !r.CreateTemplates { + condition.Message = "Templates creation is disabled" + } + if err != nil { + condition.Status = metav1.ConditionFalse + condition.Message = err.Error() + condition.Reason = hmc.FailedReason + } + meta.SetStatusCondition(&release.Status.Conditions, condition) } func (r *ReleaseReconciler) ensureManagement(ctx context.Context) error { @@ -150,45 +182,36 @@ func (r *ReleaseReconciler) ensureManagement(ctx context.Context) error { return nil } -func (r *ReleaseReconciler) reconcileHMCTemplates(ctx context.Context, req ctrl.Request) error { +func (r *ReleaseReconciler) reconcileHMCTemplates(ctx context.Context, releaseName, releaseVersion string, releaseUID types.UID) error { l := ctrl.LoggerFrom(ctx) if !r.CreateTemplates { l.Info("Templates creation is disabled") return nil } - if initialReconcile(req) && !r.CreateRelease { + if releaseName == "" && !r.CreateRelease { l.Info("Initial creation of HMC Release is skipped") return nil } - releaseName := utils.ReleaseNameFromVersion(build.Version) - version := build.Version var ownerRefs []metav1.OwnerReference - release := &hmc.Release{} - if !initialReconcile(req) { - if err := r.Client.Get(ctx, req.NamespacedName, release); err != nil { - l.Error(err, "failed to get Release") + if releaseName == "" { + releaseName = utils.ReleaseNameFromVersion(build.Version) + releaseVersion = build.Version + err := helm.ReconcileHelmRepository(ctx, r.Client, defaultRepoName, r.SystemNamespace, r.DefaultRegistryConfig.HelmRepositorySpec()) + if err != nil { + l.Error(err, "Failed to reconcile default HelmRepository", "namespace", r.SystemNamespace) return err } - releaseName = req.Name - version = release.Spec.Version + } else { ownerRefs = []metav1.OwnerReference{ { APIVersion: hmc.GroupVersion.String(), - Kind: release.Kind, - Name: release.Name, - UID: release.UID, + Kind: hmc.ReleaseKind, + Name: releaseName, + UID: releaseUID, }, } } - if initialReconcile(req) { - err := helm.ReconcileHelmRepository(ctx, r.Client, defaultRepoName, r.SystemNamespace, r.DefaultRegistryConfig.HelmRepositorySpec()) - if err != nil { - l.Error(err, "Failed to reconcile default HelmRepository", "namespace", r.SystemNamespace) - return err - } - } - hmcTemplatesName := utils.TemplatesChartFromReleaseName(releaseName) helmChart := &sourcev1.HelmChart{ ObjectMeta: metav1.ObjectMeta{ @@ -205,7 +228,7 @@ func (r *ReleaseReconciler) reconcileHMCTemplates(ctx context.Context, req ctrl. helmChart.Labels[hmc.HMCManagedLabelKey] = hmc.HMCManagedLabelValue helmChart.Spec = sourcev1.HelmChartSpec{ Chart: r.HMCTemplatesChartName, - Version: version, + Version: releaseVersion, SourceRef: sourcev1.LocalHelmChartSourceReference{ Kind: sourcev1.HelmRepositoryKind, Name: defaultRepoName, @@ -221,10 +244,6 @@ func (r *ReleaseReconciler) reconcileHMCTemplates(ctx context.Context, req ctrl. l.Info(fmt.Sprintf("Successfully %s %s/%s HelmChart", operation, r.SystemNamespace, hmcTemplatesName)) } - if _, err := helm.ArtifactReady(helmChart); err != nil { - return fmt.Errorf("HelmChart %s/%s Artifact is not ready: %w", r.SystemNamespace, hmcTemplatesName, err) - } - opts := helm.ReconcileHelmReleaseOpts{ ChartRef: &hcv2.CrossNamespaceSourceReference{ Kind: helmChart.Kind, @@ -233,7 +252,7 @@ func (r *ReleaseReconciler) reconcileHMCTemplates(ctx context.Context, req ctrl. }, } - if initialReconcile(req) { + if releaseName == "" { createReleaseValues := map[string]any{ "createRelease": true, } @@ -244,13 +263,20 @@ func (r *ReleaseReconciler) reconcileHMCTemplates(ctx context.Context, req ctrl. opts.Values = &apiextensionsv1.JSON{Raw: raw} } - _, operation, err = helm.ReconcileHelmRelease(ctx, r.Client, hmcTemplatesName, r.SystemNamespace, opts) + hr, operation, err := helm.ReconcileHelmRelease(ctx, r.Client, hmcTemplatesName, r.SystemNamespace, opts) if err != nil { return err } if operation == controllerutil.OperationResultCreated || operation == controllerutil.OperationResultUpdated { l.Info(fmt.Sprintf("Successfully %s %s/%s HelmRelease", operation, r.SystemNamespace, hmcTemplatesName)) } + hrReadyCondition := fluxconditions.Get(hr, fluxmeta.ReadyCondition) + if hrReadyCondition == nil || hrReadyCondition.ObservedGeneration != hr.Generation { + return fmt.Errorf("HelmRelease %s/%s is not ready yet. Waiting for reconciliation", r.SystemNamespace, hmcTemplatesName) + } + if hrReadyCondition.Status == metav1.ConditionFalse { + return fmt.Errorf("HelmRelease %s/%s is not ready yet. %s", r.SystemNamespace, hmcTemplatesName, hrReadyCondition.Message) + } return nil } diff --git a/templates/provider/hmc/templates/crds/hmc.mirantis.com_releases.yaml b/templates/provider/hmc/templates/crds/hmc.mirantis.com_releases.yaml index ee19b66f7..4ff05bbd2 100644 --- a/templates/provider/hmc/templates/crds/hmc.mirantis.com_releases.yaml +++ b/templates/provider/hmc/templates/crds/hmc.mirantis.com_releases.yaml @@ -145,26 +145,14 @@ spec: - type type: object type: array + observedGeneration: + description: ObservedGeneration is the last observed generation. + format: int64 + type: integer ready: description: Ready indicates whether HMC is ready to be upgraded to this Release. type: boolean - templates: - description: Templates indicates the status of templates associated - with the Release. - properties: - error: - description: Error stores as error message in case of failed installation - type: string - success: - description: Success represents if a component installation was - successful - type: boolean - template: - description: Template is the name of the Template associated with - this component. - type: string - type: object type: object type: object served: true diff --git a/templates/provider/hmc/templates/rbac/controller/roles.yaml b/templates/provider/hmc/templates/rbac/controller/roles.yaml index 21c49b962..739275821 100644 --- a/templates/provider/hmc/templates/rbac/controller/roles.yaml +++ b/templates/provider/hmc/templates/rbac/controller/roles.yaml @@ -54,6 +54,12 @@ rules: - get - list - watch +- apiGroups: + - hmc.mirantis.com + resources: + - releases/status + verbs: + - update - apiGroups: - hmc.mirantis.com resources: