Skip to content

Commit 24b40e3

Browse files
refactor: use a pointer for field skipAnalysis in CanarySpec
This refactor tries to fulfil a special use case fluxcd#1660: when a custom controller uses the Flagger API to render the Canary object to json/yaml, it makes sure the skipAnalysis field is rendered when the value is "false", so that the field is always communicated to the k8s API server. Comparison is as follows: before, when skipAnalysis is "false" the canary object is rendered as such: ``` apiVersion: flagger.app/v1beta1 kind: Canary ... spec: analysis: ... ``` After, it is as such: ``` apiVersion: flagger.app/v1beta1 kind: Canary ... spec: skipAnalysis: false # this is the field we expected analysis: ... ```
1 parent 133fdec commit 24b40e3

File tree

3 files changed

+13
-6
lines changed

3 files changed

+13
-6
lines changed

pkg/apis/flagger/v1beta1/canary.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ type CanarySpec struct {
105105

106106
// SkipAnalysis promotes the canary without analysing it
107107
// +optional
108-
SkipAnalysis bool `json:"skipAnalysis,omitempty"`
108+
SkipAnalysis *bool `json:"skipAnalysis,omitempty"`
109109

110110
// revert canary mutation on deletion of canary resource
111111
// +optional
@@ -615,8 +615,13 @@ func (c *Canary) GetMetricInterval() string {
615615
// SkipAnalysis returns true if the analysis is nil
616616
// or if spec.SkipAnalysis is true
617617
func (c *Canary) SkipAnalysis() bool {
618+
if c.Spec.SkipAnalysis == nil {
619+
return false
620+
}
621+
618622
if c.Spec.Analysis == nil && c.Spec.CanaryAnalysis == nil {
619623
return true
620624
}
621-
return c.Spec.SkipAnalysis
625+
626+
return *c.Spec.SkipAnalysis
622627
}

pkg/controller/scheduler_daemonset_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,8 @@ func TestScheduler_DaemonSetSkipAnalysis(t *testing.T) {
110110
cd, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get(context.TODO(), "podinfo", metav1.GetOptions{})
111111
require.NoError(t, err)
112112

113-
cd.Spec.SkipAnalysis = true
113+
skipAnalysis := true
114+
cd.Spec.SkipAnalysis = &skipAnalysis
114115
_, err = mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Update(context.TODO(), cd, metav1.UpdateOptions{})
115116
require.NoError(t, err)
116117

@@ -126,7 +127,7 @@ func TestScheduler_DaemonSetSkipAnalysis(t *testing.T) {
126127

127128
c, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get(context.TODO(), "podinfo", metav1.GetOptions{})
128129
require.NoError(t, err)
129-
assert.True(t, c.Spec.SkipAnalysis)
130+
assert.True(t, c.SkipAnalysis())
130131
assert.Equal(t, flaggerv1.CanaryPhaseSucceeded, c.Status.Phase)
131132
}
132133

pkg/controller/scheduler_deployment_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,8 @@ func TestScheduler_DeploymentSkipAnalysis(t *testing.T) {
130130
// enable skip
131131
cd, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get(context.TODO(), "podinfo", metav1.GetOptions{})
132132
require.NoError(t, err)
133-
cd.Spec.SkipAnalysis = true
133+
skipAnalysis := true
134+
cd.Spec.SkipAnalysis = &skipAnalysis
134135
_, err = mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Update(context.TODO(), cd, metav1.UpdateOptions{})
135136
require.NoError(t, err)
136137

@@ -148,7 +149,7 @@ func TestScheduler_DeploymentSkipAnalysis(t *testing.T) {
148149

149150
c, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get(context.TODO(), "podinfo", metav1.GetOptions{})
150151
require.NoError(t, err)
151-
assert.True(t, c.Spec.SkipAnalysis)
152+
assert.True(t, c.SkipAnalysis())
152153
assert.Equal(t, flaggerv1.CanaryPhaseSucceeded, c.Status.Phase)
153154
}
154155

0 commit comments

Comments
 (0)