Skip to content

Commit 47171f9

Browse files
corrierilucacorrieriluca
authored
fix(controller): plan and apply should use last branch commit (#948)
* fix(controller): include error in failed to get last result event * fix(controller): use LastBranchCommit when creating Plan/Apply run * fix(tests): replace relevant-commit with branch-commit * fix(tests): put a relevant-commit when needed
1 parent a0cadb7 commit 47171f9

8 files changed

Lines changed: 36 additions & 35 deletions

File tree

‎internal/controllers/terraformlayer/controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
117117
lastResult, err = r.Datastore.GetPlan(layer.Namespace, layer.Name, layer.Status.LastRun.Name, "", "short")
118118
if err != nil {
119119
log.Errorf("failed to get plan for layer %s: %s", layer.Name, err)
120-
r.Recorder.Event(layer, corev1.EventTypeNormal, "Reconciliation", "Failed to get last Result")
120+
r.Recorder.Eventf(layer, corev1.EventTypeNormal, "Reconciliation", "Failed to get last Result: %s", err)
121121
lastResult = []byte("Error getting last Result")
122122
}
123123
}

‎internal/controllers/terraformlayer/states.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,10 @@ func (s *PlanNeeded) getHandler() Handler {
7070
if isActionBlocked(r, layer, repository, syncwindow.PlanAction) {
7171
return ctrl.Result{RequeueAfter: r.Config.Controller.Timers.WaitAction}, nil
7272
}
73-
revision, ok := layer.Annotations[annotations.LastRelevantCommit]
73+
revision, ok := layer.Annotations[annotations.LastBranchCommit]
7474
if !ok {
75-
r.Recorder.Event(layer, corev1.EventTypeWarning, "Reconciliation", "Layer has no last relevant commit annotation, Plan run not created")
76-
log.Errorf("layer %s has no last relevant commit annotation, run not created", layer.Name)
75+
r.Recorder.Event(layer, corev1.EventTypeWarning, "Reconciliation", "Layer has no last branch commit annotation, Plan run not created")
76+
log.Errorf("layer %s has no last branch commit annotation, run not created", layer.Name)
7777
return ctrl.Result{RequeueAfter: r.Config.Controller.Timers.OnError}, nil
7878
}
7979
run := r.getRun(layer, revision, PlanAction)
@@ -102,10 +102,10 @@ func (s *ApplyNeeded) getHandler() Handler {
102102
if isActionBlocked(r, layer, repository, syncwindow.ApplyAction) {
103103
return ctrl.Result{RequeueAfter: r.Config.Controller.Timers.WaitAction}, nil
104104
}
105-
revision, ok := layer.Annotations[annotations.LastRelevantCommit]
105+
revision, ok := layer.Annotations[annotations.LastBranchCommit]
106106
if !ok {
107-
r.Recorder.Event(layer, corev1.EventTypeWarning, "Reconciliation", "Layer has no last relevant commit annotation, Apply run not created")
108-
log.Errorf("layer %s has no last relevant commit annotation, run not created", layer.Name)
107+
r.Recorder.Event(layer, corev1.EventTypeWarning, "Reconciliation", "Layer has no last branch commit annotation, Apply run not created")
108+
log.Errorf("layer %s has no last branch commit annotation, run not created", layer.Name)
109109
return ctrl.Result{RequeueAfter: r.Config.Controller.Timers.OnError}, nil
110110
}
111111
run := r.getRun(layer, revision, ApplyAction)

‎internal/controllers/terraformlayer/states_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ func TestPlanNeededEventIncludesCreateError(t *testing.T) {
3636
Name: "infra-s3-buckets-logistics-toolings-dc1-nl-prd-defect-tracker-prd-nl1",
3737
Namespace: "default",
3838
Annotations: map[string]string{
39-
annotations.LastRelevantCommit: "abc123",
39+
annotations.LastBranchCommit: "abc123",
4040
},
4141
},
4242
}
@@ -66,8 +66,8 @@ func TestApplyNeededEventIncludesCreateError(t *testing.T) {
6666
Name: "infra-s3-buckets-logistics-toolings-dc1-nl-prd-defect-tracker-prd-nl1",
6767
Namespace: "default",
6868
Annotations: map[string]string{
69-
annotations.LastRelevantCommit: "abc123",
70-
annotations.LastPlanRun: "plan-run/0",
69+
annotations.LastBranchCommit: "abc123",
70+
annotations.LastPlanRun: "plan-run/0",
7171
},
7272
},
7373
}

