Merge pull request #112 from ricardomaraschini/conditions

Reporting ImageImport condition

chart/templates/crd.yaml

@@ -57,6 +57,64 @@ spec:
           status:
             type: object
             properties:
+              condition:
+                type: object
+                properties:
+                  lastTransitionTime:
+                    description: lastTransitionTime is the last time the condition
+                      transitioned from one status to another. This should be
+                      when the underlying condition changed.  If that is not known,
+                      then using the time when the API field changed is acceptable.
+                    format: date-time
+                    type: string
+                  message:
+                    description: message is a human readable message indicating
+                      details about the transition. This may be an empty string.
+                    maxLength: 32768
+                    type: string
+                  observedGeneration:
+                    description: observedGeneration represents the .metadata.generation
+                      that the condition was set based upon. For instance, if
+                      .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration
+                      is 9, the condition is out of date with respect to the current
+                      state of the instance.
+                    format: int64
+                    minimum: 0
+                    type: integer
+                  reason:
+                    description: reason contains a programmatic identifier indicating
+                      the reason for the condition's last transition. Producers
+                      of specific condition types may define expected values and
+                      meanings for this field, and whether the values are considered
+                      a guaranteed API. The value should be a CamelCase string.
+                      This field may not be empty.
+                    maxLength: 1024
+                    minLength: 1
+                    pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
+                    type: string
+                  status:
+                    description: status of the condition, one of True, False,
+                      Unknown.
+                    enum:
+                    - "True"
+                    - "False"
+                    - Unknown
+                    type: string
+                  type:
+                    description: type of condition in CamelCase or in foo.example.com/CamelCase.
+                      --- Many .condition.type values are consistent across resources
+                      like Available, but because arbitrary conditions can be
+                      useful (see .node.status.conditions), the ability to deconflict
+                      is important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
+                    maxLength: 316
+                    pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
+                    type: string
+                required:
+                - lastTransitionTime
+                - message
+                - reason
+                - status
+                - type
               importAttempts:
                 type: array
                 nullable: true

infra/images/v1beta1/types.go

@@ -34,9 +34,21 @@ var (
 	ImageKind = "Image"
 	// ImageImportKind holds the kind we use when saving ImageImports in the k8s API.
 	ImageImportKind = "ImageImport"
-	// FlaggedForDeletionAnnotation is the annotation set in an object whenever the operator
-	// feels like it has no use for it.
-	FlaggedForDeletionAnnotation = "tagger.dev/delete"
+	// ImageImportConsumedFlagAnnotation is the annotation set in an ImageImport object
+	// whenever the temporary ImageImport object has already been consumed and is not
+	// needed anymore.
+	ImageImportConsumedFlagAnnotation = "tagger.dev/consumed"
+	// ConditionTypeImported is a condition we report in ImageImport objects, presenting the
+	// current Import status back to the user.
+	ConditionTypeImported = "Imported"
+	// ConditionReasonProgressing is used to indicate the ImageImport is progressing.
+	ConditionReasonProgressing = "Progressing"
+	// ConditionReasonImageImported is used to indicate an ImageImport attempt has been
+	// executed sucessfully.
+	ConditionReasonImageImported = "ImageImported"
+	// ConditionReasonNoMoreAttempts is used when we can't proceed attempting to process an
+	// ImageImport object.
+	ConditionReasonNoMoreAttempts = "NoMoreAttempts"
 )
 
 // +genclient
@@ -95,34 +107,36 @@ func (t *Image) Validate() error {
 	return nil
 }
 
