From 99da1bb0a581270563cefe0c0982659f6306be26 Mon Sep 17 00:00:00 2001 From: Lorenzo Felletti Date: Tue, 19 Nov 2024 23:47:21 +0000 Subject: [PATCH 1/4] refactor: finalizer example --- .../src/cronjob-tutorial/testdata/finalizer_example.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/book/src/cronjob-tutorial/testdata/finalizer_example.go b/docs/book/src/cronjob-tutorial/testdata/finalizer_example.go index a3101d02cb5..00c683dad48 100644 --- a/docs/book/src/cronjob-tutorial/testdata/finalizer_example.go +++ b/docs/book/src/cronjob-tutorial/testdata/finalizer_example.go @@ -67,7 +67,12 @@ func (r *CronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct // then lets add the finalizer and update the object. This is equivalent // to registering our finalizer. if !controllerutil.ContainsFinalizer(cronJob, myFinalizerName) { - controllerutil.AddFinalizer(cronJob, myFinalizerName) + log.Info("Adding Finalizer for CronJob") + if ok := controllerutil.AddFinalizer(cronJob, myFinalizerName); !ok { + log.Error(err, "Failed to add finalizer into the custom resource") + return ctrl.Result{Requeue: true}, nil + } + if err := r.Update(ctx, cronJob); err != nil { return ctrl.Result{}, err } From 5bdafe05e59ae10744f677c4d45d6c1d082093b9 Mon Sep 17 00:00:00 2001 From: Lorenzo Felletti Date: Mon, 25 Nov 2024 17:02:08 +0000 Subject: [PATCH 2/4] refactor: finalizer example to re-fetch after updating resource --- .../src/cronjob-tutorial/testdata/finalizer_example.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/book/src/cronjob-tutorial/testdata/finalizer_example.go b/docs/book/src/cronjob-tutorial/testdata/finalizer_example.go index 00c683dad48..b2b8e54b9ad 100644 --- a/docs/book/src/cronjob-tutorial/testdata/finalizer_example.go +++ b/docs/book/src/cronjob-tutorial/testdata/finalizer_example.go @@ -76,6 +76,14 @@ func (r *CronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct if err := r.Update(ctx, cronJob); err != nil { return ctrl.Result{}, err } + + // we re-fetch after having updated the CronJob, so that we have the latest + // state of the resource, and avoid the error "the object has been modified, + // please apply your changes to the latest version and try again". + if err := r.Get(ctx, req.NamespacedName, cronJob); err != nil { + log.Error(err, "unable to fetch CronJob") + return ctrl.Result{}, client.IgnoreNotFound(err) + } } } else { // The object is being deleted From c767eca2eb22394e7bc4d7801eb62c7ebbb9355b Mon Sep 17 00:00:00 2001 From: Lorenzo Felletti Date: Thu, 28 Nov 2024 23:19:45 +0000 Subject: [PATCH 3/4] feat: add status condition to CronJob example --- .../testdata/project/api/v1/cronjob_types.go | 5 +- .../internal/controller/cronjob_controller.go | 50 ++++++++++++++++--- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/docs/book/src/cronjob-tutorial/testdata/project/api/v1/cronjob_types.go b/docs/book/src/cronjob-tutorial/testdata/project/api/v1/cronjob_types.go index ee782bfb488..95d76ce7941 100644 --- a/docs/book/src/cronjob-tutorial/testdata/project/api/v1/cronjob_types.go +++ b/docs/book/src/cronjob-tutorial/testdata/project/api/v1/cronjob_types.go @@ -144,9 +144,12 @@ const ( // CronJobStatus defines the observed state of CronJob. type CronJobStatus struct { - // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster // Important: Run "make" to regenerate code after modifying this file + // Represents the observations of a CronJob's current state. + // For further information see: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` + // A list of pointers to currently running jobs. // +optional Active []corev1.ObjectReference `json:"active,omitempty"` diff --git a/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller.go b/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller.go index c85d8883994..96f1a356a7b 100644 --- a/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller.go +++ b/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller.go @@ -31,6 +31,8 @@ import ( "github.com/robfig/cron" kbatch "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ref "k8s.io/client-go/tools/reference" @@ -66,6 +68,14 @@ type Clock interface { Now() time.Time } +/* +Definitions to manage status conditions of the CronJob +*/ +const ( + typeAvailableCronJob = "AvailableCronJob" + typeDegradedCronJob = "DegradedCronJob" +) + // +kubebuilder:docs-gen:collapse=Clock /* @@ -111,15 +121,41 @@ func (r *CronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct Many client methods also take variadic options at the end. */ - var cronJob batchv1.CronJob - if err := r.Get(ctx, req.NamespacedName, &cronJob); err != nil { - log.Error(err, "unable to fetch CronJob") - // we'll ignore not-found errors, since they can't be fixed by an immediate - // requeue (we'll need to wait for a new notification), and we can get them - // on deleted requests. - return ctrl.Result{}, client.IgnoreNotFound(err) + cronJob := &batchv1.CronJob{} + if err := r.Get(ctx, req.NamespacedName, cronJob); err != nil { + if apierrors.IsNotFound(err) { + // we'll ignore not-found errors, since they can't be fixed by an immediate + // requeue (we'll need to wait for a new notification), and we can get them + // on deleted requests. + // If the CronJob is not found then it usually means that it was deleted or + // not created. In this way we will stop the reconciliation + log.Info("CronJob resource not found. Ignoring since object must be deleted") + return ctrl.Result{}, nil + } + // Error reading the object - requeue the request. + log.Error(err, "Failed to fetch CronJob") + return ctrl.Result{}, err } + // Let's just set the status as Unknown when no status is available + if cronJob.Status.Conditions == nil || len(cronJob.Status.Conditions) == 0 { + meta.SetStatusCondition(&cronJob.Status.Conditions, metav1.Condition{Type: typeAvailableCronJob, Status: metav1.ConditionUnknown, Reason: "Reconciling", Message: "Starting reconciliation"}) + if err := r.Status().Update(ctx, cronJob); err != nil { + log.Error(err, "Failed to update CronJob status") + return ctrl.Result{}, err + } + + // Re-fetch the CronJob after updating the status so that we have + // the latest state of the resource on the cluster and avoid raising + // an error should we try to update it again in the following operations + if err := r.Get(ctx, req.NamespacedName, cronJob); err != nil { + log.Error(err, "Failed to re-fetch CronJob") + return ctrl.Result{}, err + } + } + + // TODO(dev): add finalizer logic + /* ### 2: List all active jobs, and update the status From 44088d93cd1c3d697dd718029133158bb4131b19 Mon Sep 17 00:00:00 2001 From: Lorenzo Felletti Date: Tue, 10 Dec 2024 22:06:43 +0000 Subject: [PATCH 4/4] fix: remove double reference --- .../project/internal/controller/cronjob_controller.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller.go b/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller.go index 96f1a356a7b..784d84dd85e 100644 --- a/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller.go +++ b/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller.go @@ -291,7 +291,7 @@ func (r *CronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct The status subresource ignores changes to spec, so it's less likely to conflict with any other updates, and can have separate permissions. */ - if err := r.Status().Update(ctx, &cronJob); err != nil { + if err := r.Status().Update(ctx, cronJob); err != nil { log.Error(err, "unable to update CronJob status") return ctrl.Result{}, err } @@ -432,7 +432,7 @@ func (r *CronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct // figure out the next times that we need to create // jobs at (or anything we missed). - missedRun, nextRun, err := getNextSchedule(&cronJob, r.Now()) + missedRun, nextRun, err := getNextSchedule(cronJob, r.Now()) if err != nil { log.Error(err, "unable to figure out CronJob schedule") // we don't really care about requeuing until we get an update that @@ -536,7 +536,7 @@ func (r *CronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct // +kubebuilder:docs-gen:collapse=constructJobForCronJob // actually make the job... - job, err := constructJobForCronJob(&cronJob, missedRun) + job, err := constructJobForCronJob(cronJob, missedRun) if err != nil { log.Error(err, "unable to construct job from template") // don't bother requeuing until we get a change to the spec