fluxcd
/
source-controller
mirror of https://github.com/fluxcd/source-controller.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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>
This commit is contained in:
Hidde Beydals 2021-08-09 13:24:49 +02:00
parent 588ccbfe99
commit 15bc9e71b1
7 changed files with 51 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
api/v1beta1/bucket_types.go
View File

@ -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').

16
api/v1beta1/condition_types.go
View File

@ -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 may be impossible, 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"

13
api/v1beta1/gitrepository_types.go
View File

@ -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.

28
controllers/bucket_controller.go
View File

@ -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,
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason,
"Failed to construct S3 client: %s", err.Error())
r.Eventf(ctx, obj, events.EventSeverityError, sourcev1.BucketOperationFailedReason,
"Failed to construct S3 client: %s", err.Error())
@ -299,11 +299,11 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, obj *sourcev1.Bu
defer cancel()
exists, err := s3Client.BucketExists(ctxTimeout, obj.Spec.BucketName)
if err != nil {
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,
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason,
"Bucket '%s' does not exist", obj.Spec.BucketName)
r.Eventf(ctx, obj, events.EventSeverityError, sourcev1.BucketOperationFailedReason,
"Bucket '%s' does not exist", obj.Spec.BucketName)
@ -314,7 +314,7 @@ 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,
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason,
"Failed to get '%s' file: %s", sourceignore.IgnoreFile, err.Error())
r.Eventf(ctx, obj, events.EventSeverityError, sourcev1.BucketOperationFailedReason,
"Failed to get '%s' file: %s", sourceignore.IgnoreFile, err.Error())
@ -323,7 +323,7 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, obj *sourcev1.Bu
}
ps, err := sourceignore.ReadIgnoreFile(path, nil)
if err != nil {
conditions.MarkTrue(obj, sourcev1.DownloadFailedCondition, sourcev1.BucketOperationFailedReason,
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason,
"Failed to read '%s' file: %s", sourceignore.IgnoreFile, err.Error())
r.Eventf(ctx, obj, events.EventSeverityError, sourcev1.BucketOperationFailedReason,
"Failed to read '%s' file: %s", sourceignore.IgnoreFile, err.Error())
@ -344,7 +344,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,
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason,
"Failed to list objects from bucket '%s': %s", obj.Spec.BucketName, err.Error())
r.Eventf(ctx, obj, events.EventSeverityError, sourcev1.BucketOperationFailedReason,
"Failed to list objects from bucket '%s': %s", obj.Spec.BucketName, err.Error())
@ -396,7 +396,7 @@ 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)
r.Eventf(ctx, obj, events.EventSeverityError, sourcev1.BucketOperationFailedReason,
"Download from bucket '%s' failed: %s", obj.Spec.BucketName, err)
@ -405,7 +405,7 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, obj *sourcev1.Bu
r.Eventf(ctx, obj, events.EventSeverityInfo, sourcev1.BucketOperationSucceedReason,
"Downloaded %d files from bucket '%s' revision '%s'", len(index), obj.Spec.BucketName, revision)
}
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))

8
controllers/bucket_controller_test.go
View File

@ -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'"),
},
},
{

22
controllers/gitrepository_controller.go
View File

@ -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() {

12
controllers/gitrepository_controller_test.go
View File

@ -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 '<url>', error: Certificate"),
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, "Failed to checkout and determine revision: unable to clone '<url>', 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{