Skip to content

Commit 6b89d56

Browse files
stefanprodanstefanprodan
authored
Merge pull request #1792 from steved/steved/phase-succeeded-webhook
fix: send succeeded webhooks with correct phase
2 parents ae72e15 + 6fcbe19 commit 6b89d56

File tree

2 files changed

+252
-5
lines changed

2 files changed

+252
-5
lines changed

‎pkg/controller/events_test.go‎

Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,239 @@
1+
package controller
2+
3+
import (
4+
"encoding/json"
5+
"net/http"
6+
"net/http/httptest"
7+
"sync"
8+
"testing"
9+
10+
flaggerv1 "github.com/fluxcd/flagger/pkg/apis/flagger/v1beta1"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
14+
)
15+
16+
func TestWebhooks(t *testing.T) {
17+
notReady := flaggerv1.CanaryWebhookPayload{
18+
Metadata: map[string]string{
19+
"eventMessage": "podinfo-primary.default not ready: waiting for rollout to finish: 0 out of 1 new replicas have been updated",
20+
},
21+
}
22+
metricsInitialized := flaggerv1.CanaryWebhookPayload{
23+
Metadata: map[string]string{
24+
"eventMessage": "all the metrics providers are available!",
25+
},
26+
}
27+
28+
tc := []struct {
29+
name string
30+
expectedEvents map[string][]flaggerv1.CanaryWebhookPayload
31+
test func(t *testing.T, mock fixture)
32+
}{
33+
{
34+
name: "skip-analysis",
35+
expectedEvents: map[string][]flaggerv1.CanaryWebhookPayload{
36+
"event": {
37+
metricsInitialized,
38+
notReady,
39+
metricsInitialized,
40+
{Phase: flaggerv1.CanaryPhaseInitialized, Metadata: map[string]string{"eventMessage": "Initialization done! podinfo.default"}},
41+
{Phase: flaggerv1.CanaryPhaseInitialized, Metadata: map[string]string{"eventMessage": "Confirm-rollout check confirm-rollout-webhook passed"}},
42+
{Phase: flaggerv1.CanaryPhaseProgressing, Metadata: map[string]string{"eventMessage": "New revision detected! Scaling up podinfo.default"}},
43+
{Phase: flaggerv1.CanaryPhaseProgressing, Metadata: map[string]string{"eventMessage": "Copying podinfo.default template spec to podinfo-primary.default"}},
44+
{Phase: flaggerv1.CanaryPhaseSucceeded, Metadata: map[string]string{"eventMessage": "Promotion completed! Canary analysis was skipped for podinfo.default"}},
45+
},
46+
"confirm-rollout": {
47+
{Phase: flaggerv1.CanaryPhaseInitialized, Metadata: map[string]string{"eventMessage": ""}},
48+
},
49+
},
50+
test: func(t *testing.T, mocks fixture) {
51+
mocks.ctrl.advanceCanary("podinfo", "default")
52+
mocks.makePrimaryReady(t)
53+
mocks.ctrl.advanceCanary("podinfo", "default")
54+
55+
// enable skip
56+
cd, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get(t.Context(), "podinfo", metav1.GetOptions{})
57+
require.NoError(t, err)
58+
cd.Spec.SkipAnalysis = true
59+
_, err = mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Update(t.Context(), cd, metav1.UpdateOptions{})
60+
require.NoError(t, err)
61+
62+
// update
63+
dep2 := newDeploymentTestDeploymentV2()
64+
_, err = mocks.kubeClient.AppsV1().Deployments("default").Update(t.Context(), dep2, metav1.UpdateOptions{})
65+
require.NoError(t, err)
66+
67+
// detect changes
68+
mocks.ctrl.advanceCanary("podinfo", "default")
69+
mocks.makeCanaryReady(t)
70+
71+
// advance
72+
mocks.ctrl.advanceCanary("podinfo", "default")
73+
},
74+
},
75+
{
76+
name: "canary",
77+
expectedEvents: map[string][]flaggerv1.CanaryWebhookPayload{
78+
"event": {
79+
metricsInitialized,
80+
notReady,
81+
metricsInitialized,
82+
{Phase: flaggerv1.CanaryPhaseInitialized, Metadata: map[string]string{"eventMessage": "Initialization done! podinfo.default"}},
83+
{Phase: flaggerv1.CanaryPhaseInitialized, Metadata: map[string]string{"eventMessage": "Confirm-rollout check confirm-rollout-webhook passed"}},
84+
{Phase: flaggerv1.CanaryPhaseProgressing, Metadata: map[string]string{"eventMessage": "New revision detected! Scaling up podinfo.default"}},
85+
{Phase: flaggerv1.CanaryPhaseProgressing, Metadata: map[string]string{"eventMessage": "Starting canary analysis for podinfo.default"}},
86+
{Phase: flaggerv1.CanaryPhaseProgressing, Metadata: map[string]string{"eventMessage": "Pre-rollout check pre-rollout-webhook passed"}},
87+
{Phase: flaggerv1.CanaryPhaseProgressing, Metadata: map[string]string{"eventMessage": "Confirm-traffic-increase check confirm-traffic-increase-webhook passed"}},
88+
{Phase: flaggerv1.CanaryPhaseProgressing, Metadata: map[string]string{"eventMessage": "Advance podinfo.default canary weight 100"}},
89+
{Phase: flaggerv1.CanaryPhaseProgressing, Metadata: map[string]string{"eventMessage": "Confirm-traffic-increase check confirm-traffic-increase-webhook passed"}},
90+
{Phase: flaggerv1.CanaryPhaseProgressing, Metadata: map[string]string{"eventMessage": "Confirm-promotion check confirm-promotion-webhook passed"}},
91+
{Phase: flaggerv1.CanaryPhaseProgressing, Metadata: map[string]string{"eventMessage": "Copying podinfo.default template spec to podinfo-primary.default"}},
92+
{Phase: flaggerv1.CanaryPhasePromoting, Metadata: map[string]string{"eventMessage": "Advance podinfo.default primary weight 50"}},
93+
{Phase: flaggerv1.CanaryPhasePromoting, Metadata: map[string]string{"eventMessage": "Advance podinfo.default primary weight 100"}},
94+
{Phase: flaggerv1.CanaryPhaseSucceeded, Metadata: map[string]string{"eventMessage": "Post-rollout check post-rollout-webhook passed"}},
95+
{Phase: flaggerv1.CanaryPhaseSucceeded, Metadata: map[string]string{"eventMessage": "Promotion completed! Scaling down podinfo.default"}},
96+
},
97+
"confirm-rollout": {
98+
{Phase: flaggerv1.CanaryPhaseInitialized, Metadata: map[string]string{"eventMessage": ""}},
99+
},
100+
"confirm-promotion": {
101+
{Phase: flaggerv1.CanaryPhaseProgressing, Metadata: map[string]string{"eventMessage": ""}},
102+
},
103+
"confirm-traffic-increase": {
104+
{Phase: flaggerv1.CanaryPhaseProgressing, Metadata: map[string]string{"eventMessage": ""}},
105+
{Phase: flaggerv1.CanaryPhaseProgressing, Metadata: map[string]string{"eventMessage": ""}},
106+
},
107+
"pre-rollout": {
108+
{Phase: flaggerv1.CanaryPhaseProgressing, Metadata: map[string]string{"eventMessage": ""}},
109+
},
110+
"post-rollout": {
111+
{Phase: flaggerv1.CanaryPhaseSucceeded, Metadata: map[string]string{"eventMessage": ""}},
112+
},
113+
"rollout": {
114+
{Phase: flaggerv1.CanaryPhaseProgressing, Metadata: map[string]string{"eventMessage": ""}},
115+
},
116+
},
117+
test: func(t *testing.T, mocks fixture) {
118+
mocks.canary.Spec.Analysis.Interval = "1m"
119+
mocks.canary.Spec.Analysis.StepWeight = 100
120+
mocks.canary.Spec.Analysis.StepWeightPromotion = 50
121+
_, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Update(t.Context(), mocks.canary, metav1.UpdateOptions{})
122+
require.NoError(t, err)
123+
124+
// initializing
125+
mocks.ctrl.advanceCanary("podinfo", "default")
126+
127+
// make primary ready
128+
mocks.makePrimaryReady(t)
129+
130+
// initialized
131+
mocks.ctrl.advanceCanary("podinfo", "default")
132+
require.NoError(t, assertPhase(mocks.flaggerClient, "podinfo", flaggerv1.CanaryPhaseInitialized))
133+
134+
// update
135+
dep2 := newDeploymentTestDeploymentV2()
136+
_, err = mocks.kubeClient.AppsV1().Deployments("default").Update(t.Context(), dep2, metav1.UpdateOptions{})
137+
require.NoError(t, err)
138+
139+
// detect changes
140+
mocks.ctrl.advanceCanary("podinfo", "default")
141+
require.NoError(t, assertPhase(mocks.flaggerClient, "podinfo", flaggerv1.CanaryPhaseProgressing))
142+
mocks.makeCanaryReady(t)
143+
144+
// progressing
145+
mocks.ctrl.advanceCanary("podinfo", "default")
146+
require.NoError(t, assertPhase(mocks.flaggerClient, "podinfo", flaggerv1.CanaryPhaseProgressing))
147+
148+
// start promotion
149+
mocks.ctrl.advanceCanary("podinfo", "default")
150+
require.NoError(t, assertPhase(mocks.flaggerClient, "podinfo", flaggerv1.CanaryPhasePromoting))
151+
152+
// end promotion
153+
mocks.ctrl.advanceCanary("podinfo", "default")
154+
require.NoError(t, assertPhase(mocks.flaggerClient, "podinfo", flaggerv1.CanaryPhasePromoting))
155+
156+
// finalising
157+
mocks.ctrl.advanceCanary("podinfo", "default")
158+
require.NoError(t, assertPhase(mocks.flaggerClient, "podinfo", flaggerv1.CanaryPhaseFinalising))
159+
160+
// succeeded
161+
mocks.ctrl.advanceCanary("podinfo", "default")
162+
require.NoError(t, assertPhase(mocks.flaggerClient, "podinfo", flaggerv1.CanaryPhaseSucceeded))
163+
},
164+
},
165+
}
166+
167+
for _, tt := range tc {
168+
t.Run(tt.name, func(t *testing.T) {
169+
var mu sync.Mutex
170+
events := map[string][]flaggerv1.CanaryWebhookPayload{}
171+
172+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
173+
mu.Lock()
174+
defer mu.Unlock()
175+
176+
var payload flaggerv1.CanaryWebhookPayload
177+
err := json.NewDecoder(r.Body).Decode(&payload)
178+
require.NoError(t, err)
179+
180+
// only record the event attributes we're comparing
181+
eventType := r.URL.Path[1:]
182+
events[eventType] = append(events[eventType], flaggerv1.CanaryWebhookPayload{
183+
Phase: payload.Phase,
184+
Metadata: map[string]string{
185+
"eventMessage": payload.Metadata["eventMessage"],
186+
},
187+
})
188+
189+
w.WriteHeader(http.StatusOK)
190+
}))
191+
192+
defer server.Close()
193+
194+
c := newDeploymentTestCanary()
195+
c.Spec.Analysis.Webhooks = []flaggerv1.CanaryWebhook{
196+
{
197+
Type: flaggerv1.EventHook,
198+
Name: "event-webhook",
199+
URL: server.URL + "/event",
200+
},
201+
{
202+
Type: flaggerv1.PreRolloutHook,
203+
Name: "pre-rollout-webhook",
204+
URL: server.URL + "/pre-rollout",
205+
},
206+
{
207+
Type: flaggerv1.RolloutHook,
208+
Name: "rollout-webhook",
209+
URL: server.URL + "/rollout",
210+
},
211+
{
212+
Type: flaggerv1.ConfirmPromotionHook,
213+
Name: "confirm-promotion-webhook",
214+
URL: server.URL + "/confirm-promotion",
215+
},
216+
{
217+
Type: flaggerv1.ConfirmRolloutHook,
218+
Name: "confirm-rollout-webhook",
219+
URL: server.URL + "/confirm-rollout",
220+
},
221+
{
222+
Type: flaggerv1.PostRolloutHook,
223+
Name: "post-rollout-webhook",
224+
URL: server.URL + "/post-rollout",
225+
},
226+
{
227+
Type: flaggerv1.ConfirmTrafficIncreaseHook,
228+
Name: "confirm-traffic-increase-webhook",
229+
URL: server.URL + "/confirm-traffic-increase",
230+
},
231+
}
232+
233+
mocks := newDeploymentFixture(c)
234+
tt.test(t, mocks)
235+
236+
assert.Equal(t, tt.expectedEvents, events)
237+
})
238+
}
239+
}

