Merge pull request #612 from fluxcd/granular-reasons

api/v1beta2/condition_types.go

@@ -43,17 +43,41 @@ const (
 	// of a Source's Artifact.
 	// If True, the Source can be in an ArtifactOutdatedCondition.
 	BuildFailedCondition string = "BuildFailed"
+
+	// StorageOperationFailedCondition indicates a transient or persistent
+	// failure related to storage. If True, the reconciliation failed while
+	// performing some filesystem operation.
+	StorageOperationFailedCondition string = "StorageOperationFailed"
 )
 
 const (
 	// URLInvalidReason signals that a given Source has an invalid URL.
 	URLInvalidReason string = "URLInvalid"
 
-	// StorageOperationFailedReason signals a failure caused by a storage
-	// operation.
-	StorageOperationFailedReason string = "StorageOperationFailed"
-
 	// AuthenticationFailedReason signals that a Secret does not have the
 	// required fields, or the provided credentials do not match.
 	AuthenticationFailedReason string = "AuthenticationFailed"
+
+	// DirCreationFailedReason signals a failure caused by a directory creation
+	// operation.
+	DirCreationFailedReason string = "DirectoryCreationFailed"
+
+	// StatOperationFailedReason signals a failure caused by a stat operation on
+	// a path.
+	StatOperationFailedReason string = "StatOperationFailed"
+
+	// ReadOperationFailedReason signals a failure caused by a read operation.
+	ReadOperationFailedReason string = "ReadOperationFailed"
+
+	// AcquireLockFailedReason signals a failure in acquiring lock.
+	AcquireLockFailedReason string = "AcquireLockFailed"
+
+	// InvalidPathReason signals a failure caused by an invalid path.
+	InvalidPathReason string = "InvalidPath"
+
+	// ArchiveOperationFailedReason signals a failure in archive operation.
+	ArchiveOperationFailedReason string = "ArchiveOperationFailed"
+
+	// SymlinkUpdateFailedReason signals a failure in updating a symlink.
+	SymlinkUpdateFailedReason string = "SymlinkUpdateFailed"
 )

controllers/bucket_controller.go

