Skip to content

Commit

Permalink
use client.Patch() for [add|remov]ing canary label (#69)
Browse files Browse the repository at this point in the history 
* use client.Patch() for [add|remov]ing canary label
* use client.Patch for Annotations update
  • Loading branch information
@clamoriniere
clamoriniere committed Dec 21, 2020
1 parent efa4090 commit 9df60a1
Show file tree
Hide file tree
Showing 2 changed files with 156 additions and 12 deletions.
23 changes: 15 additions & 8 deletions controllers/extendeddaemonsetreplicaset/strategy/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func manageUnscheduledPodNodes(pods []*corev1.Pod) []string {
}

// annotateCanaryDeploymentWithReason annotates the Canary deployment with a reason
func annotateCanaryDeploymentWithReason(client client.Client, eds *datadoghqv1alpha1.ExtendedDaemonSet, valueKey string, reasonKey string, reason datadoghqv1alpha1.ExtendedDaemonSetStatusReason) error {
func annotateCanaryDeploymentWithReason(c client.Client, eds *datadoghqv1alpha1.ExtendedDaemonSet, valueKey string, reasonKey string, reason datadoghqv1alpha1.ExtendedDaemonSetStatusReason) error {
newEds := eds.DeepCopy()
if newEds.Annotations == nil {
newEds.Annotations = make(map[string]string)
Expand All @@ -149,13 +149,11 @@ func annotateCanaryDeploymentWithReason(client client.Client, eds *datadoghqv1al
return nil
}
}

patch := client.MergeFrom(newEds.DeepCopy())
newEds.Annotations[valueKey] = valueTrue
newEds.Annotations[reasonKey] = string(reason)

if err := client.Update(context.TODO(), newEds); err != nil {
return err
}
return nil
return c.Patch(context.TODO(), newEds, patch)
}

// pauseCanaryDeployment updates two annotations so that the Canary deployment is marked as paused, along with a reason
Expand Down Expand Up @@ -206,8 +204,14 @@ func addPodLabel(c client.Client, pod *corev1.Pod, k, v string) error {
// The label is there, nothing to do
return nil
}

// A merge patch will preserve other fields modified at runtime.
patch := client.MergeFrom(pod.DeepCopy())
if pod.Labels == nil {
pod.Labels = make(map[string]string)
}
pod.Labels[k] = v
return c.Update(context.TODO(), pod)
return c.Patch(context.TODO(), pod, patch)
}

// deletePodLabel deletes a given pod label, no-op if the pod is nil or if the label doesn't exists
Expand All @@ -225,6 +229,9 @@ func deletePodLabel(c client.Client, pod *corev1.Pod, k string) error {
// The label is not there, nothing to do
return nil
}

// A merge patch will preserve other fields modified at runtime.
patch := client.MergeFrom(pod.DeepCopy())
delete(pod.Labels, k)
return c.Update(context.TODO(), pod)
return c.Patch(context.TODO(), pod, patch)
}
145 changes: 141 additions & 4 deletions controllers/extendeddaemonsetreplicaset/strategy/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,19 @@
package strategy

import (
"context"
"testing"

datadoghqv1alpha1 "github.com/DataDog/extendeddaemonset/api/v1alpha1"
"github.com/DataDog/extendeddaemonset/api/v1alpha1/test"
commontest "github.com/DataDog/extendeddaemonset/pkg/controller/test"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

datadoghqv1alpha1 "github.com/DataDog/extendeddaemonset/api/v1alpha1"
"github.com/DataDog/extendeddaemonset/api/v1alpha1/test"
commontest "github.com/DataDog/extendeddaemonset/pkg/controller/test"
)

func Test_compareWithExtendedDaemonsetSettingOverwrite(t *testing.T) {
Expand Down Expand Up @@ -195,3 +196,139 @@ func Test_failCanaryDeployment(t *testing.T) {
})
}
}

func Test_addPodLabel(t *testing.T) {
key := "key1"
val := "val1"
podNoLabel := commontest.NewPod("foo", "pod1", "node1", nil)
podLabeled := podNoLabel.DeepCopy()
podLabeled.Labels = make(map[string]string)
podLabeled.Labels[key] = val

validatationLabelFunc := func(t *testing.T, c client.Client, pod *corev1.Pod) {
wantPod := &corev1.Pod{}
nNs := types.NamespacedName{
Namespace: pod.Namespace,
Name: pod.Name,
}
err := c.Get(context.TODO(), nNs, wantPod)
assert.Nilf(t, err, "error must be nil, err: %v", err)

if gotVal, ok := wantPod.Labels[key]; ok {
assert.Equal(t, val, gotVal)
} else {
t.Fatalf("Label not present, pod: %#v", wantPod.Labels)
}
}

type args struct {
c client.Client
pod *corev1.Pod
k string
v string
}
tests := []struct {
name string
args args
validationFunc func(*testing.T, client.Client, *corev1.Pod)
wantErr bool
}{
{
name: "add label",
args: args{
c: fake.NewFakeClient(podNoLabel),
pod: podNoLabel,
k: key,
v: val,
},
validationFunc: validatationLabelFunc,
wantErr: false,
},
{
name: "label already present",
args: args{
c: fake.NewFakeClient(podLabeled),
pod: podLabeled,
k: key,
v: val,
},
wantErr: false,
validationFunc: validatationLabelFunc,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := addPodLabel(tt.args.c, tt.args.pod, tt.args.k, tt.args.v); (err != nil) != tt.wantErr {
t.Errorf("addPodLabel() error = %v, wantErr %v", err, tt.wantErr)
}
if tt.validationFunc != nil {
tt.validationFunc(t, tt.args.c, tt.args.pod)
}
})
}
}

func Test_deletePodLabel(t *testing.T) {
key := "key1"
val := "val1"
podNoLabel := commontest.NewPod("foo", "pod1", "node1", nil)
podLabeled := podNoLabel.DeepCopy()
podLabeled.Labels = make(map[string]string)
podLabeled.Labels[key] = val

validatationNoLabelFunc := func(t *testing.T, c client.Client, pod *corev1.Pod) {
wantPod := &corev1.Pod{}
nNs := types.NamespacedName{
Namespace: pod.Namespace,
Name: pod.Name,
}
err := c.Get(context.TODO(), nNs, wantPod)
assert.Nilf(t, err, "error must be nil, err: %v", err)
if _, ok := wantPod.Labels[key]; ok {
t.Fatalf("Label is present, pod: %#v", wantPod.Labels)
}
}

type args struct {
c client.Client
pod *corev1.Pod
k string
}
tests := []struct {
name string
args args
validationFunc func(*testing.T, client.Client, *corev1.Pod)
wantErr bool
}{
{
name: "delete label",
args: args{
c: fake.NewFakeClient(podLabeled),
pod: podLabeled,
k: key,
},
validationFunc: validatationNoLabelFunc,
wantErr: false,
},
{
name: "label not present",
args: args{
c: fake.NewFakeClient(podNoLabel),
pod: podNoLabel,
k: key,
},
wantErr: false,
validationFunc: validatationNoLabelFunc,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := deletePodLabel(tt.args.c, tt.args.pod, tt.args.k); (err != nil) != tt.wantErr {
t.Errorf("deletePodLabel() error = %v, wantErr %v", err, tt.wantErr)
}
if tt.validationFunc != nil {
tt.validationFunc(t, tt.args.c, tt.args.pod)
}
})
}
}

0 comments on commit 9df60a1

Please sign in to comment.