Skip to content

Commit

Permalink
fix: canary suspension
Browse files Browse the repository at this point in the history
  • Loading branch information
adityathebe authored and moshloop committed Oct 9, 2024
1 parent 5146347 commit 217e8de
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 14 deletions.
28 changes: 21 additions & 7 deletions pkg/controllers/canary_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,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)
Expand Down Expand Up @@ -203,14 +204,9 @@ func (r *CanaryReconciler) updateCanaryInDB(ctx dutyContext.Context, canary *v1.
dbCanary = cacheObj.(*pkg.Canary)
}

// Compare canary spec and spec in database
// If they do not match, persist the canary in database
canarySpecJSON, err := json.Marshal(canary.Spec)
if err != nil {
if changed, err := hasCanaryChanged(canary, dbCanary); err != nil {
return nil, err
}
opts := jsondiff.DefaultJSONOptions()
if diff, _ := jsondiff.Compare(canarySpecJSON, dbCanary.Spec, &opts); diff != jsondiff.FullMatch {
} else if changed {
dbCanary, err = r.persistAndCacheCanary(ctx, canary)
if err != nil {
return nil, err
Expand All @@ -220,6 +216,24 @@ func (r *CanaryReconciler) updateCanaryInDB(ctx dutyContext.Context, canary *v1.
return dbCanary, nil
}

func hasCanaryChanged(canary *v1.Canary, dbCanary *pkg.Canary) (bool, error) {
if !utils.IsMapIdentical(canary.Annotations, dbCanary.Annotations) {
return true, nil
}

// Compare canary spec and spec in database
// If they do not match, persist the canary in database
canarySpecJSON, err := json.Marshal(canary.Spec)
if err != nil {
return false, err
}

opts := jsondiff.DefaultJSONOptions()
diff, _ := jsondiff.Compare(canarySpecJSON, dbCanary.Spec, &opts)
specChanged := diff != jsondiff.FullMatch
return specChanged, nil
}

func (r *CanaryReconciler) Report() {
for payload := range canaryJobs.CanaryStatusChannel {
var canary v1.Canary
Expand Down
12 changes: 8 additions & 4 deletions pkg/db/canary.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion pkg/jobs/canary/canary_jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 3 additions & 2 deletions pkg/jobs/canary/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions pkg/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
14 changes: 14 additions & 0 deletions pkg/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,3 +172,17 @@ func ParseTime(t string) *time.Time {

return nil
}

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
}

0 comments on commit 217e8de

Please sign in to comment.