Consolidate condition types into FetchFailed

This commit consolidates the `DownloadFailed` and `CheckoutFailed`
Condition types into a new more generic `FetchFailed` type to simplify
the API and observations by consumers.

Signed-off-by: Hidde Beydals <hello@hidde.co>

api/v1beta1/bucket_types.go

@@ -33,13 +33,6 @@ const (
  AmazonBucketProvider string = "aws"
 )
 
-const (
- // DownloadFailedCondition indicates a transient or persistent download failure. If True, observations on the
- // upstream Source revision are not possible, and the Artifact available for the Source may be outdated.
- // This is a "negative polarity" or "abnormal-true" type, and is only present on the resource if it is True.
- DownloadFailedCondition string = "DownloadFailed"
-)
-
 // BucketSpec defines the desired state of an S3 compatible bucket
 type BucketSpec struct {
  // The S3 compatible storage provider name, default ('generic').

api/v1beta1/condition_types.go

@@ -18,6 +18,22 @@ package v1beta1
 
 const SourceFinalizer = "finalizers.fluxcd.io"
 
+const (
+ // ArtifactUnavailableCondition indicates there is no Artifact available for the Source.
+ // This is a "negative polarity" or "abnormal-true" type, and is only present on the resource if it is True.
+ ArtifactUnavailableCondition string = "ArtifactUnavailable"
+
+ // ArtifactOutdatedCondition indicates the current Artifact of the Source is outdated.
+ // This is a "negative polarity" or "abnormal-true" type, and is only present on the resource if it is True.
+ ArtifactOutdatedCondition string = "ArtifactOutdated"
+
+ // FetchFailedCondition indicates a transient or persistent fetch failure of an upstream Source.
+ // If True, observations on the upstream Source revision are not possible, and the Artifact available for the Source
+ // may be outdated.
+ // This is a "negative polarity" or "abnormal-true" type, and is only present on the resource if it is True.
+ FetchFailedCondition string = "FetchFailed"
+)
+
 const (
  // URLInvalidReason represents the fact that a given source has an invalid URL.
  URLInvalidReason string = "URLInvalid"

api/v1beta1/gitrepository_types.go

@@ -34,15 +34,6 @@ const (
 )
 
 const (
- // ArtifactUnavailableCondition indicates there is no Artifact available for the Source.
- // This is a "negative polarity" or "abnormal-true" type, and is only present on the resource if it is True.
- ArtifactUnavailableCondition string = "ArtifactUnavailable"
-
- // CheckoutFailedCondition indicates a transient or persistent checkout failure. If True, observations on the
- // upstream Source revision are not possible, and the Artifact available for the Source may be outdated.
- // This is a "negative polarity" or "abnormal-true" type, and is only present on the resource if it is True.
- CheckoutFailedCondition string = "CheckoutFailed"
-
  // SourceVerifiedCondition indicates the integrity of the Source has been verified. If True, the integrity check
  // succeeded. If False, it failed. The Condition is only present on the resource if the integrity has been verified.
  SourceVerifiedCondition string = "SourceVerified"
@@ -51,10 +42,6 @@ const (
  // exist, or does not have an Artifact.
  // This is a "negative polarity" or "abnormal-true" type, and is only present on the resource if it is True.
  IncludeUnavailableCondition string = "IncludeUnavailable"
-
- // ArtifactOutdatedCondition indicates the current Artifact of the Source is outdated.
- // This is a "negative polarity" or "abnormal-true" type, and is only present on the resource if it is True.
- ArtifactOutdatedCondition string = "ArtifactOutdated"
 )
 
 // GitRepositorySpec defines the desired state of a Git repository.

controllers/bucket_controller.go

@@ -119,12 +119,12 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
  meta.ReadyCondition,
  conditions.WithConditions(
  sourcev1.ArtifactOutdatedCondition,
- sourcev1.DownloadFailedCondition,
+ sourcev1.FetchFailedCondition,
  sourcev1.ArtifactUnavailableCondition,
  ),
  conditions.WithNegativePolarityConditions(
  sourcev1.ArtifactOutdatedCondition,
- sourcev1.DownloadFailedCondition,
+ sourcev1.FetchFailedCondition,
  sourcev1.ArtifactUnavailableCondition,
  ),
  )
@@ -134,7 +134,7 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
  patch.WithOwnedConditions{
  Conditions: []string{
  sourcev1.ArtifactOutdatedCondition,
- sourcev1.DownloadFailedCondition,
+ sourcev1.FetchFailedCondition,
  sourcev1.ArtifactUnavailableCondition,
  meta.ReadyCondition,
  meta.ReconcilingCondition,
@@ -258,8 +258,8 @@ func (r *BucketReconciler) reconcileStorage(ctx context.Context, obj *sourcev1.B
 //
 // The bucket contents are downloaded to the given dir using the defined configuration, while taking ignore rules into
 // account. In case of an error during the download process (including transient errors), it records
-// v1beta1.DownloadFailedCondition=True and returns early.
-// On a successful download, it removes v1beta1.DownloadFailedCondition, and compares the current revision of HEAD to
+// v1beta1.FetchFailedCondition=True and returns early.
+// On a successful download, it removes v1beta1.FetchFailedCondition, and compares the current revision of HEAD to
 // the artifact on the object, and records v1beta1.ArtifactOutdatedCondition if they differ.
 // If the download was successful, the given artifact pointer is set to a new artifact with the available metadata.
 //
@@ -274,7 +274,7 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, obj *sourcev1.Bu
  Name: obj.Spec.SecretRef.Name,
  }
  if err := r.Client.Get(ctx, name, secret); err != nil {
- conditions.MarkTrue(obj, sourcev1.DownloadFailedCondition, sourcev1.AuthenticationFailedReason,
+ conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason,
  "Failed to get secret '%s': %s", name.String(), err.Error())
  r.Eventf(ctx, obj, events.EventSeverityError, sourcev1.AuthenticationFailedReason,
  "Failed to get secret '%s': %s", name.String(), err.Error())
@@ -286,7 +286,7 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, obj *sourcev1.Bu
  // Build the client with the configuration from the object and secret
  s3Client, err := r.buildClient(obj, secret)
  if err != nil {
- conditions.MarkTrue(obj, sourcev1.DownloadFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to construct S3 client: %s", err.Error())
+ conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to construct S3 client: %s", err.Error())
  // Return error as the contents of the secret may change
  return ctrl.Result{}, err
  }
@@ -297,25 +297,25 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, obj *sourcev1.Bu
  exists, err := s3Client.BucketExists(ctxTimeout, obj.Spec.BucketName)
  if err != nil {
  // Error may be transient
- conditions.MarkTrue(obj, sourcev1.DownloadFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to verify existence of bucket '%s': %s", obj.Spec.BucketName, err.Error())
+ conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to verify existence of bucket '%s': %s", obj.Spec.BucketName, err.Error())
  return ctrl.Result{}, err
  }
  if !exists {
- conditions.MarkTrue(obj, sourcev1.DownloadFailedCondition, sourcev1.BucketOperationFailedReason, "Bucket '%s' does not exist", obj.Spec.BucketName)
+ conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason, "Bucket '%s' does not exist", obj.Spec.BucketName)
  return ctrl.Result{}, fmt.Errorf("bucket '%s' does not exist", obj.Spec.BucketName)
  }
 
  // Look for file with ignore rules first
  path := filepath.Join(dir, sourceignore.IgnoreFile)
  if err := s3Client.FGetObject(ctxTimeout, obj.Spec.BucketName, sourceignore.IgnoreFile, path, minio.GetObjectOptions{}); err != nil {
  if resp, ok := err.(minio.ErrorResponse); ok && resp.Code != "NoSuchKey" {
- conditions.MarkTrue(obj, sourcev1.DownloadFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to get '%s' file: %s", sourceignore.IgnoreFile, err.Error())
+ conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to get '%s' file: %s", sourceignore.IgnoreFile, err.Error())
  return ctrl.Result{}, err
  }
  }
  ps, err := sourceignore.ReadIgnoreFile(path, nil)
  if err != nil {
- conditions.MarkTrue(obj, sourcev1.DownloadFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to read '%s' file: %s", sourceignore.IgnoreFile, err.Error())
+ conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to read '%s' file: %s", sourceignore.IgnoreFile, err.Error())
  return ctrl.Result{}, err
  }
  // In-spec patterns take precedence
@@ -333,7 +333,7 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, obj *sourcev1.Bu
  UseV1: s3utils.IsGoogleEndpoint(*s3Client.EndpointURL()),
  }) {
  if err = object.Err; err != nil {
- conditions.MarkTrue(obj, sourcev1.DownloadFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to list objects from bucket '%s': %s", obj.Spec.BucketName, err.Error())
+ conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to list objects from bucket '%s': %s", obj.Spec.BucketName, err.Error())
  return ctrl.Result{}, err
  }
 
@@ -351,7 +351,7 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, obj *sourcev1.Bu
  // Calculate revision checksum from the collected index values
  revision, err := r.revision(index)
  if err != nil {
- conditions.MarkTrue(obj, sourcev1.DownloadFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to calculate revision: %s", err.Error())
+ conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to calculate revision: %s", err.Error())
  return ctrl.Result{}, err
  }
 
@@ -381,12 +381,12 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, obj *sourcev1.Bu
  return nil
  })
  if err = group.Wait(); err != nil {
- conditions.MarkTrue(obj, sourcev1.DownloadFailedCondition, sourcev1.BucketOperationFailedReason,
+ conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason,
  "Download from bucket '%s' failed: %s", obj.Spec.BucketName, err)
  return ctrl.Result{}, err
  }
  }
- conditions.Delete(obj, sourcev1.DownloadFailedCondition)
+ conditions.Delete(obj, sourcev1.FetchFailedCondition)
 
  // Create potential new artifact
  *artifact = r.Storage.NewArtifactFor(obj.Kind, obj, revision, fmt.Sprintf("%s.tar.gz", revision))

controllers/bucket_controller_test.go

@@ -305,7 +305,7 @@ func TestBucketReconciler_reconcileSource(t *testing.T) {
  },
  wantErr: true,
  assertConditions: []metav1.Condition{
- *conditions.TrueCondition(sourcev1.DownloadFailedCondition, sourcev1.AuthenticationFailedReason, "Failed to get secret '/dummy': secrets \"dummy\" not found"),
+ *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "Failed to get secret '/dummy': secrets \"dummy\" not found"),
  },
  },
  {
@@ -323,7 +323,7 @@ func TestBucketReconciler_reconcileSource(t *testing.T) {
  },
  wantErr: true,
  assertConditions: []metav1.Condition{
- *conditions.TrueCondition(sourcev1.DownloadFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to construct S3 client: invalid 'dummy' secret data: required fields"),
+ *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to construct S3 client: invalid 'dummy' secret data: required fields"),
  },
  },
  {
@@ -334,7 +334,7 @@ func TestBucketReconciler_reconcileSource(t *testing.T) {
  },
  wantErr: true,
  assertConditions: []metav1.Condition{
- *conditions.TrueCondition(sourcev1.DownloadFailedCondition, sourcev1.BucketOperationFailedReason, "Bucket 'invalid' does not exist"),
+ *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason, "Bucket 'invalid' does not exist"),
  },
  },
  {
@@ -345,7 +345,7 @@ func TestBucketReconciler_reconcileSource(t *testing.T) {
  },
  wantErr: true,
  assertConditions: []metav1.Condition{
- *conditions.TrueCondition(sourcev1.DownloadFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to verify existence of bucket 'unavailable'"),
+ *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason, "Failed to verify existence of bucket 'unavailable'"),
  },
  },
  {

controllers/gitrepository_controller.go

@@ -122,13 +122,13 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
  conditions.WithConditions(
  sourcev1.IncludeUnavailableCondition,
  sourcev1.SourceVerifiedCondition,
- sourcev1.CheckoutFailedCondition,
+ sourcev1.FetchFailedCondition,
  sourcev1.ArtifactOutdatedCondition,
  sourcev1.ArtifactUnavailableCondition,
  ),
  conditions.WithNegativePolarityConditions(
  sourcev1.ArtifactUnavailableCondition,
- sourcev1.CheckoutFailedCondition,
+ sourcev1.FetchFailedCondition,
  sourcev1.SourceVerifiedCondition,
  sourcev1.IncludeUnavailableCondition,
  sourcev1.ArtifactOutdatedCondition,
@@ -140,7 +140,7 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
  patch.WithOwnedConditions{
  Conditions: []string{
  sourcev1.ArtifactUnavailableCondition,
- sourcev1.CheckoutFailedCondition,
+ sourcev1.FetchFailedCondition,
  sourcev1.IncludeUnavailableCondition,
  sourcev1.ArtifactOutdatedCondition,
  meta.ReadyCondition,
@@ -271,8 +271,8 @@ func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context, obj *sou
 // and observes its state.
 //
 // The repository is checked out to the given dir using the defined configuration, and in case of an error during the
-// checkout process (including transient errors), it records v1beta1.CheckoutFailedCondition=True and returns early.
-// On a successful checkout it removes v1beta1.CheckoutFailedCondition, and compares the current revision of HEAD to the
+// checkout process (including transient errors), it records v1beta1.FetchFailedCondition=True and returns early.
+// On a successful checkout it removes v1beta1.FetchFailedCondition, and compares the current revision of HEAD to the
 // artifact on the object, and records v1beta1.ArtifactOutdatedCondition if they differ.
 // If instructed, the signature of the commit is verified if and recorded as v1beta1.SourceVerifiedCondition. If the
 // signature can not be verified or the verification fails, the Condition=False and it returns early.
@@ -292,7 +292,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
  })
  if err != nil {
  ctrl.LoggerFrom(ctx).Error(err, fmt.Sprintf("Failed to get auth strategy for Git implementation '%s'", obj.Spec.GitImplementation))
- conditions.MarkTrue(obj, sourcev1.CheckoutFailedCondition, sourcev1.AuthenticationFailedReason,
+ conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason,
  "Failed to get auth strategy for Git implementation '%s': %s", obj.Spec.GitImplementation, err)
  // Do not return error as recovery without changes is impossible
  return ctrl.Result{}, nil
@@ -305,7 +305,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
  }
  var secret corev1.Secret
  if err = r.Client.Get(ctx, name, &secret); err != nil {
- conditions.MarkTrue(obj, sourcev1.CheckoutFailedCondition, sourcev1.AuthenticationFailedReason,
+ conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason,
  "Failed to get secret '%s': %s", name.String(), err.Error())
  r.Eventf(ctx, obj, events.EventSeverityError, sourcev1.AuthenticationFailedReason,
  "Failed to get secret '%s': %s", name.String(), err.Error())
@@ -316,7 +316,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
  // Configure strategy with secret
  auth, err = authStrategy.Method(secret)
  if err != nil {
- conditions.MarkTrue(obj, sourcev1.CheckoutFailedCondition, sourcev1.AuthenticationFailedReason,
+ conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason,
  "Failed to configure auth strategy for Git implementation '%s': %s", obj.Spec.GitImplementation, err)
  r.Eventf(ctx, obj, events.EventSeverityError, sourcev1.AuthenticationFailedReason,
  "Failed to configure auth strategy for Git implementation '%s': %s", obj.Spec.GitImplementation, err)
@@ -332,7 +332,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
  })
  if err != nil {
  ctrl.LoggerFrom(ctx).Error(err, fmt.Sprintf("Failed to configure checkout strategy for Git implementation '%s'", obj.Spec.GitImplementation))
- conditions.MarkTrue(obj, sourcev1.CheckoutFailedCondition, sourcev1.GitOperationFailedReason,
+ conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason,
  "Failed to configure checkout strategy for Git implementation '%s': %s", obj.Spec.GitImplementation, err)
  // Do not return err as recovery without changes is impossible
  return ctrl.Result{}, nil
@@ -343,7 +343,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
  defer cancel()
  commit, revision, err := checkoutStrategy.Checkout(gitCtx, dir, obj.Spec.URL, auth)
  if err != nil {
- conditions.MarkTrue(obj, sourcev1.CheckoutFailedCondition, sourcev1.GitOperationFailedReason,
+ conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason,
  "Failed to checkout and determine revision: %s", err)
  r.Eventf(ctx, obj, events.EventSeverityError, sourcev1.GitOperationFailedReason,
  "Failed to checkout and determine revision: %s", err)
@@ -352,7 +352,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
  }
  r.Eventf(ctx, obj, events.EventSeverityInfo, sourcev1.GitOperationSucceedReason,
  "Cloned repository '%s' and checked out revision '%s'", obj.Spec.URL, revision)
- conditions.Delete(obj, sourcev1.CheckoutFailedCondition)
+ conditions.Delete(obj, sourcev1.FetchFailedCondition)
 
  // Verify commit signature
  if result, err := r.verifyCommitSignature(ctx, obj, commit); err != nil || result.IsZero() {