diff --git a/internal/controller/appdeployment_controller.go b/internal/controller/appdeployment_controller.go index 66ae478..c2df995 100644 --- a/internal/controller/appdeployment_controller.go +++ b/internal/controller/appdeployment_controller.go @@ -58,7 +58,7 @@ type AppDeploymentReconciler struct { // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.20.4/pkg/reconcile func (r *AppDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - logger := klog.FromContext(ctx).WithValues(log.AppDeploymentJobName, req.NamespacedName) + logger := klog.FromContext(ctx).WithValues(log.FieldKeyAppDeploymentJobName, req.NamespacedName) appdeployment := &v1alpha1.AppDeployment{} if err := r.Get(ctx, req.NamespacedName, appdeployment); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) diff --git a/internal/handler/appdeployment.go b/internal/handler/appdeployment.go index a86ff9b..c2773cf 100644 --- a/internal/handler/appdeployment.go +++ b/internal/handler/appdeployment.go @@ -2,7 +2,9 @@ package handler import ( "context" + "errors" "fmt" + "strings" "github.com/go-logr/logr" batchv1 "k8s.io/api/batch/v1" @@ -21,6 +23,10 @@ import ( type AppdeploymentHandlerContextKey struct{} +var ( + ErrJobFailed = errors.New("job failed") +) + //go:generate mockgen -destination=./mocks/mock_appdeployment.go -package=mocks github.com/Azure/operation-cache-controller/internal/handler AppDeploymentHandlerInterface type AppDeploymentHandlerInterface interface { EnsureApplicationValid(ctx context.Context) (reconciler.OperationResult, error) @@ -166,7 +172,15 @@ func (a *AppDeploymentHandler) initializeJobAndAwaitCompletion(ctx context.Conte a.recorder.Event(a.appDeployment, "Error", "FailedDeleteJob", err.Error()) return fmt.Errorf("failed to delete job %s: %w", job.Name, err) } - // create a new job + // complete the job if it is a teardown job + if strings.HasPrefix(jobTemplate.Name, ctrlutils.JobTypeTeardown) { + a.logger.Error(ErrJobFailed, "teardown job failed", log.FieldKeyAppDeploymentJobName, jobTemplate.Name) + a.recorder.Event(a.appDeployment, "Warning", "TeardownJobFailed", fmt.Sprintf("Teardown job %s failed, requeuing for retry", jobTemplate.Name)) + // return nil to make the teardown job complete + return nil + } + + // create a new job if it is not a teardown job if err := ctrl.SetControllerReference(a.appDeployment, jobTemplate, a.client.Scheme()); err != nil { return fmt.Errorf("failed to set controller reference for job %s: %w", job.Name, err) } @@ -203,7 +217,7 @@ func (a *AppDeploymentHandler) EnsureDeployingFinished(ctx context.Context) (rec a.appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseReady return reconciler.RequeueOnErrorOrContinue(a.client.Status().Update(ctx, a.appDeployment)) case errJobNotCompleted: - a.logger.V(1).WithValues(log.AppDeploymentJobName, provisionJob.Name).Info("provision job is not completed yet") + a.logger.V(1).WithValues(log.FieldKeyAppDeploymentJobName, provisionJob.Name).Info("provision job is not completed yet") return reconciler.Requeue() default: a.logger.Error(err, "provision job failed %s", provisionJob.Name) @@ -224,10 +238,10 @@ func (a *AppDeploymentHandler) EnsureTeardownFinished(ctx context.Context) (reco a.appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeleted return reconciler.RequeueOnErrorOrContinue(a.client.Status().Update(ctx, a.appDeployment)) case errJobNotCompleted: - a.logger.V(1).WithValues(log.AppDeploymentJobName, teardownJob.Name).Info("teardown job is not completed yet") + a.logger.V(1).WithValues(log.FieldKeyAppDeploymentJobName, teardownJob.Name).Info("teardown job is not completed yet") return reconciler.Requeue() default: - a.logger.WithValues(log.AppDeploymentJobName, teardownJob.Name).Error(err, "teardown job failed %s") + a.logger.WithValues(log.FieldKeyAppDeploymentJobName, teardownJob.Name).Error(err, "teardown job failed %s") return reconciler.RequeueWithError(err) } } diff --git a/internal/handler/appdeployment_test.go b/internal/handler/appdeployment_test.go index 5a764cc..3186326 100644 --- a/internal/handler/appdeployment_test.go +++ b/internal/handler/appdeployment_test.go @@ -728,6 +728,10 @@ func TestAppDeploymentAdapter_EnsureTeardownFinished(t *testing.T) { Type: batchv1.JobFailed, Status: "True", }, + { + Type: batchv1.JobComplete, + Status: "True", + }, }, Failed: 1, }, @@ -837,13 +841,13 @@ func TestAppDeploymentAdapter_EnsureTeardownFinished(t *testing.T) { return nil }) mockClient.EXPECT().Delete(ctx, gomock.Any(), gomock.Any()).Return(nil) - mockClient.EXPECT().Create(ctx, gomock.Any()).Return(nil) - mockRecorder.EXPECT().Event(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + mockRecorder.EXPECT().Event(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1) mockStatusWriter.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() res, err := adapter.EnsureTeardownFinished(ctx) assert.NoError(t, err) - assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, res) + assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: false}, res) }) + } func TestAppDeploymentAdapter_EnsureTeardownFinished_JobErrors(t *testing.T) { diff --git a/internal/log/const.go b/internal/log/const.go index d5ceeb9..328b4c5 100644 --- a/internal/log/const.go +++ b/internal/log/const.go @@ -2,6 +2,6 @@ package log const ( // log keys - AppDeploymentJobName = "jobName" - AppDeploymentName = "appDeploymentName" + FieldKeyAppDeploymentJobName = "jobName" + AppDeploymentName = "appDeploymentName" )