Skip to content

Commit f9a2dbf

Browse files
ShmuelOpsclaude
andcommitted
fix: reconcile PrefectAutomation every pass for continuous self-heal
UpdateAutomationStatus stamped Status.LastSyncTime, so needsSync short-circuited on a 10-minute drift timer and didn't re-check Prefect in between. A manual edit or delete of the automation in Prefect wasn't reconciled until the drift window elapsed or the operator restarted. PrefectDeployment never persists LastSyncTime, so needsSync's nil check makes it reconcile against Prefect every pass and self-heal continuously. Match that: stop stamping LastSyncTime on PrefectAutomation (needsSync is otherwise unchanged). syncWithPrefect is idempotent, so re-running it each pass is safe. Tests: UpdateAutomationStatus leaves LastSyncTime nil; needsSync returns true in steady state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent fa792dc commit f9a2dbf

3 files changed

Lines changed: 95 additions & 3 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
Copyright 2024.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package controller
18+
19+
import (
20+
"testing"
21+
22+
prefectiov1 "github.com/PrefectHQ/prefect-operator/api/v1"
23+
)
24+
25+
// UpdateAutomationStatus never stamps LastSyncTime, so needsSync sees a nil
26+
// LastSyncTime on every pass and returns true — the controller reconciles
27+
// against Prefect each loop and self-heals an out-of-band edit/delete within one
28+
// requeue interval, matching PrefectDeployment's continuous reconcile.
29+
func TestAutomationNeedsSync(t *testing.T) {
30+
r := &PrefectAutomationReconciler{}
31+
id := "automation-1"
32+
hash := "hash-1"
33+
34+
// Steady state as persisted by UpdateAutomationStatus: id + spec hash +
35+
// observed generation set, but LastSyncTime left nil.
36+
steady := &prefectiov1.PrefectAutomation{}
37+
steady.Generation = 2
38+
steady.Status.Id = &id
39+
steady.Status.SpecHash = hash
40+
steady.Status.ObservedGeneration = 2
41+
42+
if !r.needsSync(steady, hash) {
43+
t.Fatal("needsSync = false in steady state; want true so out-of-band changes self-heal every loop")
44+
}
45+
}

internal/prefect/convert_automation.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"fmt"
2222
"strconv"
2323

24-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2524
"k8s.io/apimachinery/pkg/runtime"
2625

2726
prefectiov1 "github.com/PrefectHQ/prefect-operator/api/v1"
@@ -304,10 +303,16 @@ func nonNilStrings(s []string) []string {
304303
}
305304

306305
// UpdateAutomationStatus updates the K8s PrefectAutomation status from an API Automation.
306+
//
307+
// LastSyncTime is deliberately NOT stamped: needsSync treats a nil LastSyncTime
308+
// as "sync now", so leaving it unset makes the controller reconcile against
309+
// Prefect on every pass and self-heal an out-of-band edit/delete within one
310+
// requeue interval — matching PrefectDeployment, whose status likewise carries
311+
// no LastSyncTime. Persisting it here previously gated re-sync behind a 10-minute
312+
// drift window, so a manual change in Prefect wasn't corrected until then (or an
313+
// operator restart).
307314
func UpdateAutomationStatus(k8sAutomation *prefectiov1.PrefectAutomation, automation *Automation) {
308315
id := automation.ID
309316
k8sAutomation.Status.Id = &id
310317
k8sAutomation.Status.Ready = true
311-
now := metav1.Now()
312-
k8sAutomation.Status.LastSyncTime = &now
313318
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
Copyright 2024.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package prefect
18+
19+
import (
20+
"testing"
21+
22+
prefectiov1 "github.com/PrefectHQ/prefect-operator/api/v1"
23+
)
24+
25+
// UpdateAutomationStatus must set Id/Ready but leave LastSyncTime nil: needsSync
26+
// treats a nil LastSyncTime as "sync now", so this is what makes the controller
27+
// reconcile against Prefect every pass and self-heal an out-of-band edit/delete
28+
// (parity with PrefectDeployment, whose status also carries no LastSyncTime).
29+
func TestUpdateAutomationStatusLeavesLastSyncTimeNil(t *testing.T) {
30+
k8s := &prefectiov1.PrefectAutomation{}
31+
UpdateAutomationStatus(k8s, &Automation{ID: "abc"})
32+
33+
if k8s.Status.Id == nil || *k8s.Status.Id != "abc" {
34+
t.Fatalf("Status.Id = %v, want \"abc\"", k8s.Status.Id)
35+
}
36+
if !k8s.Status.Ready {
37+
t.Fatal("Status.Ready = false, want true")
38+
}
39+
if k8s.Status.LastSyncTime != nil {
40+
t.Fatal("Status.LastSyncTime must stay nil so the controller re-syncs every reconcile")
41+
}
42+
}

0 commit comments

Comments
 (0)