@@ -74,21 +74,24 @@ const maxConcurrentBucketFetches = 100
 var bucketReadyCondition = summarize.Conditions{
 	Target: meta.ReadyCondition,
 	Owned: []string{
-		sourcev1.ArtifactOutdatedCondition,
 		sourcev1.FetchFailedCondition,
+		sourcev1.ArtifactOutdatedCondition,
+		sourcev1.StorageOperationFailedCondition,
 		meta.ReadyCondition,
 		meta.ReconcilingCondition,
 		meta.StalledCondition,
 	},
 	Summarize: []string{
-		sourcev1.ArtifactOutdatedCondition,
 		sourcev1.FetchFailedCondition,
+		sourcev1.ArtifactOutdatedCondition,
+		sourcev1.StorageOperationFailedCondition,
 		meta.StalledCondition,
 		meta.ReconcilingCondition,
 	},
 	NegativePolarity: []string{
-		sourcev1.ArtifactOutdatedCondition,
 		sourcev1.FetchFailedCondition,
+		sourcev1.ArtifactOutdatedCondition,
+		sourcev1.StorageOperationFailedCondition,
 		meta.StalledCondition,
 		meta.ReconcilingCondition,
 	},
@@ -313,16 +316,19 @@ func (r *BucketReconciler) reconcile(ctx context.Context, obj *sourcev1.Bucket,
 	// Create temp working dir
 	tmpDir, err := os.MkdirTemp("", fmt.Sprintf("%s-%s-%s-", obj.Kind, obj.Namespace, obj.Name))
 	if err != nil {
-		return sreconcile.ResultEmpty, &serror.Event{
+		e := &serror.Event{
-			Err:    fmt.Errorf("failed to create temporary directory: %w", err),
+			Err:    fmt.Errorf("failed to create temporary working directory: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.DirCreationFailedReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
+		return sreconcile.ResultEmpty, e
 	}
 	defer func() {
 		if err = os.RemoveAll(tmpDir); err != nil {
 			ctrl.LoggerFrom(ctx).Error(err, "failed to remove temporary working directory")
 		}
 	}()
+	conditions.Delete(obj, sourcev1.StorageOperationFailedCondition)
 
 	// Run the sub-reconcilers and build the result of reconciliation.
 	var (
@@ -521,23 +527,29 @@ func (r *BucketReconciler) reconcileArtifact(ctx context.Context, obj *sourcev1.
 
 	// Ensure target path exists and is a directory
 	if f, err := os.Stat(dir); err != nil {
-		return sreconcile.ResultEmpty, &serror.Event{
+		e := &serror.Event{
 			Err:    fmt.Errorf("failed to stat source path: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.StatOperationFailedReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
+		return sreconcile.ResultEmpty, e
 	} else if !f.IsDir() {
-		return sreconcile.ResultEmpty, &serror.Event{
+		e := &serror.Event{
 			Err:    fmt.Errorf("source path '%s' is not a directory", dir),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.InvalidPathReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
+		return sreconcile.ResultEmpty, e
 	}
 
 	// Ensure artifact directory exists and acquire lock
 	if err := r.Storage.MkdirAll(artifact); err != nil {
-		return sreconcile.ResultEmpty, &serror.Event{
+		e := &serror.Event{
 			Err:    fmt.Errorf("failed to create artifact directory: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.DirCreationFailedReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
+		return sreconcile.ResultEmpty, e
 	}
 	unlock, err := r.Storage.Lock(artifact)
 	if err != nil {
@@ -550,10 +562,12 @@ func (r *BucketReconciler) reconcileArtifact(ctx context.Context, obj *sourcev1.
 
 	// Archive directory to storage
 	if err := r.Storage.Archive(&artifact, dir, nil); err != nil {
-		return sreconcile.ResultEmpty, &serror.Event{
+		e := &serror.Event{
 			Err:    fmt.Errorf("unable to archive artifact to storage: %s", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.ArchiveOperationFailedReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
+		return sreconcile.ResultEmpty, e
 	}
 	r.annotatedEventLogf(ctx, obj, map[string]string{
 		"revision": artifact.Revision,
@@ -566,12 +580,13 @@ func (r *BucketReconciler) reconcileArtifact(ctx context.Context, obj *sourcev1.
 	// Update symlink on a "best effort" basis
 	url, err := r.Storage.Symlink(artifact, "latest.tar.gz")
 	if err != nil {
-		r.eventLogf(ctx, obj, corev1.EventTypeWarning, sourcev1.StorageOperationFailedReason,
+		r.eventLogf(ctx, obj, events.EventTypeTrace, sourcev1.SymlinkUpdateFailedReason,
 			"failed to update status URL symlink: %s", err)
 	}
 	if url != "" {
 		obj.Status.URL = url
 	}
+	conditions.Delete(obj, sourcev1.StorageOperationFailedCondition)
 	return sreconcile.ResultSuccess, nil
 }
 

controllers/bucket_controller_test.go

@@ -947,6 +947,9 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
 			},
 			want:    sreconcile.ResultEmpty,
 			wantErr: true,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(sourcev1.StorageOperationFailedCondition, sourcev1.StatOperationFailedReason, "failed to stat source path"),
+			},
 		},
 		{
 			name: "Dir path is not a directory",
@@ -963,6 +966,9 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
 			},
 			want:    sreconcile.ResultEmpty,
 			wantErr: true,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(sourcev1.StorageOperationFailedCondition, sourcev1.InvalidPathReason, "is not a directory"),
+			},
 		},
 	}
 

controllers/gitrepository_controller.go

@@ -63,6 +63,7 @@ var gitRepositoryReadyCondition = summarize.Conditions{
 		sourcev1.FetchFailedCondition,
 		sourcev1.IncludeUnavailableCondition,
 		sourcev1.ArtifactOutdatedCondition,
+		sourcev1.StorageOperationFailedCondition,
 		meta.ReadyCondition,
 		meta.ReconcilingCondition,
 		meta.StalledCondition,
@@ -72,6 +73,7 @@ var gitRepositoryReadyCondition = summarize.Conditions{
 		sourcev1.SourceVerifiedCondition,
 		sourcev1.FetchFailedCondition,
 		sourcev1.ArtifactOutdatedCondition,
+		sourcev1.StorageOperationFailedCondition,
 		meta.StalledCondition,
 		meta.ReconcilingCondition,
 	},
@@ -79,6 +81,7 @@ var gitRepositoryReadyCondition = summarize.Conditions{
 		sourcev1.FetchFailedCondition,
 		sourcev1.IncludeUnavailableCondition,
 		sourcev1.ArtifactOutdatedCondition,
+		sourcev1.StorageOperationFailedCondition,
 		meta.StalledCondition,
 		meta.ReconcilingCondition,
 	},
@@ -213,16 +216,19 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.G
 	// Create temp dir for Git clone
 	tmpDir, err := util.TempDirForObj("", obj)
 	if err != nil {
-		return sreconcile.ResultEmpty, &serror.Event{
+		e := &serror.Event{
-			Err:    fmt.Errorf("failed to create temporary directory: %w", err),
+			Err:    fmt.Errorf("failed to create temporary working directory: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.DirCreationFailedReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
+		return sreconcile.ResultEmpty, e
 	}
 	defer func() {
 		if err = os.RemoveAll(tmpDir); err != nil {
 			ctrl.LoggerFrom(ctx).Error(err, "failed to remove temporary working directory")
 		}
 	}()
+	conditions.Delete(obj, sourcev1.StorageOperationFailedCondition)
 
 	// Run the sub-reconcilers and build the result of reconciliation.
 	var (
@@ -322,7 +328,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
 				Err:    fmt.Errorf("failed to get secret '%s': %w", name.String(), err),
 				Reason: sourcev1.AuthenticationFailedReason,
 			}
-			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, e.Err.Error())
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 			// Return error as the world as observed may change
 			return sreconcile.ResultEmpty, e
 		}
@@ -338,7 +344,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
 			Err:    fmt.Errorf("failed to configure auth strategy for Git implementation '%s': %w", obj.Spec.GitImplementation, err),
 			Reason: sourcev1.AuthenticationFailedReason,
 		}
-		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, e.Err.Error())
+		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 		// Return error as the contents of the secret may change
 		return sreconcile.ResultEmpty, e
 	}
@@ -358,7 +364,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
 			Err:    fmt.Errorf("failed to configure checkout strategy for Git implementation '%s': %w", obj.Spec.GitImplementation, err),
 			Reason: sourcev1.GitOperationFailedReason,
 		}
-		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, e.Err.Error())
+		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 		// Do not return err as recovery without changes is impossible
 		return sreconcile.ResultEmpty, e
 	}
@@ -372,7 +378,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
 			Err:    fmt.Errorf("failed to checkout and determine revision: %w", err),
 			Reason: sourcev1.GitOperationFailedReason,
 		}
-		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, e.Err.Error())
+		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 		// Coin flip on transient or persistent error, return error and hope for the best
 		return sreconcile.ResultEmpty, e
 	}
@@ -429,15 +435,17 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context,
 	// Ensure target path exists and is a directory
 	if f, err := os.Stat(dir); err != nil {
 		e := &serror.Event{
-			Err:    fmt.Errorf("failed to stat target path: %w", err),
+			Err:    fmt.Errorf("failed to stat target artifact path: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.StatOperationFailedReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
 		return sreconcile.ResultEmpty, e
 	} else if !f.IsDir() {
 		e := &serror.Event{
 			Err:    fmt.Errorf("invalid target path: '%s' is not a directory", dir),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.InvalidPathReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
 		return sreconcile.ResultEmpty, e
 	}
 
@@ -445,8 +453,9 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context,
 	if err := r.Storage.MkdirAll(artifact); err != nil {
 		e := &serror.Event{
 			Err:    fmt.Errorf("failed to create artifact directory: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.DirCreationFailedReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
 		return sreconcile.ResultEmpty, e
 	}
 	unlock, err := r.Storage.Lock(artifact)
@@ -472,10 +481,12 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context,
 
 	// Archive directory to storage
 	if err := r.Storage.Archive(&artifact, dir, SourceIgnoreFilter(ps, nil)); err != nil {
-		return sreconcile.ResultEmpty, &serror.Event{
+		e := &serror.Event{
 			Err:    fmt.Errorf("unable to archive artifact to storage: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.ArchiveOperationFailedReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
+		return sreconcile.ResultEmpty, e
 	}
 	r.AnnotatedEventf(obj, map[string]string{
 		"revision": artifact.Revision,
@@ -489,12 +500,13 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context,
 	// Update symlink on a "best effort" basis
 	url, err := r.Storage.Symlink(artifact, "latest.tar.gz")
 	if err != nil {
-		r.eventLogf(ctx, obj, corev1.EventTypeWarning, sourcev1.StorageOperationFailedReason,
+		r.eventLogf(ctx, obj, events.EventTypeTrace, sourcev1.SymlinkUpdateFailedReason,
 			"failed to update status URL symlink: %s", err)
 	}
 	if url != "" {
 		obj.Status.URL = url
 	}
+	conditions.Delete(obj, sourcev1.StorageOperationFailedCondition)
 	return sreconcile.ResultSuccess, nil
 }
 
@@ -520,7 +532,7 @@ func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context,
 				Err:    fmt.Errorf("path calculation for include '%s' failed: %w", incl.GitRepositoryRef.Name, err),
 				Reason: "IllegalPath",
 			}
-			conditions.MarkTrue(obj, sourcev1.IncludeUnavailableCondition, "IllegalPath", e.Err.Error())
+			conditions.MarkTrue(obj, sourcev1.IncludeUnavailableCondition, e.Reason, e.Err.Error())
 			return sreconcile.ResultEmpty, e
 		}
 
@@ -531,7 +543,7 @@ func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context,
 				Err:    fmt.Errorf("could not get resource for include '%s': %w", incl.GitRepositoryRef.Name, err),
 				Reason: "NotFound",
 			}
-			conditions.MarkTrue(obj, sourcev1.IncludeUnavailableCondition, "NotFound", e.Err.Error())
+			conditions.MarkTrue(obj, sourcev1.IncludeUnavailableCondition, e.Reason, e.Err.Error())
 			return sreconcile.ResultEmpty, err
 		}
 
@@ -541,7 +553,7 @@ func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context,
 				Err:    fmt.Errorf("no artifact available for include '%s'", incl.GitRepositoryRef.Name),
 				Reason: "NoArtifact",
 			}
-			conditions.MarkTrue(obj, sourcev1.IncludeUnavailableCondition, "NoArtifact", e.Err.Error())
+			conditions.MarkTrue(obj, sourcev1.IncludeUnavailableCondition, e.Reason, e.Err.Error())
 			return sreconcile.ResultEmpty, e
 		}
 
@@ -551,7 +563,7 @@ func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context,
 				Err:    fmt.Errorf("failed to copy '%s' include from %s to %s: %w", incl.GitRepositoryRef.Name, incl.GetFromPath(), incl.GetToPath(), err),
 				Reason: "CopyFailure",
 			}
-			conditions.MarkTrue(obj, sourcev1.IncludeUnavailableCondition, "CopyFailure", e.Err.Error())
+			conditions.MarkTrue(obj, sourcev1.IncludeUnavailableCondition, e.Reason, e.Err.Error())
 			return sreconcile.ResultEmpty, e
 		}
 		artifacts[i] = dep.GetArtifact().DeepCopy()
@@ -598,7 +610,7 @@ func (r *GitRepositoryReconciler) verifyCommitSignature(ctx context.Context, obj
 			Err:    fmt.Errorf("PGP public keys secret error: %w", err),
 			Reason: "VerificationError",
 		}
-		conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, meta.FailedReason, e.Err.Error())
+		conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, e.Err.Error())
 		return sreconcile.ResultEmpty, e
 	}
 
@@ -612,7 +624,7 @@ func (r *GitRepositoryReconciler) verifyCommitSignature(ctx context.Context, obj
 			Err:    fmt.Errorf("signature verification of commit '%s' failed: %w", commit.Hash.String(), err),
 			Reason: "InvalidCommitSignature",
 		}
-		conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, meta.FailedReason, e.Err.Error())
+		conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, e.Err.Error())
 		// Return error in the hope the secret changes
 		return sreconcile.ResultEmpty, e
 	}

controllers/gitrepository_controller_test.go

@@ -812,11 +812,17 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			name:    "Target path does not exists",
 			dir:     "testdata/git/foo",
 			wantErr: true,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(sourcev1.StorageOperationFailedCondition, sourcev1.StatOperationFailedReason, "failed to stat target artifact path"),
+			},
 		},
 		{
 			name:    "Target path is not a directory",
 			dir:     "testdata/git/repository/foo.txt",
 			wantErr: true,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(sourcev1.StorageOperationFailedCondition, sourcev1.InvalidPathReason, "invalid target path"),
+			},
 		},
 	}
 	artifactSize := func(g *WithT, artifactURL string) *int64 {
@@ -1172,7 +1178,7 @@ func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) {
 			},
 			wantErr: true,
 			assertConditions: []metav1.Condition{
-				*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, meta.FailedReason, "signature verification of commit 'shasum' failed: failed to verify commit with any of the given key rings"),
+				*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, "InvalidCommitSignature", "signature verification of commit 'shasum' failed: failed to verify commit with any of the given key rings"),
 			},
 		},
 		{
@@ -1188,7 +1194,7 @@ func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) {
 			},
 			wantErr: true,
 			assertConditions: []metav1.Condition{
-				*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, meta.FailedReason, "PGP public keys secret error: secrets \"none-existing\" not found"),
+				*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, "VerificationError", "PGP public keys secret error: secrets \"none-existing\" not found"),
 			},
 		},
 		{

controllers/helmchart_controller.go

@@ -72,6 +72,7 @@ var helmChartReadyCondition = summarize.Conditions{
 		sourcev1.BuildFailedCondition,
 		sourcev1.FetchFailedCondition,
 		sourcev1.ArtifactOutdatedCondition,
+		sourcev1.StorageOperationFailedCondition,
 		meta.ReadyCondition,
 		meta.ReconcilingCondition,
 		meta.StalledCondition,
@@ -80,6 +81,7 @@ var helmChartReadyCondition = summarize.Conditions{
 		sourcev1.BuildFailedCondition,
 		sourcev1.FetchFailedCondition,
 		sourcev1.ArtifactOutdatedCondition,
+		sourcev1.StorageOperationFailedCondition,
 		meta.StalledCondition,
 		meta.ReconcilingCondition,
 	},
@@ -87,6 +89,7 @@ var helmChartReadyCondition = summarize.Conditions{
 		sourcev1.BuildFailedCondition,
 		sourcev1.FetchFailedCondition,
 		sourcev1.ArtifactOutdatedCondition,
+		sourcev1.StorageOperationFailedCondition,
 		meta.StalledCondition,
 		meta.ReconcilingCondition,
 	},
@@ -305,7 +308,7 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, obj *sourcev1
 			Err:    fmt.Errorf("failed to get source: %w", err),
 			Reason: "SourceUnavailable",
 		}
-		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, "SourceUnavailable", e.Err.Error())
+		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 
 		// Return Kubernetes client errors, but ignore others which can only be
 		// solved by a change in generation
