Skip to content

Commit

Permalink
removed progressive status from error cases
Browse files Browse the repository at this point in the history
  • Loading branch information
Skarlso committed Nov 3, 2023
1 parent 1e6f52b commit 7f078d1
Show file tree
Hide file tree
Showing 12 changed files with 253 additions and 402 deletions.
8 changes: 7 additions & 1 deletion api/v1alpha1/condition_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const (
// ConvertComponentDescriptorFailedReason is used when the Component Descriptor cannot be converted.
ConvertComponentDescriptorFailedReason = "ConvertComponentDescriptorFailed"

// NameGenerationFailedReason is used when the componentn name could not be generated.
// NameGenerationFailedReason is used when the component name could not be generated.
NameGenerationFailedReason = "NameGenerationFailed"

// CreateOrUpdateComponentDescriptorFailedReason is used when the Component Descriptor cannot be created or updated on the resource.
Expand All @@ -44,6 +44,12 @@ const (
// ComponentDescriptorNotFoundReason is used when the component descriptor cannot be found.
ComponentDescriptorNotFoundReason = "ComponentDescriptorNotFound"

// ComponentVersionNotFoundReason is used when the component version cannot be found.
ComponentVersionNotFoundReason = "ComponentVersionNotFound"

// ComponentVersionNotReadyReason is used when the component version is not ready.
ComponentVersionNotReadyReason = "ComponentVersionNotReady"

// CreateOrUpdateSnapshotFailedReason is used when the snapshot cannot be created or updated.
CreateOrUpdateSnapshotFailedReason = "CreateOrUpdateSnapshotFailed"

Expand Down
11 changes: 11 additions & 0 deletions api/v1alpha1/localization_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,17 @@ type Localization struct {
Status MutationStatus `json:"status,omitempty"`
}

func (in *Localization) GetVID() map[string]string {
metadata := make(map[string]string)
metadata[GroupVersion.Group+"/localization_digest"] = in.Status.LatestSnapshotDigest

return metadata
}

func (in *Localization) SetObservedGeneration(v int64) {
in.Status.ObservedGeneration = v
}

// GetConditions returns the conditions of the Localization.
func (in *Localization) GetConditions() []metav1.Condition {
return in.Status.Conditions
Expand Down
11 changes: 11 additions & 0 deletions api/v1alpha1/resource_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,17 @@ type Resource struct {
Status ResourceStatus `json:"status,omitempty"`
}

func (in *Resource) GetVID() map[string]string {
metadata := make(map[string]string)
metadata[GroupVersion.Group+"/resource_version"] = in.Status.LastAppliedResourceVersion

return metadata
}

func (in *Resource) SetObservedGeneration(v int64) {
in.Status.ObservedGeneration = v
}

// GetConditions returns the conditions of the Resource.
func (in *Resource) GetConditions() []metav1.Condition {
return in.Status.Conditions
Expand Down
11 changes: 11 additions & 0 deletions api/v1alpha1/snapshot_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,17 @@ type SnapshotStatus struct {
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
}

func (in *Snapshot) GetVID() map[string]string {
metadata := make(map[string]string)
metadata[GroupVersion.Group+"/snapshot_digest"] = in.Status.LastReconciledDigest

return metadata
}

func (in *Snapshot) SetObservedGeneration(v int64) {
in.Status.ObservedGeneration = v
}

// GetComponentVersion returns the component version for the snapshot
func (in Snapshot) GetComponentVersion() string {
return in.Spec.Identity[ComponentVersionKey]
Expand Down
62 changes: 37 additions & 25 deletions controllers/componentversion_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,11 @@ 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.
_ = r.markAsFailed(
MarkAsFailed(
r.EventRecorder,
obj,
v1alpha1.AuthenticatedContextCreationFailedReason,
fmt.Errorf("authentication failed for repository: %s with error: %s", obj.Spec.Repository.URL, err),
fmt.Sprintf("authentication failed for repository: %s with error: %s", obj.Spec.Repository.URL, err),
)

return ctrl.Result{
Expand All @@ -116,10 +117,11 @@ 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.
_ = r.markAsFailed(
MarkAsFailed(
r.EventRecorder,
obj,
v1alpha1.CheckVersionFailedReason,
fmt.Errorf("version check failed for %s %s with error: %s", obj.Spec.Component, obj.Spec.Version.Semver, err),
fmt.Sprintf("version check failed for %s %s with error: %s", obj.Spec.Component, obj.Spec.Version.Semver, err),
)

return ctrl.Result{
Expand All @@ -144,10 +146,11 @@ func (r *ComponentVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req

ok, err := r.OCMClient.VerifyComponent(ctx, octx, obj, version)
if err != nil {
_ = r.markAsFailed(
MarkAsFailed(
r.EventRecorder,
obj,
v1alpha1.VerificationFailedReason,
fmt.Errorf("failed to verify %s with constraint %s with error: %s", obj.Spec.Component, obj.Spec.Version.Semver, err),
fmt.Sprintf("failed to verify %s with constraint %s with error: %s", obj.Spec.Component, obj.Spec.Version.Semver, err),
)

return ctrl.Result{
Expand All @@ -156,10 +159,11 @@ func (r *ComponentVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
}

if !ok {
_ = r.markAsFailed(
MarkAsFailed(
r.EventRecorder,
obj,
v1alpha1.VerificationFailedReason,
errors.New("attempted to verify component, but the digest didn't match"),
"attempted to verify component, but the digest didn't match",
)

return ctrl.Result{
Expand All @@ -178,11 +182,14 @@ 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 {
return ctrl.Result{}, r.markAsFailed(
err = fmt.Errorf("failed to get component version: %w", err)
MarkAsFailed(
r.EventRecorder,
obj,
v1alpha1.ComponentVersionInvalidReason,
fmt.Errorf("failed to get component version: %w", err),
err.Error(),
)
return ctrl.Result{}, err
}

defer cv.Close()
Expand All @@ -191,23 +198,29 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con
dv := &compdesc.DescriptorVersion{}
cd, err := dv.ConvertFrom(cv.GetDescriptor())
if err != nil {
return ctrl.Result{}, r.markAsFailed(
err = fmt.Errorf("failed to convert component descriptor: %w", err)
MarkAsFailed(
r.EventRecorder,
obj,
v1alpha1.ConvertComponentDescriptorFailedReason,
fmt.Errorf("failed to convert component descriptor: %w", err),
err.Error(),
)
return ctrl.Result{}, err
}

rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "component fetched, creating descriptors")

// setup the component descriptor kubernetes resource
componentName, err := component.ConstructUniqueName(cd.GetName(), cd.GetVersion(), nil)
if err != nil {
return ctrl.Result{}, r.markAsFailed(
err = fmt.Errorf("failed to generate name: %w", err)
MarkAsFailed(
r.EventRecorder,
obj,
v1alpha1.NameGenerationFailedReason,
fmt.Errorf("failed to generate name: %w", err),
err.Error(),
)
return ctrl.Result{}, err
}

descriptor := &v1alpha1.ComponentDescriptor{
Expand Down Expand Up @@ -237,11 +250,14 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con
})

if err != nil {
return ctrl.Result{}, r.markAsFailed(
err = fmt.Errorf("failed to create or update component descriptor: %w", err)
MarkAsFailed(
r.EventRecorder,
obj,
v1alpha1.CreateOrUpdateComponentDescriptorFailedReason,
fmt.Errorf("failed to create or update component descriptor: %w", err),
err.Error(),
)
return ctrl.Result{}, err
}

componentDescriptor := v1alpha1.Reference{
Expand All @@ -258,11 +274,14 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con

componentDescriptor.References, err = r.parseReferences(ctx, octx, obj, cv.GetDescriptor().References)
if err != nil {
return ctrl.Result{}, r.markAsFailed(
err = fmt.Errorf("failed to parse references: %w", err)
MarkAsFailed(
r.EventRecorder,
obj,
v1alpha1.ParseReferencesFailedReason,
fmt.Errorf("failed to parse references: %w", err),
err.Error(),
)
return ctrl.Result{}, err
}
}

Expand Down Expand Up @@ -403,10 +422,3 @@ func (r *ComponentVersionReconciler) createComponentDescriptor(ctx context.Conte

return descriptor, nil
}

func (r *ComponentVersionReconciler) markAsFailed(obj *v1alpha1.ComponentVersion, reason string, err error) error {
conditions.MarkFalse(obj, meta.ReadyCondition, reason, err.Error())
event.New(r.EventRecorder, obj, eventv1.EventSeverityError, err.Error(), nil)

return err
}
60 changes: 36 additions & 24 deletions controllers/configuration_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,11 @@ import (
"fmt"
"time"

eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
"github.com/fluxcd/pkg/apis/meta"
"github.com/fluxcd/pkg/runtime/conditions"
"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/event"
"golang.org/x/exp/slices"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -30,7 +28,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"
Expand Down Expand Up @@ -153,8 +150,6 @@ func (r *ConfigurationReconciler) Reconcile(
ctx context.Context,
req ctrl.Request,
) (result ctrl.Result, err error) {
logger := log.FromContext(ctx).WithName("configuration-controller")

obj := &v1alpha1.Configuration{}
if err = r.Client.Get(ctx, req.NamespacedName, obj); err != nil {
if apierrors.IsNotFound(err) {
Expand All @@ -166,8 +161,7 @@ func (r *ConfigurationReconciler) Reconcile(

// return early if obj is suspended
if obj.Spec.Suspend {
logger.Info("configuration object suspended")
return result, nil
return ctrl.Result{}, nil
}

patchHelper := patch.NewSerialPatcher(obj, r.Client)
Expand All @@ -187,25 +181,40 @@ func (r *ConfigurationReconciler) Reconcile(
// check dependencies are ready
ready, err := r.checkReadiness(ctx, obj.GetNamespace(), &obj.Spec.SourceRef)
if err != nil {
r.markAsFailed(obj, "SourceRefNotReadyWithError", err.Error(), "source ref not yet ready with error: %s", obj.Spec.SourceRef.Name)
MarkAsFailed(r.EventRecorder, obj, "SourceRefNotReadyWithError", err.Error())

return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
}
if !ready {
r.markAsFailed(obj, "SourceRefNotReady", "source not ready yet", "source ref not yet ready: %s", obj.Spec.SourceRef.Name)
MarkAsFailed(
r.EventRecorder,
obj,
"SourceRefNotReady",
fmt.Sprintf("source ref not yet ready: %s", obj.Spec.SourceRef.Name),
)

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

if obj.Spec.ConfigRef != nil {
ready, err := r.checkReadiness(ctx, obj.GetNamespace(), obj.Spec.ConfigRef)
if err != nil {
r.markAsFailed(obj, "ConfigRefNotReadyWithError", err.Error(), "config ref not yet ready with error: %s", obj.Spec.ConfigRef.Name)
MarkAsFailed(
r.EventRecorder,
obj,
"ConfigRefNotReadyWithError",
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 {
r.markAsFailed(obj, "ConfigRefNotReady", "config ref not ready", "config ref not yet ready: %s", obj.Spec.ConfigRef.Name)
MarkAsFailed(
r.EventRecorder,
obj,
"ConfigRefNotReady",
fmt.Sprintf("config ref not yet ready: %s", obj.Spec.ConfigRef.Name),
)

return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
}
Expand All @@ -214,13 +223,23 @@ func (r *ConfigurationReconciler) Reconcile(
if obj.Spec.PatchStrategicMerge != nil {
ready, err := r.checkFluxSourceReadiness(ctx, obj.Spec.PatchStrategicMerge.Source.SourceRef)
if err != nil {
r.markAsFailed(obj, "PatchStrategicMergeSourceRefNotReadyWithError", err.Error(), "patch strategic merge source ref not yet ready with error: %s", obj.Spec.PatchStrategicMerge.Source.SourceRef.Name)
MarkAsFailed(
r.EventRecorder,
obj,
"PatchStrategicMergeSourceRefNotReadyWithError",
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 {
r.markAsFailed(obj, "PatchStrategicMergeSourceRefNotReady", "patch strategic merge source not ready yet", "patch strategic merge source ref not yet ready: %s", obj.Spec.PatchStrategicMerge.Source.SourceRef.Name)
MarkAsFailed(
r.EventRecorder,
obj,
"PatchStrategicMergeSourceRefNotReady",
fmt.Sprintf("patch strategic merge source ref not yet ready: %s", obj.Spec.PatchStrategicMerge.Source.SourceRef.Name),
)

return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
}
Expand All @@ -231,7 +250,8 @@ func (r *ConfigurationReconciler) Reconcile(
if obj.GetSnapshotName() == "" {
name, err := snapshot.GenerateSnapshotName(obj.GetName())
if err != nil {
r.markAsFailed(obj, "GenerateSnapshotNameError", err.Error(), "failed to generate snapshot name for: %s", obj.GetName())
err := fmt.Errorf("failed to generate snapshot name for: %s: %s", obj.GetName(), err)
MarkAsFailed(r.EventRecorder, obj, v1alpha1.NameGenerationFailedReason, err.Error())

return ctrl.Result{}, err
}
Expand All @@ -248,8 +268,6 @@ func (r *ConfigurationReconciler) reconcile(
ctx context.Context,
obj *v1alpha1.Configuration,
) (ctrl.Result, error) {
rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "reconciliation in progress")

if obj.Generation != obj.Status.ObservedGeneration {
rreconcile.ProgressiveStatus(
false,
Expand All @@ -269,13 +287,13 @@ func (r *ConfigurationReconciler) reconcile(

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

return ctrl.Result{}, err
}

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

return ctrl.Result{}, err
}
Expand Down Expand Up @@ -452,9 +470,3 @@ func makeRequestsForConfigurations(ll ...v1alpha1.Configuration) []reconcile.Req

return requests
}

func (r *ConfigurationReconciler) markAsFailed(obj *v1alpha1.Configuration, reason, msg, format string, messageArgs ...any) {
rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, format, messageArgs...)
conditions.MarkFalse(obj, meta.ReadyCondition, reason, msg)
event.New(r.EventRecorder, obj, eventv1.EventSeverityError, msg, nil)
}
Loading

0 comments on commit 7f078d1

Please sign in to comment.