Skip to content

Commit

Permalink
refactor: use a pointer for field: skipAnalysis in Canary object
Browse files Browse the repository at this point in the history
This refactor tries to fulfill a special use case {ticket number}: when a custom
controller uses the Flagger API to create/update a Canary object using
k8s client-go:

This makes sure that the skipAnalysis field is always rendered after
marshalling to json format.
  • Loading branch information
bruce-personio committed Jun 13, 2024
1 parent 133fdec commit f1ef8eb
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 6 deletions.
9 changes: 7 additions & 2 deletions pkg/apis/flagger/v1beta1/canary.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ type CanarySpec struct {

// SkipAnalysis promotes the canary without analysing it
// +optional
SkipAnalysis bool `json:"skipAnalysis,omitempty"`
SkipAnalysis *bool `json:"skipAnalysis,omitempty"`

// revert canary mutation on deletion of canary resource
// +optional
Expand Down Expand Up @@ -615,8 +615,13 @@ func (c *Canary) GetMetricInterval() string {
// SkipAnalysis returns true if the analysis is nil
// or if spec.SkipAnalysis is true
func (c *Canary) SkipAnalysis() bool {
if c.Spec.SkipAnalysis == nil {
return false
}

if c.Spec.Analysis == nil && c.Spec.CanaryAnalysis == nil {
return true
}
return c.Spec.SkipAnalysis

return *c.Spec.SkipAnalysis
}
5 changes: 3 additions & 2 deletions pkg/controller/scheduler_daemonset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ func TestScheduler_DaemonSetSkipAnalysis(t *testing.T) {
cd, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get(context.TODO(), "podinfo", metav1.GetOptions{})
require.NoError(t, err)

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

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

c, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get(context.TODO(), "podinfo", metav1.GetOptions{})
require.NoError(t, err)
assert.True(t, c.Spec.SkipAnalysis)
assert.True(t, c.SkipAnalysis())
assert.Equal(t, flaggerv1.CanaryPhaseSucceeded, c.Status.Phase)
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/controller/scheduler_deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ func TestScheduler_DeploymentSkipAnalysis(t *testing.T) {
// enable skip
cd, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get(context.TODO(), "podinfo", metav1.GetOptions{})
require.NoError(t, err)
cd.Spec.SkipAnalysis = true
skipAnalysis := true
cd.Spec.SkipAnalysis = &skipAnalysis
_, err = mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Update(context.TODO(), cd, metav1.UpdateOptions{})
require.NoError(t, err)

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

c, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get(context.TODO(), "podinfo", metav1.GetOptions{})
require.NoError(t, err)
assert.True(t, c.Spec.SkipAnalysis)
assert.True(t, c.SkipAnalysis())
assert.Equal(t, flaggerv1.CanaryPhaseSucceeded, c.Status.Phase)
}

Expand Down

0 comments on commit f1ef8eb

Please sign in to comment.