Skip to content

Commit

Permalink
Refactor and align with stable components patterns
Browse files Browse the repository at this point in the history 
This is an attempt to bring the api and controller logic closer to what
the other controller components already have set as patterns.

1. Adopt the k8s standard Condition type.
2. Rename `ScanInterval` to `Interval` to be consistent with the
   `Interval` attribute other Spec types have defined, translating to
   reconciliation interval. This attribute is now required.
3. Add `ScanTime` attribute to the `ScanResult` type, enabling keeping
   track of the last successful scan execution. Use this value for scan
   frequency throttling.
4. Add optional `Timeout` attribute to allow custom scan timeout
   handling. The default value is equal to that of the `Interval` attr.

Signed-off-by: Aurel Canciu <aurelcanciu@gmail.com>
  • Loading branch information
@relu
relu committed Nov 23, 2020
1 parent 85f75a0 commit c20dc7c
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 88 deletions.
41 changes: 25 additions & 16 deletions api/v1alpha1/imagerepository_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ limitations under the License.
package v1alpha1

import (
"time"

corev1 "k8s.io/api/core/v1"
apimeta "k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/fluxcd/pkg/apis/meta"
Expand All @@ -32,10 +33,15 @@ type ImageRepositorySpec struct {
// Image is the name of the image repository
// +required
Image string `json:"image,omitempty"`
// ScanInterval is the (minimum) length of time to wait between
// Interval is the length of time to wait between
// scans of the image repository.
// +required
Interval metav1.Duration `json:"interval,omitempty"`

// Timeout for image scanning.
// Defaults to 'Interval' duration.
// +optional
ScanInterval *metav1.Duration `json:"scanInterval,omitempty"`
Timeout *metav1.Duration `json:"timeout,omitempty"`

// SecretRef can be given the name of a secret containing
// credentials to use for the image registry. The secret should be
Expand All @@ -50,7 +56,8 @@ type ImageRepositorySpec struct {
}

type ScanResult struct {
TagCount int `json:"tagCount"`
TagCount int `json:"tagCount"`
ScanTime *metav1.Time `json:"scanTime,omitempty"`
}

// ImageRepositoryStatus defines the observed state of ImageRepository
Expand All @@ -76,26 +83,28 @@ type ImageRepositoryStatus struct {
}

// SetImageRepositoryReadiness sets the ready condition with the given status, reason and message.
func SetImageRepositoryReadiness(ir ImageRepository, status metav1.ConditionStatus, reason, message string) ImageRepository {
func SetImageRepositoryReadiness(ir *ImageRepository, status metav1.ConditionStatus, reason, message string) {
ir.Status.ObservedGeneration = ir.ObjectMeta.Generation
meta.SetResourceCondition(&ir, meta.ReadyCondition, status, reason, message)
return ir
}

// GetLastTransitionTime returns the LastTransitionTime attribute of the ready conditon
func GetLastTransitionTime(ir ImageRepository) *metav1.Time {
if rc := apimeta.FindStatusCondition(ir.Status.Conditions, meta.ReadyCondition); rc != nil {
return &rc.LastTransitionTime
}

return nil
meta.SetResourceCondition(ir, meta.ReadyCondition, status, reason, message)
}

// GetStatusConditions returns a pointer to the Status.Conditions slice
func (in *ImageRepository) GetStatusConditions() *[]metav1.Condition {
return &in.Status.Conditions
}

// GetTimeout returns the timeout with default.
func (in ImageRepository) GetTimeout() time.Duration {
duration := in.Spec.Interval.Duration
if in.Spec.Timeout != nil {
duration = in.Spec.Timeout.Duration
}
if duration < time.Second {
return time.Second
}
return duration
}

// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
// +kubebuilder:printcolumn:name="Last scan",type=string,JSONPath=`.status.lastScanTime`
Expand Down
11 changes: 8 additions & 3 deletions api/v1alpha1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 9 additions & 3 deletions config/crd/bases/image.toolkit.fluxcd.io_imagerepositories.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ spec:
image:
description: Image is the name of the image repository
type: string
scanInterval:
description: ScanInterval is the (minimum) length of time to wait
between scans of the image repository.
interval:
description: Interval is the length of time to wait between scans
of the image repository.
type: string
secretRef:
description: SecretRef can be given the name of a secret containing
Expand All @@ -66,6 +66,9 @@ spec:
image scans. It does not apply to already started scans. Defaults
to false.
type: boolean
timeout:
description: Timeout for image scanning. Defaults to 'Interval' duration.
type: string
type: object
status:
description: ImageRepositoryStatus defines the observed state of ImageRepository
Expand Down Expand Up @@ -151,6 +154,9 @@ spec:
lastScanResult:
description: LastScanResult contains the number of fetched tags.
properties:
scanTime:
format: date-time
type: string
tagCount:
type: integer
required:
Expand Down
89 changes: 47 additions & 42 deletions controllers/imagerepository_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ import (
imagev1alpha1 "github.com/fluxcd/image-reflector-controller/api/v1alpha1"
)

const (
scanTimeout = 10 * time.Second
defaultScanInterval = 10 * time.Minute
)

type DatabaseWriter interface {
SetTags(repo string, tags []string)
}
Expand All @@ -70,6 +65,7 @@ type ImageRepositoryReconciler struct {

func (r *ImageRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
ctx := context.Background()
reconcileStart := time.Now()

// NB: In general, if an error is returned then controller-runtime
// will requeue the request with back-off. In the following this
Expand All @@ -78,21 +74,20 @@ func (r *ImageRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, er

var imageRepo imagev1alpha1.ImageRepository
if err := r.Get(ctx, req.NamespacedName, &imageRepo); err != nil {
// _Might_ get requeued
return ctrl.Result{}, client.IgnoreNotFound(err)
}

log := r.Log.WithValues("controller", strings.ToLower(imagev1alpha1.ImageRepositoryKind), "request", req.NamespacedName)

if imageRepo.Spec.Suspend {
msg := "ImageRepository is suspended, skipping reconciliation"
status := imagev1alpha1.SetImageRepositoryReadiness(
imageRepo,
imagev1alpha1.SetImageRepositoryReadiness(
&imageRepo,
metav1.ConditionFalse,
meta.SuspendedReason,
msg,
)
if err := r.Status().Update(ctx, &status); err != nil {
if err := r.Status().Update(ctx, &imageRepo); err != nil {
log.Error(err, "unable to update status")
return ctrl.Result{Requeue: true}, err
}
Expand All @@ -102,47 +97,52 @@ func (r *ImageRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, er

ref, err := name.ParseReference(imageRepo.Spec.Image)
if err != nil {
status := imagev1alpha1.SetImageRepositoryReadiness(
imageRepo,
imagev1alpha1.SetImageRepositoryReadiness(
&imageRepo,
metav1.ConditionFalse,
imagev1alpha1.ImageURLInvalidReason,
err.Error(),
)
if err := r.Status().Update(ctx, &status); err != nil {
if err := r.Status().Update(ctx, &imageRepo); err != nil {
return ctrl.Result{Requeue: true}, err
}
log.Error(err, "Unable to parse image name", "imageName", imageRepo.Spec.Image)
return ctrl.Result{Requeue: true}, err
}

imageRepo.Status.CanonicalImageName = ref.Context().String()
// Set CanonicalImageName based on the parsed reference
if c := ref.Context().String(); imageRepo.Status.CanonicalImageName != c {
imageRepo.Status.CanonicalImageName = c
if err = r.Status().Update(ctx, &imageRepo); err != nil {
return ctrl.Result{Requeue: true}, err
}
}

// Throttle scans based on spec Interval
now := time.Now()
ok, when := r.shouldScan(imageRepo, now)
if ok {
ctx, cancel := context.WithTimeout(ctx, scanTimeout)
defer cancel()

reconciledRepo, reconcileErr := r.scan(ctx, imageRepo, ref)
if err = r.Status().Update(ctx, &reconciledRepo); err != nil {
reconcileErr := r.scan(ctx, &imageRepo, ref)
if err = r.Status().Update(ctx, &imageRepo); 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),
)
}
}

log.Info(fmt.Sprintf("reconciliation finished in %s, next run in %s",
time.Now().Sub(reconcileStart).String(),
when.String(),
))

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()
func (r *ImageRepositoryReconciler) scan(ctx context.Context, imageRepo *imagev1alpha1.ImageRepository, ref name.Reference) error {
timeout := imageRepo.GetTimeout()
ctx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()

var options []remote.Option
if imageRepo.Spec.SecretRef != nil {
Expand All @@ -151,39 +151,45 @@ func (r *ImageRepositoryReconciler) scan(ctx context.Context, imageRepo imagev1a
Namespace: imageRepo.GetNamespace(),
Name: imageRepo.Spec.SecretRef.Name,
}, &secret); err != nil {
return imagev1alpha1.SetImageRepositoryReadiness(
imagev1alpha1.SetImageRepositoryReadiness(
imageRepo,
metav1.ConditionFalse,
meta.ReconciliationFailedReason,
err.Error(),
), err
)
return err
}
auth, err := authFromSecret(secret, ref.Context().RegistryStr())
if err != nil {
return imagev1alpha1.SetImageRepositoryReadiness(
imagev1alpha1.SetImageRepositoryReadiness(
imageRepo,
metav1.ConditionFalse,
meta.ReconciliationFailedReason,
err.Error(),
), err
)
return err
}
options = append(options, remote.WithAuth(auth))
}

tags, err := remote.ListWithContext(ctx, ref.Context(), options...)
if err != nil {
return imagev1alpha1.SetImageRepositoryReadiness(
imagev1alpha1.SetImageRepositoryReadiness(
imageRepo,
metav1.ConditionFalse,
meta.ReconciliationFailedReason,
err.Error(),
), err
)
return err
}

canonicalName := ref.Context().String()
// TODO: add context and error handling to database ops
r.Database.SetTags(canonicalName, tags)

scanTime := metav1.Now()
imageRepo.Status.LastScanResult.TagCount = len(tags)
imageRepo.Status.LastScanResult.ScanTime = &scanTime

// if the reconcile request annotation was set, consider it
// handled (NB it doesn't matter here if it was changed since last
Expand All @@ -192,30 +198,29 @@ func (r *ImageRepositoryReconciler) scan(ctx context.Context, imageRepo imagev1a
imageRepo.Status.SetLastHandledReconcileRequest(token)
}

return imagev1alpha1.SetImageRepositoryReadiness(
imagev1alpha1.SetImageRepositoryReadiness(
imageRepo,
metav1.ConditionTrue,
meta.ReconciliationSucceededReason,
fmt.Sprintf("successful scan, found %v tags", len(tags)),
), nil
)

return 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) {
scanInterval := defaultScanInterval
if repo.Spec.ScanInterval != nil {
scanInterval = repo.Spec.ScanInterval.Duration
}
scanInterval := repo.Spec.Interval.Duration

// never scanned; do it now
lastTransitionTime := imagev1alpha1.GetLastTransitionTime(repo)
if lastTransitionTime == nil {
lastScanTime := repo.Status.LastScanResult.ScanTime
if lastScanTime == nil {
return true, scanInterval
}

// Is the controller seeing this because the reconcileAt
// Is the controller seeing this because the reconcileAt return nil
// annotation was tweaked? Despite the name of the annotation, all
// that matters is that it's different.
if syncAt, ok := meta.ReconcileAnnotationValue(repo.GetAnnotations()); ok {
Expand All @@ -235,7 +240,7 @@ func (r *ImageRepositoryReconciler) shouldScan(repo imagev1alpha1.ImageRepositor
return true, scanInterval
}

when := scanInterval - now.Sub(lastTransitionTime.Time)
when := scanInterval - now.Sub(lastScanTime.Time)
if when < time.Second {
return true, scanInterval
}
Expand Down
Loading

0 comments on commit c20dc7c

Please sign in to comment.