@@ -395,7 +398,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 				Err:    fmt.Errorf("failed to get secret '%s': %w", repo.Spec.SecretRef.Name, err),
 				Reason: sourcev1.AuthenticationFailedReason,
 			}
-			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, e.Err.Error())
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 			// Return error as the world as observed may change
 			return sreconcile.ResultEmpty, e
 		}
@@ -407,7 +410,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 				Err:    fmt.Errorf("failed to configure Helm client with secret data: %w", err),
 				Reason: sourcev1.AuthenticationFailedReason,
 			}
-			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, e.Err.Error())
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 			// Requeue as content of secret might change
 			return sreconcile.ResultEmpty, e
 		}
@@ -419,7 +422,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 				Err:    fmt.Errorf("failed to create TLS client config with secret data: %w", err),
 				Reason: sourcev1.AuthenticationFailedReason,
 			}
-			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, e.Err.Error())
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 			// Requeue as content of secret might change
 			return sreconcile.ResultEmpty, e
 		}
@@ -436,14 +439,14 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 				Err:    fmt.Errorf("invalid Helm repository URL: %w", err),
 				Reason: sourcev1.URLInvalidReason,
 			}
-			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.URLInvalidReason, e.Err.Error())
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 			return sreconcile.ResultEmpty, e
 		default:
 			e := &serror.Stalling{
 				Err:    fmt.Errorf("failed to construct Helm client: %w", err),
 				Reason: meta.FailedReason,
 			}
