Skip to content

Commit

Permalink
fix bug when workflow already executed when recocile, it will never b…
Browse files Browse the repository at this point in the history
…e scheduled

Signed-off-by: Yu Jiang <yu_jiang@intuit.com>
  • Loading branch information
Yu Jiang committed Sep 15, 2024
1 parent 52b6ad0 commit d48a23d
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 7 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*.so
*.dylib
testbin/*
active-monitor-controller

# Temporary or metadata files
*.yaml-e
Expand Down
6 changes: 6 additions & 0 deletions Dockerfile-local
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Use distroless as minimal base image to package the manager binary
# Refer to https://github.com/GoogleContainerTools/distroless for more details
FROM gcr.io/distroless/static:latest
WORKDIR /
COPY active-monitor-controller .
ENTRYPOINT [ "/active-monitor-controller" ]
8 changes: 8 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ test: manifests generate fmt vet envtest ## Run tests.
build: manifests generate fmt vet ## Build manager binary.
go build -o bin/manager cmd/main.go

.PHONY: build-amd64
build: manifests generate fmt vet ## Build manager binary.
CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o active-monitor-controller cmd/main.go

.PHONY: run
run: manifests generate fmt vet ## Run a controller from your host.
go run ./cmd/main.go
Expand All @@ -83,6 +87,10 @@ run: manifests generate fmt vet ## Run a controller from your host.
docker-build: ## Build docker image with the manager.
$(CONTAINER_TOOL) build -t ${IMG} .

.PHONY: docker-build-local
docker-build: ## Build docker image with the manager.
$(CONTAINER_TOOL) build -t ${IMG} -f Dockerfile-local .

.PHONY: docker-push
docker-push: ## Push docker image with the manager.
$(CONTAINER_TOOL) push ${IMG}
Expand Down
25 changes: 18 additions & 7 deletions internal/controllers/healthcheck_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ func (r *HealthCheckReconciler) processHealthCheck(ctx context.Context, log logr
finishedAtTime = healthCheck.Status.FinishedAt.Time.Unix()
}

log.Info("FinishedAtTime", "finishedAtTime", finishedAtTime)

// workflows can be paused by setting repeatAfterSec to <= 0 and not specifying the schedule for cron.
if hcSpec.RepeatAfterSec <= 0 && hcSpec.Schedule.Cron == "" {
log.Info("Workflow will be skipped due to repeatAfterSec value", "repeatAfterSec", hcSpec.RepeatAfterSec)
Expand All @@ -217,8 +219,8 @@ func (r *HealthCheckReconciler) processHealthCheck(ctx context.Context, log logr
// we need to update the spec so have to healthCheck.Spec.RepeatAfterSec instead of local variable hcSpec
healthCheck.Spec.RepeatAfterSec = int(schedule.Next(time.Now()).Sub(time.Now())/time.Second) + 1
log.Info("spec.RepeatAfterSec value is set", "RepeatAfterSec", healthCheck.Spec.RepeatAfterSec)
} else if int(time.Now().Unix()-finishedAtTime) < hcSpec.RepeatAfterSec {
log.Info("Workflow already executed", "finishedAtTime", finishedAtTime)
} else if int(time.Now().Unix()-finishedAtTime) < hcSpec.RepeatAfterSec && r.RepeatTimersByName[healthCheck.GetName()] != nil {
log.Info("Workflow already executed, and there is repeat schedule has been added to RepeatTimersByName map", "finishedAtTime", finishedAtTime)

Check warning on line 223 in internal/controllers/healthcheck_controller.go

View check run for this annotation

Codecov / codecov/patch

internal/controllers/healthcheck_controller.go#L223

Added line #L223 was not covered by tests
return ctrl.Result{}, nil
}

Expand Down Expand Up @@ -421,18 +423,25 @@ func (r *HealthCheckReconciler) deleteRBACForWorkflow(ctx context.Context, log l
// this function exists to assist with how a function called by the timer.AfterFunc() method operates to call a
// function which takes parameters, it is easiest to build this closure which holds access to the parameters we need.
// the helper returns a function object taking no parameters directly, this is what we want to give AfterFunc
func (r *HealthCheckReconciler) createSubmitWorkflowHelper(ctx context.Context, log logr.Logger, wfNamespace string, hc *activemonitorv1alpha1.HealthCheck) func() {
func (r *HealthCheckReconciler) createSubmitWorkflowHelper(ctx context.Context, log logr.Logger, wfNamespace string, prevHealthCheck *activemonitorv1alpha1.HealthCheck) func() {
return func() {
log.Info("Creating and Submitting Workflow...")
wfName, err := r.createSubmitWorkflow(ctx, log, hc)

healthCheckNew := &activemonitorv1alpha1.HealthCheck{}
if err := r.Get(ctx, client.ObjectKey{Name: prevHealthCheck.Name, Namespace: prevHealthCheck.Namespace}, healthCheckNew); err != nil {
log.Error(err, "Error getting healthcheck resource")
return
}

wfName, err := r.createSubmitWorkflow(ctx, log, healthCheckNew)
if err != nil {
log.Error(err, "Error creating or submitting workflow")
r.Recorder.Event(hc, v1.EventTypeWarning, "Warning", "Error creating or submitting workflow")
r.Recorder.Event(healthCheckNew, v1.EventTypeWarning, "Warning", "Error creating or submitting workflow")

Check warning on line 439 in internal/controllers/healthcheck_controller.go

View check run for this annotation

Codecov / codecov/patch

internal/controllers/healthcheck_controller.go#L439

Added line #L439 was not covered by tests
}
err = r.watchWorkflowReschedule(ctx, ctrl.Request{}, log, wfNamespace, wfName, hc)
err = r.watchWorkflowReschedule(ctx, ctrl.Request{}, log, wfNamespace, wfName, healthCheckNew)
if err != nil {
log.Error(err, "Error watching or rescheduling workflow")
r.Recorder.Event(hc, v1.EventTypeWarning, "Warning", "Error watching or rescheduling workflow")
r.Recorder.Event(healthCheckNew, v1.EventTypeWarning, "Warning", "Error watching or rescheduling workflow")
}
}
}
Expand Down Expand Up @@ -652,6 +661,8 @@ func (r *HealthCheckReconciler) watchWorkflowReschedule(ctx context.Context, req
break
}
}

log.Info("waiting for workflow to complete", "namespace", wfNamespace, "name", wfName)
}

// since the workflow has taken an unknown duration of time to complete, it's possible that its parent
Expand Down

0 comments on commit d48a23d

Please sign in to comment.