‎pkg/controller/scheduler.go‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -400,17 +400,22 @@ func (c *Controller) advanceCanary(name string, namespace string) {
400400
c.recordEventWarningf(cd, "%v", err)
401401
return
402402
}
403+
403404
c.recorder.SetStatus(cd, flaggerv1.CanaryPhaseSucceeded)
404405
c.recorder.IncSuccesses(metrics.CanaryMetricLabels{
405406
Name: cd.Spec.TargetRef.Name,
406407
Namespace: cd.Namespace,
407408
DeploymentStrategy: cd.DeploymentStrategy(),
408409
AnalysisStatus: metrics.AnalysisStatusCompleted,
409410
})
410-
c.runPostRolloutHooks(cd, flaggerv1.CanaryPhaseSucceeded)
411-
c.recordEventInfof(cd, "Promotion completed! Scaling down %s.%s", cd.Spec.TargetRef.Name, cd.Namespace)
412-
c.alert(cd, "Canary analysis completed successfully, promotion finished.",
411+
412+
canarySucceeded := cd.DeepCopy()
413+
canarySucceeded.Status.Phase = flaggerv1.CanaryPhaseSucceeded
414+
c.runPostRolloutHooks(canarySucceeded, flaggerv1.CanaryPhaseSucceeded)
415+
c.recordEventInfof(canarySucceeded, "Promotion completed! Scaling down %s.%s", cd.Spec.TargetRef.Name, cd.Namespace)
416+
c.alert(canarySucceeded, "Canary analysis completed successfully, promotion finished.",
413417
false, flaggerv1.SeverityInfo)
418+
414419
return
415420
}
416421

@@ -825,9 +830,12 @@ func (c *Controller) shouldSkipAnalysis(canary *flaggerv1.Canary, canaryControll
825830
DeploymentStrategy: canary.DeploymentStrategy(),
826831
AnalysisStatus: metrics.AnalysisStatusSkipped,
827832
})
828-
c.recordEventInfof(canary, "Promotion completed! Canary analysis was skipped for %s.%s",
833+
834+
canarySucceeded := canary.DeepCopy()
835+
canarySucceeded.Status.Phase = flaggerv1.CanaryPhaseSucceeded
836+
c.recordEventInfof(canarySucceeded, "Promotion completed! Canary analysis was skipped for %s.%s",
829837
canary.Spec.TargetRef.Name, canary.Namespace)
830-
c.alert(canary, "Canary analysis was skipped, promotion finished.",
838+
c.alert(canarySucceeded, "Canary analysis was skipped, promotion finished.",
831839
false, flaggerv1.SeverityInfo)
832840

833841
return true

0 commit comments

Comments
 (0)