-			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, meta.FailedReason, e.Err.Error())
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 			return sreconcile.ResultEmpty, e
 		}
 	}
@@ -486,9 +489,9 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
 	if err != nil {
 		e := &serror.Event{
 			Err:    fmt.Errorf("failed to create temporary working directory: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.DirCreationFailedReason,
 		}
-		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedReason, e.Err.Error())
+		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 		return sreconcile.ResultEmpty, e
 	}
 	defer os.RemoveAll(tmpDir)
@@ -498,9 +501,9 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
 	if err := os.Mkdir(sourceDir, 0700); err != nil {
 		e := &serror.Event{
 			Err:    fmt.Errorf("failed to create directory to untar source into: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.DirCreationFailedReason,
 		}
-		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedReason, e.Err.Error())
+		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 		return sreconcile.ResultEmpty, e
 	}
 
@@ -509,9 +512,9 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
 	if err != nil {
 		e := &serror.Event{
 			Err:    fmt.Errorf("failed to open source artifact: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.ReadOperationFailedReason,
 		}
-		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedReason, e.Err.Error())
+		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 		return sreconcile.ResultEmpty, e
 	}
 	if _, err = untar.Untar(f, sourceDir); err != nil {
@@ -535,7 +538,7 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
 			Err:    fmt.Errorf("path calculation for chart '%s' failed: %w", obj.Spec.Chart, err),
 			Reason: "IllegalPath",
 		}
