diff --git a/api/v1beta1/bucket_types.go b/api/v1beta1/bucket_types.go index 2b0a1ea4f..d4a5c567d 100644 --- a/api/v1beta1/bucket_types.go +++ b/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'). diff --git a/api/v1beta1/condition_types.go b/api/v1beta1/condition_types.go index 4077a2ab6..a8f786ff6 100644 --- a/api/v1beta1/condition_types.go +++ b/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" diff --git a/api/v1beta1/gitrepository_types.go b/api/v1beta1/gitrepository_types.go index 9a232d634..3a1c71d0f 100644 --- a/api/v1beta1/gitrepository_types.go +++ b/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. diff --git a/controllers/bucket_controller.go b/controllers/bucket_controller.go index 8f5cde486..2f8a8d0fa 100644 --- a/controllers/bucket_controller.go +++ b/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,11 +297,11 @@ 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) } @@ -309,13 +309,13 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, obj *sourcev1.Bu 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)) diff --git a/controllers/bucket_controller_test.go b/controllers/bucket_controller_test.go index 60fe1bec7..75d5fb972 100644 --- a/controllers/bucket_controller_test.go +++ b/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'"), }, }, { diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index 7ba49dad3..baa48188b 100644 --- a/controllers/gitrepository_controller.go +++ b/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() { diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index 704718b58..903f58e8b 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -220,7 +220,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) { }, wantErr: true, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(sourcev1.CheckoutFailedCondition, sourcev1.GitOperationFailedReason, "x509: certificate signed by unknown authority"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, "x509: certificate signed by unknown authority"), }, }, { @@ -245,7 +245,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) { }, wantErr: true, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(sourcev1.CheckoutFailedCondition, sourcev1.GitOperationFailedReason, "Failed to checkout and determine revision: unable to clone '', error: Certificate"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, "Failed to checkout and determine revision: unable to clone '', error: Certificate"), }, }, { @@ -306,7 +306,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) { }, wantErr: true, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(sourcev1.CheckoutFailedCondition, "AuthenticationFailed", "Failed to get secret '/non-existing': secrets \"non-existing\" not found"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "Failed to get secret '/non-existing': secrets \"non-existing\" not found"), }, }, } @@ -1145,7 +1145,7 @@ func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) { { name: "mixed failed conditions", beforeFunc: func(obj *sourcev1.GitRepository) { - conditions.MarkTrue(obj, sourcev1.CheckoutFailedCondition, "Foo", "") + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, "Foo", "") conditions.MarkTrue(obj, sourcev1.IncludeUnavailableCondition, "Foo", "") conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, "Foo", "") conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "Foo", "") @@ -1160,7 +1160,7 @@ func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) { name: "reconciling and failed conditions", beforeFunc: func(obj *sourcev1.GitRepository) { conditions.MarkTrue(obj, meta.ReconcilingCondition, "Foo", "") - conditions.MarkTrue(obj, sourcev1.CheckoutFailedCondition, "Foo", "") + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, "Foo", "") }, want: ctrl.Result{RequeueAfter: interval}, assertConditions: []metav1.Condition{ @@ -1171,7 +1171,7 @@ func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) { name: "stalled and failed conditions", beforeFunc: func(obj *sourcev1.GitRepository) { conditions.MarkTrue(obj, meta.StalledCondition, "Foo", "") - conditions.MarkTrue(obj, sourcev1.CheckoutFailedCondition, "Foo", "") + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, "Foo", "") }, want: ctrl.Result{RequeueAfter: interval}, assertConditions: []metav1.Condition{