Merge pull request #24 from fluxcd/imgrepo-ready

Implement Ready condition for image repos

api/v1alpha1/condition_types.go

@@ -0,0 +1,70 @@
+/*
+Copyright 2020 The Flux CD contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1alpha1
+
+import (
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// Condition contains condition information for a toolkit resource.
+type Condition struct {
+	// Type of the condition, currently ('Ready').
+	// +required
+	Type string `json:"type"`
+
+	// Status of the condition, one of ('True', 'False', 'Unknown').
+	// +required
+	Status corev1.ConditionStatus `json:"status"`
+
+	// LastTransitionTime is the timestamp corresponding to the last status
+	// change of this condition.
+	// +required
+	LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"`
+
+	// Reason is a brief machine readable explanation for the condition's last
+	// transition.
+	// +required
+	Reason string `json:"reason,omitempty"`
+
+	// Message is a human readable description of the details of the last
+	// transition, complementing reason.
+	// +optional
+	Message string `json:"message,omitempty"`
+}
+
+const (
+	// ReadyCondition records the last reconciliation result.
+	ReadyCondition string = "Ready"
+)
+
+const (
+	// ReconciliationSucceededReason represents the fact that the reconciliation of the resource has succeeded.
+	ReconciliationSucceededReason string = "ReconciliationSucceeded"
+
+	// ReconciliationFailedReason represents the fact that the reconciliation of the resource has failed.
+	ReconciliationFailedReason string = "ReconciliationFailed"
+
+	// ImageURLInvalidReason represents the fact that a given repository has an invalid image URL.
+	ImageURLInvalidReason string = "ImageURLInvalid"
+
+	// ProgressingReason represents the fact that a reconciliation is underway.
+	ProgressingReason string = "Progressing"
+
+	// SuspendedReason represents the fact that the reconciliation is suspended.
+	SuspendedReason string = "Suspended"
+)

api/v1alpha1/imagerepository_types.go

@@ -17,6 +17,7 @@ limitations under the License.
 package v1alpha1
 
 import (
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -40,20 +41,49 @@ type ScanResult struct {
 
 // ImageRepositoryStatus defines the observed state of ImageRepository
 type ImageRepositoryStatus struct {
+	// +optional
+	Conditions []Condition `json:"conditions,omitempty"`
+
+	// ObservedGeneration is the last reconciled generation.
+	// +optional
+	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
+
 	// CannonicalName is the name of the image repository with all the
 	// implied bits made explicit; e.g., `docker.io/library/alpine`
 	// rather than `alpine`.
-	CanonicalImageName string `json:"canonicalImageName,omitempty"`
-	// LastError is the error from last reconciliation, or empty if
-	// reconciliation was successful.
-	LastError string `json:"lastError"`
-	// LastScanTime records the last time the repository was
-	// successfully scanned.
 	// +optional
-	LastScanTime   *metav1.Time `json:"lastScanTime,omitempty"`
+	CanonicalImageName string `json:"canonicalImageName,omitempty"`
+
+	// LastScanResult contains the number of fetched tags.
+	// +optional
 	LastScanResult ScanResult `json:"lastScanResult,omitempty"`
 }
 
+// SetImageRepositoryReadiness sets the ready condition with the given status, reason and message.
+func SetImageRepositoryReadiness(ir ImageRepository, status corev1.ConditionStatus, reason, message string) ImageRepository {
+	ir.Status.Conditions = []Condition{
+		{
+			Type:               ReadyCondition,
+			Status:             status,
+			LastTransitionTime: metav1.Now(),
+			Reason:             reason,
+			Message:            message,
+		},
+	}
+	ir.Status.ObservedGeneration = ir.ObjectMeta.Generation
+	return ir
+}
+
+func GetLastTransitionTime(ir ImageRepository) *metav1.Time {
+	for _, condition := range ir.Status.Conditions {
+		if condition.Type == ReadyCondition {
+			return &condition.LastTransitionTime
+		}
+	}
+
+	return nil
+}
+
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 // +kubebuilder:printcolumn:name="Last scan",type=string,JSONPath=`.status.lastScanTime`

api/v1alpha1/zz_generated.deepcopy.go

@@ -25,6 +25,22 @@ import (
 	runtime "k8s.io/apimachinery/pkg/runtime"
 )
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *Condition) DeepCopyInto(out *Condition) {
+	*out = *in
+	in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime)
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Condition.
+func (in *Condition) DeepCopy() *Condition {
+	if in == nil {
+		return nil
+	}
+	out := new(Condition)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ImagePolicy) DeepCopyInto(out *ImagePolicy) {
 	*out = *in
@@ -218,9 +234,12 @@ func (in *ImageRepositorySpec) DeepCopy() *ImageRepositorySpec {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ImageRepositoryStatus) DeepCopyInto(out *ImageRepositoryStatus) {
 	*out = *in
-	if in.LastScanTime != nil {
-		in, out := &in.LastScanTime, &out.LastScanTime
-		*out = (*in).DeepCopy()
+	if in.Conditions != nil {
+		in, out := &in.Conditions, &out.Conditions
+		*out = make([]Condition, len(*in))
+		for i := range *in {
+			(*in)[i].DeepCopyInto(&(*out)[i])
+		}
 	}
 	out.LastScanResult = in.LastScanResult
 }

config/crd/bases/image.toolkit.fluxcd.io_imagerepositories.yaml

@@ -60,24 +60,48 @@ spec:
                   all the implied bits made explicit; e.g., `docker.io/library/alpine`
                   rather than `alpine`.
                 type: string
-              lastError:
-                description: LastError is the error from last reconciliation, or empty
-                  if reconciliation was successful.
+              conditions:
+                items:
+                  description: Condition contains condition information for a toolkit
+                    resource.
+                  properties:
+                    lastTransitionTime:
+                      description: LastTransitionTime is the timestamp corresponding
+                        to the last status change of this condition.
+                      format: date-time
                       type: string
+                    message:
+                      description: Message is a human readable description of the
+                        details of the last transition, complementing reason.
+                      type: string
+                    reason:
+                      description: Reason is a brief machine readable explanation
+                        for the condition's last transition.
+                      type: string
+                    status:
+                      description: Status of the condition, one of ('True', 'False',
+                        'Unknown').
+                      type: string
+                    type:
+                      description: Type of the condition, currently ('Ready').
+                      type: string
+                  required:
+                  - status
+                  - type
+                  type: object
+                type: array
               lastScanResult:
+                description: LastScanResult contains the number of fetched tags.
                 properties:
                   tagCount:
                     type: integer
                 required:
                 - tagCount
                 type: object
-              lastScanTime:
-                description: LastScanTime records the last time the repository was
-                  successfully scanned.
-                format: date-time
-                type: string
-            required:
-            - lastError
+              observedGeneration:
+                description: ObservedGeneration is the last reconciled generation.
+                format: int64
+                type: integer
             type: object
         type: object
     served: true

controllers/imagerepository_controller.go

@@ -18,13 +18,14 @@ package controllers
 
 import (
 	"context"
+	"fmt"
 	"strings"
 	"time"
 
 	"github.com/go-logr/logr"
 	"github.com/google/go-containerregistry/pkg/name"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	kuberecorder "k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -63,8 +64,14 @@ type ImageRepositoryReconciler struct {
 func (r *ImageRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	ctx := context.Background()
 
+	// NB: In general, if an error is returned then controller-runtime
+	// will requeue the request with back-off. In the following this
+	// is usually made explicit by _also_ returning
+	// `ctrl.Result{Requeue: true}`.
+
 	var imageRepo imagev1alpha1.ImageRepository
 	if err := r.Get(ctx, req.NamespacedName, &imageRepo); err != nil {
+		// _Might_ get requeued
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
@@ -72,55 +79,85 @@ func (r *ImageRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, er
 
 	ref, err := name.ParseReference(imageRepo.Spec.Image)
 	if err != nil {
-		imageRepo.Status.LastError = err.Error()
-		if err := r.Status().Update(ctx, &imageRepo); err != nil {
-			return ctrl.Result{}, err
+		status := imagev1alpha1.SetImageRepositoryReadiness(
+			imageRepo,
+			corev1.ConditionFalse,
+			imagev1alpha1.ImageURLInvalidReason,
+			err.Error(),
+		)
+		if err := r.Status().Update(ctx, &status); err != nil {
+			return ctrl.Result{Requeue: true}, err
 		}
 		log.Error(err, "Unable to parse image name", "imageName", imageRepo.Spec.Image)
-		return ctrl.Result{}, nil
+		return ctrl.Result{Requeue: true}, err
 	}
 
-	canonicalName := ref.Context().String()
-	imageRepo.Status.CanonicalImageName = canonicalName
+	imageRepo.Status.CanonicalImageName = ref.Context().String()
 
 	now := time.Now()
-	ok, when := r.shouldScan(&imageRepo, now)
+	ok, when := r.shouldScan(imageRepo, now)
 	if ok {
 		ctx, cancel := context.WithTimeout(ctx, scanTimeout)
 		defer cancel()
-		tags, err := remote.ListWithContext(ctx, ref.Context()) // TODO: auth
-		if err != nil {
-			imageRepo.Status.LastError = err.Error()
-			if err = r.Status().Update(ctx, &imageRepo); err != nil {
-				return ctrl.Result{}, err
+
+		reconciledRepo, reconcileErr := r.scan(ctx, imageRepo, ref)
+		if err = r.Status().Update(ctx, &reconciledRepo); err != nil {
+			return ctrl.Result{Requeue: true}, err
+		}
+
+		if reconcileErr != nil {
+			return ctrl.Result{Requeue: true}, reconcileErr
+		} else {
+			log.Info(fmt.Sprintf("reconciliation finished in %s, next run in %s",
+				time.Now().Sub(now).String(),
+				when),
+			)
 		}
 	}
 
-		imageRepo.Status.LastScanTime = &metav1.Time{Time: now}
-		imageRepo.Status.LastScanResult.TagCount = len(tags)
-		imageRepo.Status.LastError = ""
-		// share the information in the database
-		r.Database.SetTags(canonicalName, tags)
-		log.Info("successful scan", "tag count", len(tags))
-		if err = r.Status().Update(ctx, &imageRepo); err != nil {
-			return ctrl.Result{}, err
-		}
-	}
 	return ctrl.Result{RequeueAfter: when}, nil
 }
 
+func (r *ImageRepositoryReconciler) scan(ctx context.Context, imageRepo imagev1alpha1.ImageRepository, ref name.Reference) (imagev1alpha1.ImageRepository, error) {
+	canonicalName := ref.Context().String()
+
+	// TODO: implement auth
+	tags, err := remote.ListWithContext(ctx, ref.Context())
+	if err != nil {
+		return imagev1alpha1.SetImageRepositoryReadiness(
+			imageRepo,
+			corev1.ConditionFalse,
+			imagev1alpha1.ReconciliationFailedReason,
+			err.Error(),
+		), err
+	}
+
+	// TODO: add context and error handling to database ops
+	r.Database.SetTags(canonicalName, tags)
+
+	imageRepo.Status.LastScanResult.TagCount = len(tags)
+	return imagev1alpha1.SetImageRepositoryReadiness(
+		imageRepo,
+		corev1.ConditionTrue,
+		imagev1alpha1.ReconciliationSucceededReason,
+		fmt.Sprintf("successful scan, found %v tags", len(tags)),
+	), nil
+}
+
 // shouldScan takes an image repo and the time now, and says whether
 // the repository should be scanned now, and how long to wait for the
 // next scan.
-func (r *ImageRepositoryReconciler) shouldScan(repo *imagev1alpha1.ImageRepository, now time.Time) (bool, time.Duration) {
+func (r *ImageRepositoryReconciler) shouldScan(repo imagev1alpha1.ImageRepository, now time.Time) (bool, time.Duration) {
 	scanInterval := defaultScanInterval
 	if repo.Spec.ScanInterval != nil {
 		scanInterval = repo.Spec.ScanInterval.Duration
 	}
 
-	if repo.Status.LastScanTime == nil {
+	lastTransitionTime := imagev1alpha1.GetLastTransitionTime(repo)
+	if lastTransitionTime == nil {
 		return true, scanInterval
 	}
+
 	// when recovering, it's possible that the resource has a last
 	// scan time, but there's no records because the database has been
 	// dropped and created again.
@@ -132,7 +169,7 @@ func (r *ImageRepositoryReconciler) shouldScan(repo *imagev1alpha1.ImageReposito
 		return true, scanInterval
 	}
 
-	when := scanInterval - now.Sub(repo.Status.LastScanTime.Time)
+	when := scanInterval - now.Sub(lastTransitionTime.Time)
 	if when < time.Second {
 		return true, scanInterval
 	}