Skip to content

Commit

Permalink
extracted status update logic and removed hardcoded statuses
Browse files Browse the repository at this point in the history
  • Loading branch information
Skarlso committed Nov 14, 2023
1 parent 1301b28 commit 48b6819
Show file tree
Hide file tree
Showing 9 changed files with 85 additions and 62 deletions.
18 changes: 18 additions & 0 deletions api/v1alpha1/condition_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,22 @@ const (

// CreateRepositoryNameReason is used when the generating a new repository name fails.
CreateRepositoryNameReason = "CreateRepositoryNameFailed"

// ConfigRefNotReadyWithErrorReason is used when configuration reference is not ready yet with an error.
ConfigRefNotReadyWithErrorReason = "ConfigRefNotReadyWithError"

// ConfigRefNotReadyReason is used when configuration ref is not ready yet and there was no error.
ConfigRefNotReadyReason = "ConfigRefNotReady"

// SourceRefNotReadyWithErrorReason is used when the source ref is not ready and there was an error.
SourceRefNotReadyWithErrorReason = "SourceRefNotReadyWithError"

// SourceRefNotReadyReason is used when the source ref is not ready and there was no error.
SourceRefNotReadyReason = "SourceRefNotReady"

// PatchStrategicMergeSourceRefNotReadyWithErrorReason is used when source ref for patch strategic merge is not ready and there was an error.
PatchStrategicMergeSourceRefNotReadyWithErrorReason = "PatchStrategicMergeSourceRefNotReadyWithError"

// PatchStrategicMergeSourceRefNotReadyReason is used when source ref for patch strategic merge is not ready and there was no error.
PatchStrategicMergeSourceRefNotReadyReason = "PatchStrategicMergeSourceRefNotReady"
)
21 changes: 11 additions & 10 deletions controllers/componentversion_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/fluxcd/pkg/runtime/conditions"
"github.com/fluxcd/pkg/runtime/patch"
rreconcile "github.com/fluxcd/pkg/runtime/reconcile"
"github.com/open-component-model/ocm-controller/pkg/status"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -139,7 +140,7 @@ func (r *ComponentVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req

// Always attempt to patch the object and status after each reconciliation.
defer func() {
if derr := updateStatus(ctx, patchHelper, obj, r.EventRecorder, obj.GetRequeueAfter()); derr != nil {
if derr := status.UpdateStatus(ctx, patchHelper, obj, r.EventRecorder, obj.GetRequeueAfter()); derr != nil {
retErr = errors.Join(retErr, derr)
}
}()
Expand All @@ -153,7 +154,7 @@ func (r *ComponentVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
if err != nil {
// we don't fail here, because all manifests might have been applied at once or the secret
// for authentication is being reconciled.
MarkAsStalled(
status.MarkAsStalled(
r.EventRecorder,
obj,
v1alpha1.AuthenticatedContextCreationFailedReason,
Expand All @@ -167,7 +168,7 @@ func (r *ComponentVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
update, version, err := r.checkVersion(ctx, octx, obj)
if err != nil {
// The component might not be there yet. We don't fail but keep polling instead.
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
v1alpha1.CheckVersionFailedReason,
Expand Down Expand Up @@ -196,7 +197,7 @@ func (r *ComponentVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req

ok, err := r.OCMClient.VerifyComponent(ctx, octx, obj, version)
if err != nil {
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
v1alpha1.VerificationFailedReason,
Expand All @@ -209,7 +210,7 @@ func (r *ComponentVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
}

if !ok {
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
v1alpha1.VerificationFailedReason,
Expand All @@ -233,7 +234,7 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con
cv, err := r.OCMClient.GetComponentVersion(ctx, octx, obj, obj.Spec.Component, version)
if err != nil {
err = fmt.Errorf("failed to get component version: %w", err)
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
v1alpha1.ComponentVersionInvalidReason,
Expand All @@ -249,7 +250,7 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con
cd, err := dv.ConvertFrom(cv.GetDescriptor())
if err != nil {
err = fmt.Errorf("failed to convert component descriptor: %w", err)
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
v1alpha1.ConvertComponentDescriptorFailedReason,
Expand All @@ -264,7 +265,7 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con
componentName, err := component.ConstructUniqueName(cd.GetName(), cd.GetVersion(), nil)
if err != nil {
err = fmt.Errorf("failed to generate name: %w", err)
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
v1alpha1.NameGenerationFailedReason,
Expand Down Expand Up @@ -301,7 +302,7 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con

if err != nil {
err = fmt.Errorf("failed to create or update component descriptor: %w", err)
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
v1alpha1.CreateOrUpdateComponentDescriptorFailedReason,
Expand All @@ -325,7 +326,7 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con
componentDescriptor.References, err = r.parseReferences(ctx, octx, obj, cv.GetDescriptor().References)
if err != nil {
err = fmt.Errorf("failed to parse references: %w", err)
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
v1alpha1.ParseReferencesFailedReason,
Expand Down
31 changes: 16 additions & 15 deletions controllers/configuration_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/fluxcd/pkg/runtime/patch"
rreconcile "github.com/fluxcd/pkg/runtime/reconcile"
sourcev1 "github.com/fluxcd/source-controller/api/v1"
"github.com/open-component-model/ocm-controller/pkg/status"
"golang.org/x/exp/slices"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -168,7 +169,7 @@ func (r *ConfigurationReconciler) Reconcile(

// Always attempt to patch the object and status after each reconciliation.
defer func() {
if derr := updateStatus(ctx, patchHelper, obj, r.EventRecorder, obj.GetRequeueAfter()); derr != nil {
if derr := status.UpdateStatus(ctx, patchHelper, obj, r.EventRecorder, obj.GetRequeueAfter()); derr != nil {
err = errors.Join(err, derr)
}
}()
Expand All @@ -181,16 +182,16 @@ func (r *ConfigurationReconciler) Reconcile(
// check dependencies are ready
ready, err := r.checkReadiness(ctx, obj.GetNamespace(), &obj.Spec.SourceRef)
if err != nil {
MarkNotReady(r.EventRecorder, obj, "SourceRefNotReadyWithError", err.Error())
status.MarkNotReady(r.EventRecorder, obj, v1alpha1.SourceRefNotReadyWithErrorReason, err.Error())

return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
}

if !ready {
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
"SourceRefNotReady",
v1alpha1.SourceRefNotReadyReason,
fmt.Sprintf("source ref not yet ready: %s", obj.Spec.SourceRef.Name),
)

Expand All @@ -200,20 +201,20 @@ func (r *ConfigurationReconciler) Reconcile(
if obj.Spec.ConfigRef != nil {
ready, err := r.checkReadiness(ctx, obj.GetNamespace(), obj.Spec.ConfigRef)
if err != nil {
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
"ConfigRefNotReadyWithError",
v1alpha1.ConfigRefNotReadyWithErrorReason,
fmt.Sprintf("config ref not yet ready with error: %s: %s", obj.Spec.ConfigRef.Name, err),
)

return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
}
if !ready {
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
"ConfigRefNotReady",
v1alpha1.ConfigRefNotReadyReason,
fmt.Sprintf("config ref not yet ready: %s", obj.Spec.ConfigRef.Name),
)

Expand All @@ -224,21 +225,21 @@ func (r *ConfigurationReconciler) Reconcile(
if obj.Spec.PatchStrategicMerge != nil {
ready, err := r.checkFluxSourceReadiness(ctx, obj.Spec.PatchStrategicMerge.Source.SourceRef)
if err != nil {
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
"PatchStrategicMergeSourceRefNotReadyWithError",
v1alpha1.PatchStrategicMergeSourceRefNotReadyWithErrorReason,
fmt.Sprintf("patch strategic merge source ref not yet ready with error: %s: %s", obj.Spec.PatchStrategicMerge.Source.SourceRef.Name, err),
)

return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
}

if !ready {
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
"PatchStrategicMergeSourceRefNotReady",
v1alpha1.PatchStrategicMergeSourceRefNotReadyReason,
fmt.Sprintf("patch strategic merge source ref not yet ready: %s", obj.Spec.PatchStrategicMerge.Source.SourceRef.Name),
)

Expand All @@ -252,7 +253,7 @@ func (r *ConfigurationReconciler) Reconcile(
name, err := snapshot.GenerateSnapshotName(obj.GetName())
if err != nil {
err := fmt.Errorf("failed to generate snapshot name for: %s: %s", obj.GetName(), err)
MarkNotReady(r.EventRecorder, obj, v1alpha1.NameGenerationFailedReason, err.Error())
status.MarkNotReady(r.EventRecorder, obj, v1alpha1.NameGenerationFailedReason, err.Error())

return ctrl.Result{}, err
}
Expand Down Expand Up @@ -288,13 +289,13 @@ func (r *ConfigurationReconciler) reconcile(

if errors.Is(err, errTar) {
err = fmt.Errorf("source resource is not a tar archive: %w", err)
MarkNotReady(r.EventRecorder, obj, v1alpha1.SourceReasonNotATarArchiveReason, err.Error())
status.MarkNotReady(r.EventRecorder, obj, v1alpha1.SourceReasonNotATarArchiveReason, err.Error())

return ctrl.Result{}, err
}

err = fmt.Errorf("failed to reconcile mutation object: %w", err)
MarkNotReady(r.EventRecorder, obj, v1alpha1.ReconcileMutationObjectFailedReason, err.Error())
status.MarkNotReady(r.EventRecorder, obj, v1alpha1.ReconcileMutationObjectFailedReason, err.Error())

return ctrl.Result{}, err
}
Expand Down
35 changes: 18 additions & 17 deletions controllers/localization_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
rreconcile "github.com/fluxcd/pkg/runtime/reconcile"
sourcev1 "github.com/fluxcd/source-controller/api/v1"
"github.com/fluxcd/source-controller/api/v1beta2"
"github.com/open-component-model/ocm-controller/pkg/status"
"golang.org/x/exp/slices"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -151,7 +152,7 @@ func (r *LocalizationReconciler) Reconcile(

// Always attempt to patch the object and status after each reconciliation.
defer func() {
if derr := updateStatus(ctx, patchHelper, obj, r.EventRecorder, obj.GetRequeueAfter()); derr != nil {
if derr := status.UpdateStatus(ctx, patchHelper, obj, r.EventRecorder, obj.GetRequeueAfter()); derr != nil {
err = errors.Join(err, derr)
}
}()
Expand All @@ -164,16 +165,16 @@ func (r *LocalizationReconciler) Reconcile(
// check dependencies are ready
ready, err := r.checkReadiness(ctx, obj.GetNamespace(), &obj.Spec.SourceRef)
if err != nil {
MarkNotReady(r.EventRecorder, obj, "SourceRefNotReadyWithError", err.Error())
status.MarkNotReady(r.EventRecorder, obj, v1alpha1.SourceRefNotReadyWithErrorReason, err.Error())

return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
}

if !ready {
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
"SourceRefNotReady",
v1alpha1.SourceRefNotReadyReason,
fmt.Sprintf("source ref not yet ready: %s", obj.Spec.SourceRef.Name),
)

Expand All @@ -183,20 +184,20 @@ func (r *LocalizationReconciler) Reconcile(
if obj.Spec.ConfigRef != nil {
ready, err := r.checkReadiness(ctx, obj.GetNamespace(), obj.Spec.ConfigRef)
if err != nil {
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
"ConfigRefNotReadyWithError",
v1alpha1.ConfigRefNotReadyWithErrorReason,
fmt.Sprintf("config ref not yet ready with error: %s: %s", obj.Spec.ConfigRef.Name, err),
)

return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
}
if !ready {
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
"ConfigRefNotReady",
v1alpha1.ConfigRefNotReadyReason,
fmt.Sprintf("config ref not yet ready: %s", obj.Spec.ConfigRef.Name),
)

Expand All @@ -207,21 +208,21 @@ func (r *LocalizationReconciler) Reconcile(
if obj.Spec.PatchStrategicMerge != nil {
ready, err := r.checkFluxSourceReadiness(ctx, obj.Spec.PatchStrategicMerge.Source.SourceRef)
if err != nil {
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
"PatchStrategicMergeSourceRefNotReadyWithError",
v1alpha1.PatchStrategicMergeSourceRefNotReadyWithErrorReason,
fmt.Sprintf("patch strategic merge source ref not yet ready with error: %s: %s", obj.Spec.PatchStrategicMerge.Source.SourceRef.Name, err),
)

return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
}

if !ready {
MarkNotReady(
status.MarkNotReady(
r.EventRecorder,
obj,
"PatchStrategicMergeSourceRefNotReady",
v1alpha1.PatchStrategicMergeSourceRefNotReadyReason,
fmt.Sprintf("patch strategic merge source ref not yet ready: %s", obj.Spec.PatchStrategicMerge.Source.SourceRef.Name),
)

Expand All @@ -235,7 +236,7 @@ func (r *LocalizationReconciler) Reconcile(
name, err := snapshot.GenerateSnapshotName(obj.GetName())
if err != nil {
err = fmt.Errorf("failed to generate snapshot name for: %s: %s", obj.GetName(), err)
MarkNotReady(r.EventRecorder, obj, v1alpha1.NameGenerationFailedReason, err.Error())
status.MarkNotReady(r.EventRecorder, obj, v1alpha1.NameGenerationFailedReason, err.Error())

return ctrl.Result{}, err
}
Expand Down Expand Up @@ -269,13 +270,13 @@ func (r *LocalizationReconciler) reconcile(

if errors.Is(err, errTar) {
err = fmt.Errorf("source resource is not a tar archive: %w", err)
MarkNotReady(r.EventRecorder, obj, v1alpha1.SourceReasonNotATarArchiveReason, err.Error())
status.MarkNotReady(r.EventRecorder, obj, v1alpha1.SourceReasonNotATarArchiveReason, err.Error())

return ctrl.Result{}, err
}

err = fmt.Errorf("failed to reconcile mutation object: %w", err)
MarkNotReady(r.EventRecorder, obj, v1alpha1.ReconcileMutationObjectFailedReason, err.Error())
status.MarkNotReady(r.EventRecorder, obj, v1alpha1.ReconcileMutationObjectFailedReason, err.Error())

return ctrl.Result{}, err
}
Expand Down Expand Up @@ -431,9 +432,9 @@ func makeRequestsForLocalizations(ll ...v1alpha1.Localization) []reconcile.Reque

requests := make([]reconcile.Request, len(refs))
for i, item := range refs {
// if the observedgeneration is -1
// if the ObservedGeneration is -1
// then the object has not been initialised yet
// so we should not trigger a reconcilation for sourceRef/configRefs
// so we should not trigger a reconciliation for sourceRef/configRefs
if item.Status.ObservedGeneration != -1 {
requests[i] = reconcile.Request{
NamespacedName: types.NamespacedName{
Expand Down
Loading

0 comments on commit 48b6819

Please sign in to comment.