Skip to content

Commit

Permalink
refactor HelmRepository management
Browse files Browse the repository at this point in the history 
Prior to this commit the `HelmRepository` was being managed in a "Create if not
found" style which introduced needless logic now that SSA is GA'd.

This commit replaces the management of `HelmRepository` with SSA usage. It also
removes the "Progressing" state of the Redpanda resource as it would always be
immediately overridden by either Ready or NotReady. Notably, this change moves
the update of `ObservedGeneration` to the end of reconciliation rather than the
beginning.
  • Loading branch information
@chrisseto @RafalKorepta
chrisseto authored and RafalKorepta committed Oct 21, 2024
1 parent 9c9a033 commit bc225a5
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 71 deletions.
7 changes: 0 additions & 7 deletions operator/api/redpanda/v1alpha1/redpanda_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,3 @@ func RedpandaReady(rp *Redpanda) *Redpanda {
func RedpandaNotReady(rp *Redpanda, reason, message string) *Redpanda {
return (*Redpanda)(v1alpha2.RedpandaNotReady((*v1alpha2.Redpanda)(rp), reason, message))
}

// RedpandaProgressing resets any failures and registers progress toward
// reconciling the given Redpanda by setting the meta.ReadyCondition to
// 'Unknown' for meta.ProgressingReason.
func RedpandaProgressing(rp *Redpanda) *Redpanda {
return (*Redpanda)(v1alpha2.RedpandaProgressing((*v1alpha2.Redpanda)(rp)))
}
15 changes: 0 additions & 15 deletions operator/api/redpanda/v1alpha2/redpanda_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,21 +231,6 @@ func RedpandaNotReady(rp *Redpanda, reason, message string) *Redpanda {
return rp
}

// RedpandaProgressing resets any failures and registers progress toward
// reconciling the given Redpanda by setting the meta.ReadyCondition to
// 'Unknown' for meta.ProgressingReason.
func RedpandaProgressing(rp *Redpanda) *Redpanda {
rp.Status.Conditions = []metav1.Condition{}
newCondition := metav1.Condition{
Type: meta.ReadyCondition,
Status: metav1.ConditionUnknown,
Reason: meta.ProgressingReason,
Message: "Reconciliation in progress",
}
apimeta.SetStatusCondition(rp.GetConditions(), newCondition)
return rp
}

// GetConditions returns the status conditions of the object.
func (in *Redpanda) GetConditions() *[]metav1.Condition {
return &in.Status.Conditions
Expand Down
87 changes: 38 additions & 49 deletions operator/internal/controller/redpanda/redpanda_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,10 @@ func (r *RedpandaReconciler) Reconcile(c context.Context, req ctrl.Request) (ctr
return ctrl.Result{}, err
}

// Reconciliation has completed without any errors, therefore we observe our
// generation and persist any status changes.
rp.Status.ObservedGeneration = rp.Generation

// Update status after reconciliation.
if updateStatusErr := r.patchRedpandaStatus(ctx, rp); updateStatusErr != nil {
log.Error(updateStatusErr, "unable to update status after reconciliation")
Expand Down Expand Up @@ -273,16 +277,6 @@ func (r *RedpandaReconciler) reconcile(ctx context.Context, rp *v1alpha2.Redpand
log := ctrl.LoggerFrom(ctx)
log.WithName("RedpandaReconciler.reconcile")

// Observe HelmRelease generation.
if rp.Status.ObservedGeneration != rp.Generation {
rp.Status.ObservedGeneration = rp.Generation
rp = v1alpha2.RedpandaProgressing(rp)
if updateStatusErr := r.patchRedpandaStatus(ctx, rp); updateStatusErr != nil {
log.Error(updateStatusErr, "unable to update status after generation update")
return rp, updateStatusErr
}
}

// pull our deployments and stateful sets
redpandaStatefulSets, err := redpandaStatefulSetsForCluster(ctx, r.Client, rp)
if err != nil {
Expand All @@ -294,21 +288,13 @@ func (r *RedpandaReconciler) reconcile(ctx context.Context, rp *v1alpha2.Redpand
}

// Check if HelmRepository exists or create it
rp, repo, err := r.reconcileHelmRepository(ctx, rp)
if err != nil {
if err := r.reconcileHelmRepository(ctx, rp); err != nil {
return rp, err
}

isGenerationCurrent := repo.Generation != repo.Status.ObservedGeneration
isStatusConditionReady := apimeta.IsStatusConditionTrue(repo.Status.Conditions, meta.ReadyCondition)
msgNotReady := fmt.Sprintf(resourceNotReadyStrFmt, resourceTypeHelmRepository, repo.GetNamespace(), repo.GetName())
msgReady := fmt.Sprintf(resourceReadyStrFmt, resourceTypeHelmRepository, repo.GetNamespace(), repo.GetName())
isStatusReadyNILorTRUE := ptr.Equal(rp.Status.HelmRepositoryReady, ptr.To(true))
isStatusReadyNILorFALSE := ptr.Equal(rp.Status.HelmRepositoryReady, ptr.To(false))

isResourceReady := r.checkIfResourceIsReady(log, msgNotReady, msgReady, resourceTypeHelmRepository, isGenerationCurrent, isStatusConditionReady, isStatusReadyNILorTRUE, isStatusReadyNILorFALSE, rp)
if !isResourceReady {
if !ptr.Deref(rp.Status.HelmRepositoryReady, false) {
// strip out all of the requeues since this will get requeued based on the Owns in the setup of the reconciler
msgNotReady := fmt.Sprintf(resourceNotReadyStrFmt, resourceTypeHelmRepository, rp.Namespace, rp.GetHelmReleaseName())
return v1alpha2.RedpandaNotReady(rp, "ArtifactFailed", msgNotReady), nil
}

Expand All @@ -322,14 +308,14 @@ func (r *RedpandaReconciler) reconcile(ctx context.Context, rp *v1alpha2.Redpand
return rp, err
}

isGenerationCurrent = hr.Generation != hr.Status.ObservedGeneration
isStatusConditionReady = apimeta.IsStatusConditionTrue(hr.Status.Conditions, meta.ReadyCondition) || apimeta.IsStatusConditionTrue(hr.Status.Conditions, helmv2beta2.RemediatedCondition)
msgNotReady = fmt.Sprintf(resourceNotReadyStrFmt, resourceTypeHelmRelease, hr.GetNamespace(), hr.GetName())
msgReady = fmt.Sprintf(resourceReadyStrFmt, resourceTypeHelmRelease, hr.GetNamespace(), hr.GetName())
isStatusReadyNILorTRUE = ptr.Equal(rp.Status.HelmReleaseReady, ptr.To(true))
isStatusReadyNILorFALSE = ptr.Equal(rp.Status.HelmReleaseReady, ptr.To(false))
isGenerationCurrent := hr.Generation != hr.Status.ObservedGeneration
isStatusConditionReady := apimeta.IsStatusConditionTrue(hr.Status.Conditions, meta.ReadyCondition) || apimeta.IsStatusConditionTrue(hr.Status.Conditions, helmv2beta2.RemediatedCondition)
msgNotReady := fmt.Sprintf(resourceNotReadyStrFmt, resourceTypeHelmRelease, hr.GetNamespace(), hr.GetName())
msgReady := fmt.Sprintf(resourceReadyStrFmt, resourceTypeHelmRelease, hr.GetNamespace(), hr.GetName())
isStatusReadyNILorTRUE := ptr.Equal(rp.Status.HelmReleaseReady, ptr.To(true))
isStatusReadyNILorFALSE := ptr.Equal(rp.Status.HelmReleaseReady, ptr.To(false))

isResourceReady = r.checkIfResourceIsReady(log, msgNotReady, msgReady, resourceTypeHelmRelease, isGenerationCurrent, isStatusConditionReady, isStatusReadyNILorTRUE, isStatusReadyNILorFALSE, rp)
isResourceReady := r.checkIfResourceIsReady(log, msgNotReady, msgReady, resourceTypeHelmRelease, isGenerationCurrent, isStatusConditionReady, isStatusReadyNILorTRUE, isStatusReadyNILorFALSE, rp)
if !isResourceReady {
// strip out all of the requeues since this will get requeued based on the Owns in the setup of the reconciler
return v1alpha2.RedpandaNotReady(rp, "ArtifactFailed", msgNotReady), nil
Expand Down Expand Up @@ -366,8 +352,6 @@ func (r *RedpandaReconciler) checkIfResourceIsReady(log logr.Logger, msgNotReady
}

switch kind {
case resourceTypeHelmRepository:
rp.Status.HelmRepositoryReady = ptr.To(false)
case resourceTypeHelmRelease:
rp.Status.HelmReleaseReady = ptr.To(false)
}
Expand All @@ -378,8 +362,6 @@ func (r *RedpandaReconciler) checkIfResourceIsReady(log logr.Logger, msgNotReady
// here since the condition should be true, we update the value to
// be true, and send an event
switch kind {
case resourceTypeHelmRepository:
rp.Status.HelmRepositoryReady = ptr.To(true)
case resourceTypeHelmRelease:
rp.Status.HelmReleaseReady = ptr.To(true)
}
Expand Down Expand Up @@ -436,25 +418,20 @@ func (r *RedpandaReconciler) reconcileHelmRelease(ctx context.Context, rp *v1alp
return rp, hr, nil
}

func (r *RedpandaReconciler) reconcileHelmRepository(ctx context.Context, rp *v1alpha2.Redpanda) (*v1alpha2.Redpanda, *sourcev1.HelmRepository, error) {
// Check if HelmRepository exists or create it
repo := &sourcev1.HelmRepository{}
if err := r.Client.Get(ctx, types.NamespacedName{Namespace: rp.Namespace, Name: rp.GetHelmRepositoryName()}, repo); err != nil {
if apierrors.IsNotFound(err) {
repo = r.createHelmRepositoryFromTemplate(rp)
if errCreate := r.Client.Create(ctx, repo); errCreate != nil {
r.event(rp, rp.Status.LastAttemptedRevision, v1alpha2.EventSeverityError, fmt.Sprintf("error creating HelmRepository: %s", errCreate))
return rp, repo, fmt.Errorf("error creating HelmRepository: %w", errCreate)
}
r.event(rp, rp.Status.LastAttemptedRevision, v1alpha2.EventSeverityInfo, fmt.Sprintf("HelmRepository '%s/%s' created ", rp.Namespace, rp.GetHelmRepositoryName()))
} else {
r.event(rp, rp.Status.LastAttemptedRevision, v1alpha2.EventSeverityError, fmt.Sprintf("error getting HelmRepository: %s", err))
return rp, repo, fmt.Errorf("error getting HelmRepository: %w", err)
}
func (r *RedpandaReconciler) reconcileHelmRepository(ctx context.Context, rp *v1alpha2.Redpanda) error {
repo := r.helmRepositoryFromTemplate(rp)

if err := r.apply(ctx, repo); err != nil {
return fmt.Errorf("applying HelmRepository: %w", err)
}

isGenerationCurrent := repo.Generation == repo.Status.ObservedGeneration
isStatusConditionReady := apimeta.IsStatusConditionTrue(repo.Status.Conditions, meta.ReadyCondition)

rp.Status.HelmRepository = rp.GetHelmRepositoryName()
rp.Status.HelmRepositoryReady = ptr.To(isStatusConditionReady && isGenerationCurrent)

return rp, repo, nil
return nil
}

func (r *RedpandaReconciler) reconcileDelete(ctx context.Context, rp *v1alpha2.Redpanda) (ctrl.Result, error) {
Expand Down Expand Up @@ -596,7 +573,7 @@ func (r *RedpandaReconciler) createHelmReleaseFromTemplate(ctx context.Context,
}, nil
}

func (r *RedpandaReconciler) createHelmRepositoryFromTemplate(rp *v1alpha2.Redpanda) *sourcev1.HelmRepository {
func (r *RedpandaReconciler) helmRepositoryFromTemplate(rp *v1alpha2.Redpanda) *sourcev1.HelmRepository {
return &sourcev1.HelmRepository{
ObjectMeta: metav1.ObjectMeta{
Name: rp.GetHelmRepositoryName(),
Expand Down Expand Up @@ -651,6 +628,18 @@ func (r *RedpandaReconciler) helmReleaseRequiresUpdate(ctx context.Context, hr,
}
}

func (r *RedpandaReconciler) apply(ctx context.Context, obj client.Object) error {
gvk, err := r.Client.GroupVersionKindFor(obj)
if err != nil {
return err
}

obj.SetManagedFields(nil)
obj.GetObjectKind().SetGroupVersionKind(gvk)

return r.Client.Patch(ctx, obj, client.Apply, client.ForceOwnership, client.FieldOwner("redpanda-operator"))
}

// helmChartRequiresUpdate compares the v2beta1.HelmChartTemplate of the
// v2beta1.HelmRelease to the given v1beta2.HelmChart to determine if an
// update is required.
Expand Down

0 comments on commit bc225a5

Please sign in to comment.