-// FlagForDeletion is used whenever we don't want this instance anymore. This Annotation does
-// not indicate anything at the k8s scope and it is solely used inside this operator. The value
-// in the annotation is the current date and time encoded as time.ANSIC.
-func (t *ImageImport) FlagForDeletion() {
+// FlagAsConsumed is used whenever we have already processed the data in an ImageImport object.
+// This Annotation does not indicate anything at the k8s scope and it is solely used inside this
+// operator. The value in the annotation is the current date and time encoded as time.ANSIC.
+func (t *ImageImport) FlagAsConsumed() {
 	if t.Annotations == nil {
 		t.Annotations = map[string]string{}
 	}
-	t.Annotations[FlaggedForDeletionAnnotation] = time.Now().Format(time.ANSIC)
+	t.Annotations[ImageImportConsumedFlagAnnotation] = time.Now().Format(time.ANSIC)
 }
 
-// FlaggedForDeletion returns if this ImageImport is flagged for deletion. Inspects the
+// FlaggedAsConsumed returns if this ImageImport is flagged for deletion. Inspects the
 // object's Annotations.
-func (t *ImageImport) FlaggedForDeletion() bool {
-	_, ok := t.Annotations[FlaggedForDeletionAnnotation]
+func (t *ImageImport) FlaggedAsConsumed() bool {
+	_, ok := t.Annotations[ImageImportConsumedFlagAnnotation]
 	return ok
 }
 
-// FlaggedForDeletionDuration returns the amount of time that has passed since the ImageImport
+// FlaggedAsConsumedDuration returns the amount of time that has passed since the ImageImport
 // was flagged for deletion.
-func (t *ImageImport) FlaggedForDeletionDuration() (time.Duration, error) {
-	if !t.FlaggedForDeletion() {
+func (t *ImageImport) FlaggedAsConsumedDuration() (time.Duration, error) {
+	if !t.FlaggedAsConsumed() {
 		return 0, fmt.Errorf("image import not flagged for deletion")
 	}
 
-	strsince := t.Annotations[FlaggedForDeletionAnnotation]
+	strsince := t.Annotations[ImageImportConsumedFlagAnnotation]
 	since, err := time.Parse(time.ANSIC, strsince)
 	if err != nil {
-		return 0, fmt.Errorf("bogus %s annotation: %w", FlaggedForDeletionAnnotation, err)
+		return 0, fmt.Errorf(
+			"bogus %s annotation: %w", ImageImportConsumedFlagAnnotation, err,
+		)
 	}
 
 	return time.Now().Sub(since), nil
@@ -271,7 +285,7 @@ func (t *ImageImport) FailedImportAttempts() int {
 }
 
 // RegisterImportFailure updates the import attempts slice appending a new failed attempt with
-// the provided error.
+// the provided error. This function also sets ImageImport.Status.Condition field.
 func (t *ImageImport) RegisterImportFailure(err error) {
 	t.Status.ImportAttempts = append(
 		t.Status.ImportAttempts,
@@ -281,10 +295,40 @@ func (t *ImageImport) RegisterImportFailure(err error) {
 			Reason:  err.Error(),
 		},
 	)
+
+	// we build kind of a base Condition and then adjust only the necessary fields. This
+	// base Condition means that we have failed all attempts at processing an ImportImage.
+	message := fmt.Sprintf("Import attempt %d/%d", MaxImportAttempts, MaxImportAttempts)
+	nextcond := metav1.Condition{
+		Type:               ConditionTypeImported,
+		Status:             metav1.ConditionFalse,
+		Reason:             ConditionReasonNoMoreAttempts,
+		Message:            message,
+		LastTransitionTime: metav1.NewTime(time.Now()),
+	}
+
+	failures := len(t.Status.ImportAttempts)
+	if failures >= MaxImportAttempts {
+		// here we have exhausted all import attempts, set it as Failed and return.
+		t.Status.Condition = nextcond
+		return
+	}
+
+	// if we hit here then we still have some import attempts to be executed, set its
+	// condition to Progressing.
+	message = fmt.Sprintf("Import attempt %d/%d", failures, MaxImportAttempts)
+	nextcond.Status = metav1.ConditionFalse
+	nextcond.Reason = ConditionReasonProgressing
+	nextcond.Message = message
+	nextcond.LastTransitionTime = t.Status.Condition.LastTransitionTime
+	if nextcond.LastTransitionTime.IsZero() {
+		nextcond.LastTransitionTime = metav1.NewTime(time.Now())
+	}
+	t.Status.Condition = nextcond
 }
 
 // RegisterImportSuccess appends a new ImportAttempt to the status registering it worked as
-// expected.
+// expected. This function also sets ImageImport.Status.Condition field.
 func (t *ImageImport) RegisterImportSuccess() {
 	t.Status.ImportAttempts = append(
 		t.Status.ImportAttempts,
@@ -293,6 +337,14 @@ func (t *ImageImport) RegisterImportSuccess() {
 			Succeed: true,
 		},
 	)
+
+	t.Status.Condition = metav1.Condition{
+		Type:               ConditionTypeImported,
+		Status:             metav1.ConditionTrue,
+		Reason:             ConditionReasonImageImported,
+		Message:            "Image imported successfully",
+		LastTransitionTime: metav1.NewTime(time.Now()),
+	}
 }
 
 // ImageImportSpec represents the body of the request to import a given container image tag from
@@ -307,8 +359,9 @@ type ImageImportSpec struct {
 
 // ImageImportStatus holds the current status for an image tag import attempt.
 type ImageImportStatus struct {
-	ImportAttempts []ImportAttempt `json:"importAttempts"`
-	HashReference  *HashReference  `json:"hashReference,omitempty"`
+	Condition      metav1.Condition `json:"condition"`
+	ImportAttempts []ImportAttempt  `json:"importAttempts"`
+	HashReference  *HashReference   `json:"hashReference,omitempty"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

services/image.go

@@ -77,7 +77,7 @@ func (t *Image) RecentlyFinishedImports(
 
 	var sortme []imgv1b1.ImageImport
 	for _, imp := range imports {
-		if !imp.AlreadyImported() || !imp.OwnedByImage(img) || imp.FlaggedForDeletion() {
+		if !imp.AlreadyImported() || !imp.OwnedByImage(img) || imp.FlaggedAsConsumed() {
 			continue
 		}
 
@@ -131,7 +131,7 @@ func (t *Image) Sync(ctx context.Context, img *imgv1b1.Image) error {
 	// can flag them for deletion. We ignore any errors here, the flagging process will
 	// be retried during next Sync call.
 	for _, imp := range newimports {
-		imp.FlagForDeletion()
+		imp.FlagAsConsumed()
 		if _, err := t.imgcli.TaggerV1beta1().ImageImports(img.Namespace).Update(
 			ctx, &imp, metav1.UpdateOptions{},
 		); err != nil {

services/imageimport.go

@@ -125,24 +125,24 @@ func (t *ImageImport) NewImageFor(
 }
 
 // Delete deletes an ImageImport according to some rules. In order to delete an import this
-// import must be flagged for deletion for at least one hour. The exception made is if the
-// import has a bogus or "unparseable" deletion timestamp, then we log the fact and delete.
+// import must be flagged as consumed for at least one hour. The exception made is if the
+// import has a bogus or "unparseable" consume timestamp, then we log the fact and delete.
 // We only return an error when we actually attempt to delete using k8s api, if the import
 // is filtered out by any of the forementioned rules a nil is returned instead.
 func (t *ImageImport) Delete(ctx context.Context, ii *imgv1b1.ImageImport) error {
-	if !ii.FlaggedForDeletion() {
+	if !ii.FlaggedAsConsumed() {
 		return nil
 	}
 
-	// we save all ImageImport whose deletion flags are readable and they have been
-	// flagged less than one hour ago.
-	duration, err := ii.FlaggedForDeletionDuration()
+	// we avoid to delete ImageImport whose consumed flag is readable and that have been
+	// flagged as consumed less than one hour ago.
+	duration, err := ii.FlaggedAsConsumedDuration()
 	if err == nil && duration < time.Hour {
 		return nil
 	}
 
-	// if we could not parse the deletion flag then we at least log this fact. We
-	// gonna go ahead and delete them.
+	// if we could not parse the consume flag then we at least log this fact. We gonna go
+	// ahead and delete the ImageImport.
 	if err != nil {
 		klog.Infof("deleting %s/%s: %s", ii.Namespace, ii.Name, err)
 	}
@@ -160,7 +160,7 @@ func (t *ImageImport) Sync(ctx context.Context, ii *imgv1b1.ImageImport) error {
 		return fmt.Errorf("invalid image import: %w", err)
 	}
 
-	if ii.FlaggedForDeletion() {
+	if ii.FlaggedAsConsumed() {
 		if err := t.Delete(ctx, ii); err != nil {
 			klog.V(5).Infof(
 				"unable to delete image import %s/%s: %s",
@@ -178,7 +178,7 @@ func (t *ImageImport) Sync(ctx context.Context, ii *imgv1b1.ImageImport) error {
 	// if no more attempts are going to be made on this ImageImport we can flag it for
 	// deletion. Deletion tends to take a while, check Delete() func for more on this.
 	if ii.FailedImportAttempts() >= imgv1b1.MaxImportAttempts {
-		ii.FlagForDeletion()
+		ii.FlagAsConsumed()
 		if _, err := t.imgcli.TaggerV1beta1().ImageImports(ii.Namespace).Update(
 			ctx, ii, metav1.UpdateOptions{},
 		); err != nil {