-		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, "IllegalPath", e.Err.Error())
+		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 		// We are unable to recover from this change without a change in generation
 		return sreconcile.ResultEmpty, e
 	}
@@ -639,26 +642,32 @@ func (r *HelmChartReconciler) reconcileArtifact(ctx context.Context, obj *source
 
 	// Ensure artifact directory exists and acquire lock
 	if err := r.Storage.MkdirAll(artifact); err != nil {
-		return sreconcile.ResultEmpty, &serror.Event{
+		e := &serror.Event{
 			Err:    fmt.Errorf("failed to create artifact directory: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.DirCreationFailedReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
+		return sreconcile.ResultEmpty, e
 	}
 	unlock, err := r.Storage.Lock(artifact)
 	if err != nil {
-		return sreconcile.ResultEmpty, &serror.Event{
+		e := &serror.Event{
 			Err:    fmt.Errorf("failed to acquire lock for artifact: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.AcquireLockFailedReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
+		return sreconcile.ResultEmpty, e
 	}
 	defer unlock()
 
 	// Copy the packaged chart to the artifact path
 	if err = r.Storage.CopyFromPath(&artifact, b.Path); err != nil {
-		return sreconcile.ResultEmpty, &serror.Event{
+		e := &serror.Event{
 			Err:    fmt.Errorf("unable to copy Helm chart to storage: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.ArchiveOperationFailedReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
+		return sreconcile.ResultEmpty, e
 	}
 
 	// Record it on the object
@@ -674,12 +683,13 @@ func (r *HelmChartReconciler) reconcileArtifact(ctx context.Context, obj *source
 	// Update symlink on a "best effort" basis
 	symURL, err := r.Storage.Symlink(artifact, "latest.tar.gz")
 	if err != nil {
-		r.eventLogf(ctx, obj, corev1.EventTypeWarning, sourcev1.StorageOperationFailedReason,
+		r.eventLogf(ctx, obj, events.EventTypeTrace, sourcev1.SymlinkUpdateFailedReason,
 			"failed to update status URL symlink: %s", err)
 	}
 	if symURL != "" {
 		obj.Status.URL = symURL
 	}
+	conditions.Delete(obj, sourcev1.StorageOperationFailedCondition)
 	return sreconcile.ResultSuccess, nil
 }
 

controllers/helmrepository_controller.go

@@ -59,6 +59,7 @@ var helmRepositoryReadyCondition = summarize.Conditions{
 	Owned: []string{
 		sourcev1.FetchFailedCondition,
 		sourcev1.ArtifactOutdatedCondition,
+		sourcev1.StorageOperationFailedCondition,
 		meta.ReadyCondition,
 		meta.ReconcilingCondition,
 		meta.StalledCondition,
@@ -66,12 +67,14 @@ var helmRepositoryReadyCondition = summarize.Conditions{
 	Summarize: []string{
 		sourcev1.FetchFailedCondition,
 		sourcev1.ArtifactOutdatedCondition,
+		sourcev1.StorageOperationFailedCondition,
 		meta.StalledCondition,
 		meta.ReconcilingCondition,
 	},
 	NegativePolarity: []string{
 		sourcev1.FetchFailedCondition,
 		sourcev1.ArtifactOutdatedCondition,
+		sourcev1.StorageOperationFailedCondition,
 		meta.StalledCondition,
 		meta.ReconcilingCondition,
 	},
@@ -289,7 +292,7 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
 				Err:    fmt.Errorf("failed to get secret '%s': %w", name.String(), err),
 				Reason: sourcev1.AuthenticationFailedReason,
 			}
-			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, e.Err.Error())
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 			return sreconcile.ResultEmpty, e
 		}
 
@@ -300,7 +303,7 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
 				Err:    fmt.Errorf("failed to configure Helm client with secret data: %w", err),
 				Reason: sourcev1.AuthenticationFailedReason,
 			}
-			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, e.Err.Error())
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 			// Return err as the content of the secret may change.
 			return sreconcile.ResultEmpty, e
 		}
@@ -312,7 +315,7 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
 				Err:    fmt.Errorf("failed to create TLS client config with secret data: %w", err),
 				Reason: sourcev1.AuthenticationFailedReason,
 			}
-			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, e.Err.Error())
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 			// Requeue as content of secret might change
 			return sreconcile.ResultEmpty, e
 		}
@@ -327,14 +330,14 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
 				Err:    fmt.Errorf("invalid Helm repository URL: %w", err),
 				Reason: sourcev1.URLInvalidReason,
 			}
-			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.URLInvalidReason, e.Err.Error())
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 			return sreconcile.ResultEmpty, e
 		default:
 			e := &serror.Stalling{
 				Err:    fmt.Errorf("failed to construct Helm client: %w", err),
 				Reason: meta.FailedReason,
 			}
-			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, meta.FailedReason, e.Err.Error())
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 			return sreconcile.ResultEmpty, e
 		}
 	}