‎internal/controllers/terraformlayer/testdata/error-case.yaml‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ metadata:
1111
runner.terraform.padok.cloud/plan-date: Mon May 8 11:21:53 UTC 2023
1212
runner.terraform.padok.cloud/plan-run: "run-failed/0"
1313
runner.terraform.padok.cloud/plan-sum: ""
14-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
14+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
1515
spec:
1616
branch: main
1717
path: error-case-one/
@@ -41,7 +41,7 @@ metadata:
4141
runner.terraform.padok.cloud/apply-date: Mon May 8 11:21:53 UTC 2023
4242
runner.terraform.padok.cloud/apply-run: "run-failed/0"
4343
runner.terraform.padok.cloud/apply-sum: ""
44-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
44+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
4545
spec:
4646
branch: main
4747
path: error-case-two/
@@ -68,7 +68,7 @@ metadata:
6868
runner.terraform.padok.cloud/plan-date: Sun May 7 11:21:53 UTC 2023
6969
runner.terraform.padok.cloud/plan-run: "run-failed/0"
7070
runner.terraform.padok.cloud/plan-sum: ""
71-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
71+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
7272
spec:
7373
branch: main
7474
path: error-case-three/
@@ -100,7 +100,7 @@ metadata:
100100
runner.terraform.padok.cloud/apply-commit: 840046e9db8c1348445d0018c86347967b066df0
101101
runner.terraform.padok.cloud/apply-date: Mon May 8 11:20:53 UTC 2023
102102
runner.terraform.padok.cloud/apply-sum: ""
103-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
103+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
104104
spec:
105105
branch: main
106106
path: error-case-four/
@@ -136,7 +136,7 @@ metadata:
136136
runner.terraform.padok.cloud/apply-commit: 840046e9db8c1348445d0018c86347967b066df0
137137
runner.terraform.padok.cloud/apply-date: Mon May 8 11:20:53 UTC 2023
138138
runner.terraform.padok.cloud/apply-sum: ""
139-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
139+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
140140
spec:
141141
branch: main
142142
path: error-case-five/
@@ -185,7 +185,7 @@ metadata:
185185
runner.terraform.padok.cloud/plan-commit: ca9b6c80ac8fb5cd837ae9b374b79ff33f472558
186186
runner.terraform.padok.cloud/plan-date: Sun May 7 11:21:53 UTC 2023
187187
runner.terraform.padok.cloud/plan-sum: ""
188-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
188+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
189189
spec:
190190
branch: main
191191
path: error-case-seven/
@@ -211,6 +211,7 @@ metadata:
211211
runner.terraform.padok.cloud/plan-commit: ca9b6c80ac8fb5cd837ae9b374b79ff33f472558
212212
runner.terraform.padok.cloud/plan-date: Sun May 7 11:21:53 UTC 2023
213213
runner.terraform.padok.cloud/plan-sum: ""
214+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
214215
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
215216
spec:
216217
branch: main
@@ -240,7 +241,7 @@ metadata:
240241
runner.terraform.padok.cloud/apply-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
241242
runner.terraform.padok.cloud/apply-date: Mon May 8 11:20:53 UTC 2023
242243
runner.terraform.padok.cloud/apply-sum: ""
243-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
244+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
244245
spec:
245246
branch: main
246247
path: error-case-nine/

‎internal/controllers/terraformlayer/testdata/merge-case.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ metadata:
44
name: merge-case-1
55
namespace: default
66
annotations:
7-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
7+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
88
spec:
99
branch: main
1010
path: merge-case-one/

