From 78212e58dd19467c762676d417a6ddb88755c0cd Mon Sep 17 00:00:00 2001 From: Mickael Stanislas Date: Wed, 6 Nov 2024 15:01:16 +0100 Subject: [PATCH] fix: image status --- .changelog/101.txt | 7 ++++ api/v1alpha1/image_models.go | 2 +- cmd/kimup/event.go | 22 +++++++----- cmd/kimup/main.go | 2 ++ cmd/kimup/scheduler.go | 46 +++++++++++++++++++------ docs/crd/image.md | 18 +++++----- internal/controller/image_controller.go | 1 + tools/doc/generate-doc-crd-image.go | 9 +++-- 8 files changed, 76 insertions(+), 31 deletions(-) create mode 100644 .changelog/101.txt diff --git a/.changelog/101.txt b/.changelog/101.txt new file mode 100644 index 0000000..bbccc48 --- /dev/null +++ b/.changelog/101.txt @@ -0,0 +1,7 @@ +```release-note:bug +`image` - Now the image is displayed correctly the `last-result`. +``` + +```release-note:enhancement +`image/status` - Fix the documentation of the `image/status. +``` \ No newline at end of file diff --git a/api/v1alpha1/image_models.go b/api/v1alpha1/image_models.go index 7c43295..ffe2616 100644 --- a/api/v1alpha1/image_models.go +++ b/api/v1alpha1/image_models.go @@ -4,7 +4,7 @@ type ( ImageStatusLastSync string ) -var ( +const ( // Status of the image when it is last sync success. ImageStatusLastSyncSuccess ImageStatusLastSync = "Success" diff --git a/cmd/kimup/event.go b/cmd/kimup/event.go index d29b745..b5a53a4 100644 --- a/cmd/kimup/event.go +++ b/cmd/kimup/event.go @@ -65,19 +65,23 @@ func refreshIfRequired(an annotations.Annotation, image v1alpha1.Image) { "namespace": image.Namespace, "name": image.Name, }).Info("Annotation trigger refresh") - _, err := triggers.Trigger(triggers.RefreshImage, image.Namespace, image.Name) - if err != nil { - log. - WithFields(logrus.Fields{ - "namespace": image.Namespace, - "name": image.Name, - }). - Error("Error triggering event") - } + refresh(image) an.Remove(annotations.KeyAction) } } +func refresh(image v1alpha1.Image) { + _, err := triggers.Trigger(triggers.RefreshImage, image.Namespace, image.Name) + if err != nil { + log. + WithFields(logrus.Fields{ + "namespace": image.Namespace, + "name": image.Name, + }). + Error("Error triggering event") + } +} + func setTagIfNotExists(ctx context.Context, kubeClient kubeclient.Interface, an annotations.Annotation, image *v1alpha1.Image) error { if an.Tag().IsNull() { a, err := actions.GetAction(actions.Apply) diff --git a/cmd/kimup/main.go b/cmd/kimup/main.go index 63e0270..367436d 100644 --- a/cmd/kimup/main.go +++ b/cmd/kimup/main.go @@ -119,6 +119,8 @@ func main() { } } + refresh(event.Value) + // Remove the annotation annotations.AnnotationActionKey in the map[string]string an.Remove(annotations.KeyAction) } diff --git a/cmd/kimup/scheduler.go b/cmd/kimup/scheduler.go index 8617008..59f8e2e 100644 --- a/cmd/kimup/scheduler.go +++ b/cmd/kimup/scheduler.go @@ -38,29 +38,49 @@ func initScheduler(ctx context.Context, k kubeclient.Interface) { timerEvents := metrics.Events().TriggeredDuration.NewTimer() defer timerEvents.ObserveDuration() - if l[e.Data()["namespace"].(string)+"/"+e.Data()["image"].(string)] == nil { - l[e.Data()["namespace"].(string)+"/"+e.Data()["image"].(string)] = &sync.RWMutex{} + var ( + namespaceName = e.Data()["namespace"].(string) + imageName = e.Data()["image"].(string) + ) + + if l[namespaceName+"/"+imageName] == nil { + l[namespaceName+"/"+imageName] = &sync.RWMutex{} } // Lock the image to prevent concurrent refreshes - l[e.Data()["namespace"].(string)+"/"+e.Data()["image"].(string)].Lock() - defer l[e.Data()["namespace"].(string)+"/"+e.Data()["image"].(string)].Unlock() + l[namespaceName+"/"+imageName].Lock() + defer l[namespaceName+"/"+imageName].Unlock() // Sleep for 1 second to prevent concurrent refreshes time.Sleep(1 * time.Second) retryErr := retry.RetryOnConflict(retry.DefaultBackoff, func() error { - log.Infof("Refreshing image %s in namespace %s", e.Data()["image"], e.Data()["namespace"]) + log.Infof("Refreshing image %s in namespace %s", imageName, namespaceName) ctx, cancel := context.WithTimeout(ctx, 60*time.Second) defer cancel() - image, err := k.Image().Get(ctx, e.Data()["namespace"].(string), e.Data()["image"].(string)) + image, err := k.Image().Get(ctx, namespaceName, imageName) defer func() { + // Add delay to avoid conflicts + time.Sleep(500 * time.Millisecond) + // update the status of the image image.SetStatusTime(time.Now().Format(time.RFC3339)) - err := k.Image().UpdateStatus(ctx, image) + + // Need to get image again to avoid conflicts + imageRefreshed, err := k.Image().Get(ctx, namespaceName, imageName) if err != nil { + log.WithError(err). + WithFields(log.Fields{ + "Namespace": namespaceName, + "Image": imageName, + }).Error("Error getting image") + return + } + imageRefreshed.Status = image.Status + + if err := k.Image().UpdateStatus(ctx, imageRefreshed); err != nil { log.WithError(err). WithFields(log.Fields{ "Namespace": e.Data()["namespace"], @@ -70,13 +90,16 @@ func initScheduler(ctx context.Context, k kubeclient.Interface) { }() if err != nil { image.SetStatusResult(v1alpha1.ImageStatusLastSyncErrorGetImage) - if err := crontab.RemoveJob(crontab.BuildKey(e.Data()["namespace"].(string), e.Data()["image"].(string))); err != nil { + if err := crontab.RemoveJob(crontab.BuildKey(namespaceName, imageName)); err != nil { return err } return err } k.Image().Event(&image, corev1.EventTypeNormal, "Image update triggered", "") + // Set Status to Scheduled permit in the execution of the refresh if the image have a error or not + image.SetStatusResult(v1alpha1.ImageStatusLastSyncScheduled) + var auths kubeclient.K8sDockerRegistrySecretData if image.Spec.ImagePullSecrets != nil { @@ -174,8 +197,9 @@ func initScheduler(ctx context.Context, k kubeclient.Interface) { if match { for _, action := range rule.Actions { a, err := actions.GetActionWithUntypedName(action.Type) - image.SetStatusResult(v1alpha1.ImageStatusLastSyncErrorAction) if err != nil { + image.SetStatusResult(v1alpha1.ImageStatusLastSyncErrorAction) + k.Image().Event(&image, corev1.EventTypeWarning, "Get action", fmt.Sprintf("Error getting action %s: %v", action.Type, err)) log.Errorf("Error getting action: %v", err) continue } @@ -209,7 +233,9 @@ func initScheduler(ctx context.Context, k kubeclient.Interface) { } } - image.SetStatusResult(v1alpha1.ImageStatusLastSyncSuccess) + if image.Status.Result == v1alpha1.ImageStatusLastSyncScheduled { + image.SetStatusResult(v1alpha1.ImageStatusLastSyncSuccess) + } return k.Image().Update(ctx, image) }) diff --git a/docs/crd/image.md b/docs/crd/image.md index 7aa0f06..f60914c 100644 --- a/docs/crd/image.md +++ b/docs/crd/image.md @@ -79,14 +79,14 @@ The following status can be set on an image: | Last-Sync state | Description | | --------------- | ----------- | -| `ImageStatusLastSyncErrorAction` | Status of the image when it is last sync error action. | -| `ImageStatusLastSyncErrorGetImage` | Status of the image when it is last sync get error. | -| `ImageStatusLastSyncErrorGetRule` | Status of the image when it is last sync error get rule. | -| `ImageStatusLastSyncErrorPullSecrets` | Status of the image when it is last sync error secrets. | -| `ImageStatusLastSyncErrorRegistry` | Status of the image when it is last sync error registry. | -| `ImageStatusLastSyncErrorTags` | Status of the image when it is last sync error tags. | -| `ImageStatusLastSyncError` | Status of the image when an error occurred. | -| `ImageStatusLastSyncScheduled` | Status of the image when it is last sync is scheduled. | -| `ImageStatusLastSyncSuccess` | Status of the image when it is last sync success. | +| "ActionError" | Status of the image when it is last sync error action. | +| "Error" | Status of the image when an error occurred. | +| "GetImageError" | Status of the image when it is last sync get error. | +| "GetRuleError" | Status of the image when it is last sync error get rule. | +| "PullSecretsError" | Status of the image when it is last sync error secrets. | +| "RegistryError" | Status of the image when it is last sync error registry. | +| "Scheduled" | Status of the image when it is last sync is scheduled. | +| "Success" | Status of the image when it is last sync success. | +| "TagsError" | Status of the image when it is last sync error tags. | diff --git a/internal/controller/image_controller.go b/internal/controller/image_controller.go index e78d991..2d18c32 100644 --- a/internal/controller/image_controller.go +++ b/internal/controller/image_controller.go @@ -90,6 +90,7 @@ func (r *ImageReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl r.Recorder.Event(&image, "Warning", string(ImageUpdate), fmt.Sprintf("Failed to set checksum: %v", err)) return ctrl.Result{}, err } + if err := r.Client.Update(ctx, &image); err != nil { xlog.WithError(err).Error("unable to update Image") r.Recorder.Event(&image, "Warning", string(ImageUpdate), fmt.Sprintf("Failed to update image: %v", err)) diff --git a/tools/doc/generate-doc-crd-image.go b/tools/doc/generate-doc-crd-image.go index e1eef05..85c29be 100644 --- a/tools/doc/generate-doc-crd-image.go +++ b/tools/doc/generate-doc-crd-image.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "go/ast" "go/parser" "go/token" @@ -29,7 +28,13 @@ func generateDocImageCRD() { for _, spec := range decl.(*ast.GenDecl).Specs { if _, ok := spec.(*ast.ValueSpec); ok { for _, ident := range spec.(*ast.ValueSpec).Names { - imgStatusSlice = append(imgStatusSlice, []string{fmt.Sprintf("`%s`", ident.Name), strings.TrimSuffix(ident.Obj.Decl.(*ast.ValueSpec).Doc.Text(), "\n")}) + // Get value of const + if ident.Obj.Kind == ast.Con { + value := ident.Obj.Decl.(*ast.ValueSpec).Values[0].(*ast.BasicLit).Value + description := ident.Obj.Decl.(*ast.ValueSpec).Doc.Text() + + imgStatusSlice = append(imgStatusSlice, []string{strings.TrimSuffix(value, "\n"), strings.TrimSuffix(description, "\n")}) + } } } }