@@ -344,7 +347,7 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
 			Err:    fmt.Errorf("failed to fetch Helm repository index: %w", err),
 			Reason: meta.FailedReason,
 		}
-		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, meta.FailedReason, e.Err.Error())
+		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 		// Coin flip on transient or persistent error, return error and hope for the best
 		return sreconcile.ResultEmpty, e
 	}
@@ -354,9 +357,9 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
 	if err := chartRepo.LoadFromCache(); err != nil {
 		e := &serror.Event{
 			Err:    fmt.Errorf("failed to load Helm repository from cache: %w", err),
-			Reason: sourcev1.FetchFailedCondition,
+			Reason: sourcev1.IndexationFailedReason,
 		}
-		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.IndexationFailedReason, e.Err.Error())
+		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
 		return sreconcile.ResultEmpty, e
 	}
 	defer chartRepo.Unload()
@@ -409,10 +412,12 @@ func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *s
 
 	// Create artifact dir
 	if err := r.Storage.MkdirAll(*artifact); err != nil {
-		return sreconcile.ResultEmpty, &serror.Event{
+		e := &serror.Event{
 			Err:    fmt.Errorf("failed to create artifact directory: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.DirCreationFailedReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
+		return sreconcile.ResultEmpty, e
 	}
 
 	// Acquire lock.
@@ -427,19 +432,23 @@ func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *s
 
 	// Save artifact to storage.
 	if err = r.Storage.CopyFromPath(artifact, chartRepo.CachePath); err != nil {
-		return sreconcile.ResultEmpty, &serror.Event{
+		e := &serror.Event{
 			Err:    fmt.Errorf("unable to save artifact to storage: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.ArchiveOperationFailedReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
+		return sreconcile.ResultEmpty, e
 	}
 
 	// Calculate the artifact size to be included in the NewArtifact event.
 	fi, err := os.Stat(chartRepo.CachePath)
 	if err != nil {
-		return sreconcile.ResultEmpty, &serror.Event{
+		e := &serror.Event{
 			Err:    fmt.Errorf("unable to read the artifact: %w", err),
-			Reason: sourcev1.StorageOperationFailedReason,
+			Reason: sourcev1.ReadOperationFailedReason,
 		}
+		conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
+		return sreconcile.ResultEmpty, e
 	}
 	size := units.HumanSize(float64(fi.Size()))
 
@@ -454,13 +463,13 @@ func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *s
 	// Update index symlink.
 	indexURL, err := r.Storage.Symlink(*artifact, "index.yaml")
 	if err != nil {
-		r.eventLogf(ctx, obj, corev1.EventTypeWarning, sourcev1.StorageOperationFailedReason,
+		r.eventLogf(ctx, obj, events.EventTypeTrace, sourcev1.SymlinkUpdateFailedReason,
 			"failed to update status URL symlink: %s", err)
 	}
 	if indexURL != "" {
 		obj.Status.URL = indexURL
 	}
-
+	conditions.Delete(obj, sourcev1.StorageOperationFailedCondition)
 	return sreconcile.ResultSuccess, nil
 }
 

docs/spec/v1beta2/buckets.md

@@ -903,17 +903,20 @@ without completing. This can occur due to some of the following factors:
   non-existing Secret.
 - The credentials in the referenced Secret are invalid.
 - The Bucket spec contains a generic misconfiguration.
+- A storage related failure when storing the artifact.
 
 When this happens, the controller sets the `Ready` Condition status to `False`,
 and adds a Condition with the following attributes to the Bucket's
 `.status.conditions`:
 
-- `type: FetchFailed`
+- `type: FetchFailed` | `type: StorageOperationFailed`
 - `status: "True"`
 - `reason: AuthenticationFailed` | `reason: BucketOperationFailed`
 
 This condition has a ["negative polarity"][typical-status-properties],
 and is only present on the Bucket while the status value is `"True"`.
+There may be more arbitrary values for the `reason` field to provide accurate
+reason for a condition.
 
 While the Bucket has this Condition, the controller will continue to attempt
 to produce an Artifact for the resource with an exponential backoff, until

docs/spec/v1beta2/gitrepositories.md

@@ -763,17 +763,20 @@ factors:
 - The verification of the Git commit signature failed.
 - The credentials in the referenced Secret are invalid.
 - The GitRepository spec contains a generic misconfiguration.
+- A storage related failure when storing the artifact.
 
 When this happens, the controller sets the `Ready` Condition status to `False`,
 and adds a Condition with the following attributes to the GitRepository's
 `.status.conditions`:
 
-- `type: FetchFailed` | `type: IncludeUnavailableCondition`
+- `type: FetchFailed` | `type: IncludeUnavailable` | `type: StorageOperationFailed`
 - `status: "True"`
-- `reason: AuthenticationFailed` | `reason: GitOperationFailed` | `reason: StorageOperationFailed`
+- `reason: AuthenticationFailed` | `reason: GitOperationFailed`
 
 This condition has a ["negative polarity"][typical-status-properties],
 and is only present on the GitRepository while the status value is `"True"`.
+There may be more arbitrary values for the `reason` field to provide accurate
+reason for a condition.
 
 In addition to the above Condition types, when the
 [verification of a Git commit signature](#verification) fails. A condition with

docs/spec/v1beta2/helmcharts.md

@@ -532,17 +532,20 @@ factors:
 - The credentials in the [Source reference](#source-reference) Secret are
   invalid.
 - The HelmChart spec contains a generic misconfiguration.
+- A storage related failure when storing the artifact.
 
 When this happens, the controller sets the `Ready` Condition status to `False`,
 and adds a Condition with the following attributes to the HelmChart's
 `.status.conditions`:
 
-- `type: FetchFailed`
+- `type: FetchFailed` | `type: StorageOperationFailed`
 - `status: "True"`
 - `reason: AuthenticationFailed` | `reason: StorageOperationFailed` | `reason: URLInvalid` | `reason: IllegalPath` | `reason: Failed`
 
 This condition has a ["negative polarity"][typical-status-properties],
 and is only present on the HelmChart while the status value is `"True"`.
+There may be more arbitrary values for the `reason` field to provide accurate
+reason for a condition.
 
 While the HelmChart has this Condition, the controller will continue to
 attempt to produce an Artifact for the resource with an exponential backoff,

docs/spec/v1beta2/helmrepositories.md

@@ -475,17 +475,20 @@ factors:
   non-existing Secret.
 - The credentials in the referenced Secret are invalid.
 - The HelmRepository spec contains a generic misconfiguration.
+- A storage related failure when storing the artifact.
 
 When this happens, the controller sets the `Ready` Condition status to `False`,
 and adds a Condition with the following attributes to the HelmRepository's
 `.status.conditions`:
 
-- `type: FetchFailed`
+- `type: FetchFailed` | `type: StorageOperationFailed`
 - `status: "True"`
 - `reason: AuthenticationFailed` | `reason: IndexationFailed` | `reason: Failed`
 
 This condition has a ["negative polarity"][typical-status-properties],
 and is only present on the HelmRepository while the status value is `"True"`.
+There may be more arbitrary values for the `reason` field to provide accurate
+reason for a condition.
 
 While the HelmRepository has this Condition, the controller will continue to
 attempt to produce an Artifact for the resource with an exponential backoff,