Skip to content

Commit

Permalink
Merge pull request #685 from splkforrest/add-label-value
Browse files Browse the repository at this point in the history 
Derive the label selector value from the target matchLabels
  • Loading branch information
@stefanprodan
stefanprodan authored Sep 17, 2020
2 parents fba16aa + 6c35f76 commit 6d65a2c
Show file tree
Hide file tree
Showing 24 changed files with 475 additions and 96 deletions.
12 changes: 8 additions & 4 deletions pkg/canary/config_tracker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ func TestConfigIsDisabled(t *testing.T) {

func TestConfigTracker_ConfigMaps(t *testing.T) {
t.Run("deployment", func(t *testing.T) {
mocks := newDeploymentFixture()
dc := deploymentConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDeploymentFixture(dc)
configMap := newDeploymentControllerTestConfigMap()
configMapProjected := newDeploymentControllerTestConfigProjected()

Expand Down Expand Up @@ -89,7 +90,8 @@ func TestConfigTracker_ConfigMaps(t *testing.T) {
})

t.Run("daemonset", func(t *testing.T) {
mocks := newDaemonSetFixture()
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDaemonSetFixture(dc)
configMap := newDaemonSetControllerTestConfigMap()
configMapProjected := newDaemonSetControllerTestConfigProjected()

Expand Down Expand Up @@ -156,7 +158,8 @@ func TestConfigTracker_ConfigMaps(t *testing.T) {

func TestConfigTracker_Secrets(t *testing.T) {
t.Run("deployment", func(t *testing.T) {
mocks := newDeploymentFixture()
dc := deploymentConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDeploymentFixture(dc)
secret := newDeploymentControllerTestSecret()
secretProjected := newDeploymentControllerTestSecretProjected()

Expand Down Expand Up @@ -220,7 +223,8 @@ func TestConfigTracker_Secrets(t *testing.T) {
})

t.Run("daemonset", func(t *testing.T) {
mocks := newDaemonSetFixture()
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDaemonSetFixture(dc)
secret := newDaemonSetControllerTestSecret()
secretProjected := newDaemonSetControllerTestSecretProjected()

Expand Down
2 changes: 1 addition & 1 deletion pkg/canary/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
type Controller interface {
IsPrimaryReady(canary *flaggerv1.Canary) error
IsCanaryReady(canary *flaggerv1.Canary) (bool, error)
GetMetadata(canary *flaggerv1.Canary) (string, map[string]int32, error)
GetMetadata(canary *flaggerv1.Canary) (string, string, map[string]int32, error)
SyncStatus(canary *flaggerv1.Canary, status flaggerv1.CanaryStatus) error
SetStatusFailedChecks(canary *flaggerv1.Canary, val int) error
SetStatusWeight(canary *flaggerv1.Canary, val int) error
Expand Down
30 changes: 16 additions & 14 deletions pkg/canary/daemonset_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ func (c *DaemonSetController) Promote(cd *flaggerv1.Canary) error {
return fmt.Errorf("damonset %s.%s get query error: %v", targetName, cd.Namespace, err)
}

label, err := c.getSelectorLabel(canary)
label, labelValue, err := c.getSelectorLabel(canary)
primaryLabelValue := fmt.Sprintf("%s-primary", labelValue)
if err != nil {
return fmt.Errorf("getSelectorLabel failed: %w", err)
}
Expand Down Expand Up @@ -146,7 +147,7 @@ func (c *DaemonSetController) Promote(cd *flaggerv1.Canary) error {
}

primaryCopy.Spec.Template.Annotations = annotations
primaryCopy.Spec.Template.Labels = makePrimaryLabels(canary.Spec.Template.Labels, primaryName, label)
primaryCopy.Spec.Template.Labels = makePrimaryLabels(canary.Spec.Template.Labels, primaryLabelValue, label)

// apply update
_, err = c.kubeClient.AppsV1().DaemonSets(cd.Namespace).Update(context.TODO(), primaryCopy, metav1.UpdateOptions{})
Expand Down Expand Up @@ -179,24 +180,24 @@ func (c *DaemonSetController) HasTargetChanged(cd *flaggerv1.Canary) (bool, erro
}

// GetMetadata returns the pod label selector and svc ports
func (c *DaemonSetController) GetMetadata(cd *flaggerv1.Canary) (string, map[string]int32, error) {
func (c *DaemonSetController) GetMetadata(cd *flaggerv1.Canary) (string, string, map[string]int32, error) {
targetName := cd.Spec.TargetRef.Name

canaryDae, err := c.kubeClient.AppsV1().DaemonSets(cd.Namespace).Get(context.TODO(), targetName, metav1.GetOptions{})
if err != nil {
return "", nil, fmt.Errorf("daemonset %s.%s get query error: %w", targetName, cd.Namespace, err)
return "", "", nil, fmt.Errorf("daemonset %s.%s get query error: %w", targetName, cd.Namespace, err)
}

label, err := c.getSelectorLabel(canaryDae)
label, labelValue, err := c.getSelectorLabel(canaryDae)
if err != nil {
return "", nil, fmt.Errorf("getSelectorLabel failed: %w", err)
return "", "", nil, fmt.Errorf("getSelectorLabel failed: %w", err)
}

var ports map[string]int32
if cd.Spec.Service.PortDiscovery {
ports = getPorts(cd, canaryDae.Spec.Template.Spec.Containers)
}
return label, ports, nil
return label, labelValue, ports, nil
}

func (c *DaemonSetController) createPrimaryDaemonSet(cd *flaggerv1.Canary) error {
Expand All @@ -214,7 +215,8 @@ func (c *DaemonSetController) createPrimaryDaemonSet(cd *flaggerv1.Canary) error
targetName, cd.Namespace, canaryDae.Spec.UpdateStrategy.Type)
}

label, err := c.getSelectorLabel(canaryDae)
label, labelValue, err := c.getSelectorLabel(canaryDae)
primaryLabelValue := fmt.Sprintf("%s-primary", labelValue)
if err != nil {
return fmt.Errorf("getSelectorLabel failed: %w", err)
}
Expand All @@ -240,7 +242,7 @@ func (c *DaemonSetController) createPrimaryDaemonSet(cd *flaggerv1.Canary) error
Name: primaryName,
Namespace: cd.Namespace,
Labels: map[string]string{
label: primaryName,
label: primaryLabelValue,
},
OwnerReferences: []metav1.OwnerReference{
*metav1.NewControllerRef(cd, schema.GroupVersionKind{
Expand All @@ -256,12 +258,12 @@ func (c *DaemonSetController) createPrimaryDaemonSet(cd *flaggerv1.Canary) error
UpdateStrategy: canaryDae.Spec.UpdateStrategy,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
label: primaryName,
label: primaryLabelValue,
},
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: makePrimaryLabels(canaryDae.Spec.Template.Labels, primaryName, label),
Labels: makePrimaryLabels(canaryDae.Spec.Template.Labels, primaryLabelValue, label),
Annotations: annotations,
},
// update spec with the primary secrets and config maps
Expand All @@ -281,14 +283,14 @@ func (c *DaemonSetController) createPrimaryDaemonSet(cd *flaggerv1.Canary) error
}

// getSelectorLabel returns the selector match label
func (c *DaemonSetController) getSelectorLabel(daemonSet *appsv1.DaemonSet) (string, error) {
func (c *DaemonSetController) getSelectorLabel(daemonSet *appsv1.DaemonSet) (string, string, error) {
for _, l := range c.labels {
if _, ok := daemonSet.Spec.Selector.MatchLabels[l]; ok {
return l, nil
return l, daemonSet.Spec.Selector.MatchLabels[l], nil
}
}

return "", fmt.Errorf(
return "", "", fmt.Errorf(
"daemonset %s.%s spec.selector.matchLabels must contain one of %v'",
daemonSet.Name, daemonSet.Namespace, c.labels,
)
Expand Down
51 changes: 41 additions & 10 deletions pkg/canary/daemonset_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package canary

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -14,22 +15,47 @@ import (
flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1"
)

func TestDaemonSetController_Sync(t *testing.T) {
mocks := newDaemonSetFixture()
func TestDaemonSetController_Sync_ConsistentNaming(t *testing.T) {
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDaemonSetFixture(dc)
err := mocks.controller.Initialize(mocks.canary)
require.NoError(t, err)

daePrimary, err := mocks.kubeClient.AppsV1().DaemonSets("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{})
daePrimary, err := mocks.kubeClient.AppsV1().DaemonSets("default").Get(context.TODO(), fmt.Sprintf("%s-primary", dc.name), metav1.GetOptions{})
require.NoError(t, err)

dae := newDaemonSetControllerTestPodInfo()
dae := newDaemonSetControllerTestPodInfo(dc)
primaryImage := daePrimary.Spec.Template.Spec.Containers[0].Image
sourceImage := dae.Spec.Template.Spec.Containers[0].Image
assert.Equal(t, primaryImage, sourceImage)

primarySelectorValue := daePrimary.Spec.Selector.MatchLabels[dc.label]
sourceSelectorValue := dae.Spec.Selector.MatchLabels[dc.label]
assert.Equal(t, primarySelectorValue, fmt.Sprintf("%s-primary", sourceSelectorValue))
}

func TestDaemonSetController_Sync_InconsistentNaming(t *testing.T) {
dc := daemonsetConfigs{name: "podinfo-service", label: "name", labelValue: "podinfo"}
mocks := newDaemonSetFixture(dc)
err := mocks.controller.Initialize(mocks.canary)
require.NoError(t, err)

daePrimary, err := mocks.kubeClient.AppsV1().DaemonSets("default").Get(context.TODO(), fmt.Sprintf("%s-primary", dc.name), metav1.GetOptions{})
require.NoError(t, err)

dae := newDaemonSetControllerTestPodInfo(dc)
primaryImage := daePrimary.Spec.Template.Spec.Containers[0].Image
sourceImage := dae.Spec.Template.Spec.Containers[0].Image
assert.Equal(t, primaryImage, sourceImage)

primarySelectorValue := daePrimary.Spec.Selector.MatchLabels[dc.label]
sourceSelectorValue := dae.Spec.Selector.MatchLabels[dc.label]
assert.Equal(t, primarySelectorValue, fmt.Sprintf("%s-primary", sourceSelectorValue))
}

func TestDaemonSetController_Promote(t *testing.T) {
mocks := newDaemonSetFixture()
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDaemonSetFixture(dc)
err := mocks.controller.Initialize(mocks.canary)
require.NoError(t, err)

Expand Down Expand Up @@ -58,7 +84,8 @@ func TestDaemonSetController_Promote(t *testing.T) {
}

func TestDaemonSetController_NoConfigTracking(t *testing.T) {
mocks := newDaemonSetFixture()
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDaemonSetFixture(dc)
mocks.controller.configTracker = &NopTracker{}

err := mocks.controller.Initialize(mocks.canary)
Expand All @@ -75,7 +102,8 @@ func TestDaemonSetController_NoConfigTracking(t *testing.T) {
}

func TestDaemonSetController_HasTargetChanged(t *testing.T) {
mocks := newDaemonSetFixture()
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDaemonSetFixture(dc)
err := mocks.controller.Initialize(mocks.canary)
require.NoError(t, err)

Expand Down Expand Up @@ -163,7 +191,8 @@ func TestDaemonSetController_HasTargetChanged(t *testing.T) {

func TestDaemonSetController_Scale(t *testing.T) {
t.Run("ScaleToZero", func(t *testing.T) {
mocks := newDaemonSetFixture()
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDaemonSetFixture(dc)
err := mocks.controller.Initialize(mocks.canary)
require.NoError(t, err)

Expand All @@ -179,7 +208,8 @@ func TestDaemonSetController_Scale(t *testing.T) {
}
})
t.Run("ScaleFromZeo", func(t *testing.T) {
mocks := newDaemonSetFixture()
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDaemonSetFixture(dc)
err := mocks.controller.Initialize(mocks.canary)
require.NoError(t, err)

Expand All @@ -197,7 +227,8 @@ func TestDaemonSetController_Scale(t *testing.T) {
}

func TestDaemonSetController_Finalize(t *testing.T) {
mocks := newDaemonSetFixture()
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDaemonSetFixture(dc)
err := mocks.controller.Initialize(mocks.canary)
require.NoError(t, err)

Expand Down
24 changes: 15 additions & 9 deletions pkg/canary/daemonset_fixture_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,20 @@ type daemonSetControllerFixture struct {
logger *zap.SugaredLogger
}

func newDaemonSetFixture() daemonSetControllerFixture {
type daemonsetConfigs struct {
name string
labelValue string
label string
}

func newDaemonSetFixture(dc daemonsetConfigs) daemonSetControllerFixture {
// init canary
canary := newDaemonSetControllerTestCanary()
canary := newDaemonSetControllerTestCanary(dc)
flaggerClient := fakeFlagger.NewSimpleClientset(canary)

// init kube clientset and register mock objects
kubeClient := fake.NewSimpleClientset(
newDaemonSetControllerTestPodInfo(),
newDaemonSetControllerTestPodInfo(dc),
newDaemonSetControllerTestConfigMap(),
newDaemonSetControllerTestConfigMapEnv(),
newDaemonSetControllerTestConfigMapVol(),
Expand Down Expand Up @@ -264,7 +270,7 @@ func newDaemonSetControllerTestSecretTrackerDisabled() *corev1.Secret {
}
}

func newDaemonSetControllerTestCanary() *flaggerv1.Canary {
func newDaemonSetControllerTestCanary(dc daemonsetConfigs) *flaggerv1.Canary {
cd := &flaggerv1.Canary{
TypeMeta: metav1.TypeMeta{APIVersion: flaggerv1.SchemeGroupVersion.String()},
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -273,7 +279,7 @@ func newDaemonSetControllerTestCanary() *flaggerv1.Canary {
},
Spec: flaggerv1.CanarySpec{
TargetRef: flaggerv1.CrossNamespaceObjectReference{
Name: "podinfo",
Name: dc.name,
APIVersion: "apps/v1",
Kind: "DaemonSet",
},
Expand All @@ -282,23 +288,23 @@ func newDaemonSetControllerTestCanary() *flaggerv1.Canary {
return cd
}

func newDaemonSetControllerTestPodInfo() *appsv1.DaemonSet {
func newDaemonSetControllerTestPodInfo(dc daemonsetConfigs) *appsv1.DaemonSet {
d := &appsv1.DaemonSet{
TypeMeta: metav1.TypeMeta{APIVersion: appsv1.SchemeGroupVersion.String()},
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "podinfo",
Name: dc.name,
},
Spec: appsv1.DaemonSetSpec{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"name": "podinfo",
dc.label: dc.labelValue,
},
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"name": "podinfo",
dc.label: dc.labelValue,
},
},
Spec: corev1.PodSpec{
Expand Down
6 changes: 4 additions & 2 deletions pkg/canary/daemonset_ready_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import (
)

func TestDaemonSetController_IsReady(t *testing.T) {
mocks := newDaemonSetFixture()
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDaemonSetFixture(dc)
err := mocks.controller.Initialize(mocks.canary)
require.NoError(t, err)

Expand All @@ -24,7 +25,8 @@ func TestDaemonSetController_IsReady(t *testing.T) {
}

func TestDaemonSetController_isDaemonSetReady(t *testing.T) {
mocks := newDaemonSetFixture()
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDaemonSetFixture(dc)
cd := &flaggerv1.Canary{}

// observed generation is less than desired generation
Expand Down
9 changes: 6 additions & 3 deletions pkg/canary/daemonset_status_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import (
)

func TestDaemonSetController_SyncStatus(t *testing.T) {
mocks := newDaemonSetFixture()
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDaemonSetFixture(dc)
err := mocks.controller.Initialize(mocks.canary)
require.NoError(t, err)

Expand All @@ -36,7 +37,8 @@ func TestDaemonSetController_SyncStatus(t *testing.T) {
}

func TestDaemonSetController_SetFailedChecks(t *testing.T) {
mocks := newDaemonSetFixture()
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDaemonSetFixture(dc)
err := mocks.controller.Initialize(mocks.canary)
require.NoError(t, err)

Expand All @@ -49,7 +51,8 @@ func TestDaemonSetController_SetFailedChecks(t *testing.T) {
}

func TestDaemonSetController_SetState(t *testing.T) {
mocks := newDaemonSetFixture()
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
mocks := newDaemonSetFixture(dc)
err := mocks.controller.Initialize(mocks.canary)
require.NoError(t, err)

Expand Down
Loading

0 comments on commit 6d65a2c

Please sign in to comment.