‎internal/controllers/terraformlayer/testdata/nominal-case.yaml‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ metadata:
44
name: nominal-case-1
55
namespace: default
66
annotations:
7-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
7+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
88
spec:
99
branch: main
1010
path: nominal-case-one/
@@ -30,7 +30,7 @@ metadata:
3030
runner.terraform.padok.cloud/plan-date: Mon May 8 11:21:53 UTC 2023
3131
runner.terraform.padok.cloud/plan-run: run-succeeded/0
3232
runner.terraform.padok.cloud/plan-sum: AuP6pMNxWsbSZKnxZvxD842wy0qaF9JCX8HW1nFeL1I=
33-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
33+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
3434
spec:
3535
branch: main
3636
path: nominal-case-two/
@@ -60,7 +60,7 @@ metadata:
6060
runner.terraform.padok.cloud/plan-date: Mon May 8 11:21:53 UTC 2023
6161
runner.terraform.padok.cloud/plan-run: run-succeeded/0
6262
runner.terraform.padok.cloud/plan-sum: AuP6pMNxWsbSZKnxZvxD842wy0qaF9JCX8HW1nFeL1I=
63-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
63+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
6464
spec:
6565
branch: main
6666
path: nominal-case-three/
@@ -95,7 +95,7 @@ metadata:
9595
runner.terraform.padok.cloud/apply-commit: ca9b6c80ac8fb5cd837ae9b374b79ff33f472558
9696
runner.terraform.padok.cloud/apply-date: Mon May 8 11:21:53 UTC 2023
9797
runner.terraform.padok.cloud/apply-sum: AuP6pMNxWsbSZKnxZvxD842wy0qaF9JCX8HW1nFeL1I=
98-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
98+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
9999
spec:
100100
branch: main
101101
path: nominal-case-four/
@@ -117,7 +117,7 @@ metadata:
117117
name: nominal-case-5
118118
namespace: default
119119
annotations:
120-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
120+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
121121
spec:
122122
branch: main
123123
path: shared-path/
@@ -139,7 +139,7 @@ metadata:
139139
name: nominal-case-5-shared-path
140140
namespace: default
141141
annotations:
142-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
142+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
143143
spec:
144144
branch: main
145145
path: shared-path/
@@ -178,7 +178,7 @@ metadata:
178178
runner.terraform.padok.cloud/apply-commit: ca9b6c80ac8fb5cd837ae9b374b79ff33f472558
179179
runner.terraform.padok.cloud/apply-date: Mon May 8 10:21:53 UTC 2023
180180
runner.terraform.padok.cloud/apply-sum: AuP6pMNxWsbSZKnxZvxD842wy0qaF9JCX8HW1nFeL1I=
181-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
181+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
182182
spec:
183183
branch: main
184184
path: nominal-case-six/
@@ -202,7 +202,7 @@ metadata:
202202
name: nominal-case-7
203203
namespace: default
204204
annotations:
205-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
205+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
206206
spec:
207207
branch: main
208208
path: nominal-case-seven/
@@ -229,7 +229,7 @@ metadata:
229229
app.kubernetes.io/instance: in-cluster-burrito
230230
annotations:
231231
api.terraform.padok.cloud/sync-now: "true"
232-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
232+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
233233
name: nominal-case-8
234234
namespace: default
235235
spec:

‎internal/controllers/terraformlayer/testdata/sync-window-case.yaml‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ metadata:
2525
runner.terraform.padok.cloud/plan-date: Mon May 8 11:21:53 UTC 2023
2626
runner.terraform.padok.cloud/plan-run: run-succeeded/0
2727
runner.terraform.padok.cloud/plan-sum: AuP6pMNxWsbSZKnxZvxD842wy0qaF9JCX8HW1nFeL1I=
28-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
28+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
2929
spec:
3030
branch: main
3131
path: sync-window-case-one/
@@ -69,7 +69,7 @@ metadata:
6969
runner.terraform.padok.cloud/plan-date: Mon May 8 11:21:53 UTC 2023
7070
runner.terraform.padok.cloud/plan-run: run-succeeded/0
7171
runner.terraform.padok.cloud/plan-sum: AuP6pMNxWsbSZKnxZvxD842wy0qaF9JCX8HW1nFeL1I=
72-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
72+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
7373
spec:
7474
branch: main
7575
path: sync-window-case-one/
@@ -109,7 +109,7 @@ metadata:
109109
name: sync-window-case-deny-plan-1
110110
namespace: default
111111
annotations:
112-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
112+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
113113
spec:
114114
branch: main
115115
path: sync-window-case-one/
@@ -132,7 +132,7 @@ metadata:
132132
name: sync-window-repo-allow-plan
133133
namespace: default
134134
annotations:
135-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
135+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
136136
spec:
137137
repository:
138138
url: git@github.com:padok-team/burrito-examples.git
@@ -151,7 +151,7 @@ metadata:
151151
name: sync-window-case-allow-plan-1
152152
namespace: default
153153
annotations:
154-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
154+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
155155
spec:
156156
branch: main
157157
path: sync-window-case-one/
@@ -189,7 +189,7 @@ metadata:
189189
runner.terraform.padok.cloud/plan-date: Mon May 8 11:21:53 UTC 2023
190190
runner.terraform.padok.cloud/plan-run: run-succeeded/0
191191
runner.terraform.padok.cloud/plan-sum: AuP6pMNxWsbSZKnxZvxD842wy0qaF9JCX8HW1nFeL1I=
192-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
192+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
193193
spec:
194194
branch: main
195195
path: sync-window-case-one/

‎internal/controllers/terraformlayer/testdata/unknown-cases.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ metadata:
55
name: non-existent-repository
66
namespace: default
77
annotations:
8-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
8+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
99
spec:
1010
branch: main
1111
path: nominal-case-one/
@@ -28,7 +28,7 @@ metadata:
2828
name: last-run-not-exist
2929
namespace: default
3030
annotations:
31-
webhook.terraform.padok.cloud/relevant-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
31+
webhook.terraform.padok.cloud/branch-commit: cb9f15b90861c8c4364cdde63d17837c7a9ccca9
3232
spec:
3333
branch: main
3434
path: last-run-not-exist/

0 commit comments

Comments
 (0)