From a519bf1488b0659b3832eaf9c1550c0775e66183 Mon Sep 17 00:00:00 2001
From: Aditya Thebe <contact@adityathebe.com>
Date: Mon, 7 Oct 2024 19:06:29 +0545
Subject: [PATCH] fix: canary suspension

[skip ci]
---
 go.mod                               |  2 +-
 pkg/controllers/canary_controller.go |  9 ++++++++-
 pkg/db/canary.go                     | 12 ++++++++----
 pkg/jobs/canary/canary_jobs.go       |  3 ++-
 pkg/jobs/canary/sync.go              |  5 +++--
 pkg/runner/runner.go                 |  4 ++++
 pkg/utils/utils.go                   | 14 ++++++++++++++
 7 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/go.mod b/go.mod
index cf70e3774..c57fe6a0b 100644
--- a/go.mod
+++ b/go.mod
@@ -334,7 +334,7 @@ require (
 	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
 )
 
-// replace github.com/flanksource/duty => ../duty
+replace github.com/flanksource/duty => ../duty
 
 // replace github.com/flanksource/artifacts => ../artifacts
 
diff --git a/pkg/controllers/canary_controller.go b/pkg/controllers/canary_controller.go
index 8376639aa..db3b21008 100644
--- a/pkg/controllers/canary_controller.go
+++ b/pkg/controllers/canary_controller.go
@@ -21,6 +21,7 @@ import (
 	"time"
 
 	"github.com/flanksource/canary-checker/pkg/db"
+	"github.com/flanksource/canary-checker/pkg/utils"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -77,6 +78,7 @@ func (r *CanaryReconciler) Reconcile(parentCtx gocontext.Context, req ctrl.Reque
 	if runner.IsCanaryIgnored(&canary.ObjectMeta) {
 		return ctrl.Result{}, nil
 	}
+
 	ctx := r.Context.WithObject(canary.ObjectMeta).WithName(req.NamespacedName.String())
 
 	canary.SetRunnerName(r.RunnerName)
@@ -207,8 +209,13 @@ func (r *CanaryReconciler) updateCanaryInDB(ctx dutyContext.Context, canary *v1.
 	if err != nil {
 		return nil, err
 	}
+
 	opts := jsondiff.DefaultJSONOptions()
-	if diff, _ := jsondiff.Compare(canarySpecJSON, dbCanary.Spec, &opts); diff != jsondiff.FullMatch {
+	diff, _ := jsondiff.Compare(canarySpecJSON, dbCanary.Spec, &opts)
+	specChanged := diff != jsondiff.FullMatch
+
+	annotationsChanged := !utils.IsMapIdentical(canary.Annotations, dbCanary.Annotations)
+	if annotationsChanged || specChanged {
 		dbCanary, err = r.persistAndCacheCanary(ctx, canary)
 		if err != nil {
 			return nil, err
diff --git a/pkg/db/canary.go b/pkg/db/canary.go
index 407e14586..4a61f5055 100644
--- a/pkg/db/canary.go
+++ b/pkg/db/canary.go
@@ -449,11 +449,15 @@ func PersistCanaryModel(ctx context.Context, model pkg.Canary) (*pkg.Canary, boo
 
 	var changed bool
 	if existing.ID != uuid.Nil {
-		jsonDiff, err := diff.JSONCompare(string(model.Spec), string(existing.Spec))
-		if err != nil {
-			return nil, false, fmt.Errorf("failed to compare old and existing model")
+		if !utils.IsMapIdentical(model.Annotations, existing.Annotations) {
+			changed = true
+		} else {
+			jsonDiff, err := diff.JSONCompare(string(model.Spec), string(existing.Spec))
+			if err != nil {
+				return nil, false, fmt.Errorf("failed to compare old and existing model")
+			}
+			changed = jsonDiff != ""
 		}
-		changed = jsonDiff != ""
 	}
 
 	var oldCheckIDs []string
diff --git a/pkg/jobs/canary/canary_jobs.go b/pkg/jobs/canary/canary_jobs.go
index 891c708a7..feacbb19c 100644
--- a/pkg/jobs/canary/canary_jobs.go
+++ b/pkg/jobs/canary/canary_jobs.go
@@ -63,9 +63,10 @@ func (j CanaryJob) GetNamespacedName() types.NamespacedName {
 }
 
 func (j CanaryJob) Run(ctx dutyjob.JobRuntime) error {
-	if runner.IsCanaryIgnored(&j.Canary.ObjectMeta) {
+	if runner.IsCanarySuspended(&j.Canary.ObjectMeta) || runner.IsCanaryIgnored(&j.Canary.ObjectMeta) {
 		return nil
 	}
+
 	canaryID := j.DBCanary.ID.String()
 	ctx.History.ResourceID = canaryID
 	ctx.History.ResourceType = "canary"
diff --git a/pkg/jobs/canary/sync.go b/pkg/jobs/canary/sync.go
index d8e264ed6..af2b757fb 100644
--- a/pkg/jobs/canary/sync.go
+++ b/pkg/jobs/canary/sync.go
@@ -94,7 +94,7 @@ func SyncCanaryJob(ctx context.Context, dbCanary pkg.Canary) error {
 		return nil
 	}
 
-	if runner.IsCanaryIgnored(&canary.ObjectMeta) {
+	if runner.IsCanarySuspended(&canary.ObjectMeta) || runner.IsCanaryIgnored(&canary.ObjectMeta) {
 		Unschedule(id)
 		return nil
 	}
@@ -208,9 +208,10 @@ func ScanCanaryConfigs(ctx context.Context) {
 		}
 
 		for _, canary := range configs {
-			if runner.IsCanaryIgnored(&canary.ObjectMeta) {
+			if runner.IsCanarySuspended(&canary.ObjectMeta) || runner.IsCanaryIgnored(&canary.ObjectMeta) {
 				continue
 			}
+
 			_, _, err := db.PersistCanary(ctx, canary, path.Base(configfile))
 			if err != nil {
 				logger.Errorf("could not persist %s: %v", canary.Name, err)
diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go
index 19f4e3c12..0d4316bd3 100644
--- a/pkg/runner/runner.go
+++ b/pkg/runner/runner.go
@@ -46,5 +46,9 @@ func IsCanaryIgnored(canary *metav1.ObjectMeta) bool {
 		}
 	}
 
+	return false
+}
+
+func IsCanarySuspended(canary *metav1.ObjectMeta) bool {
 	return canary.Annotations != nil && canary.Annotations["suspend"] == "true"
 }
diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go
index 8f00ed6fc..0ca2df00d 100644
--- a/pkg/utils/utils.go
+++ b/pkg/utils/utils.go
@@ -152,3 +152,17 @@ func FreePort() int {
 	address := listener.Addr().(*net.TCPAddr)
 	return address.Port
 }
+
+func IsMapIdentical[K comparable](map1, map2 map[string]K) bool {
+	if len(map1) != len(map2) {
+		return false
+	}
+
+	for k, v1 := range map1 {
+		if v2, exists := map2[k]; !exists || v1 != v2 {
+			return false
+		}
+	}
+
+	return true
+}