Skip to content

Commit fa89960

Browse files
tolushatolusha
committed
chore: Sync with options
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
1 parent 41c90f1 commit fa89960

File tree

9 files changed

+264
-201
lines changed

9 files changed

+264
-201
lines changed

‎controllers/devworkspace/solver/che_routing.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"strings"
2121

2222
dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
23-
"github.com/eclipse-che/che-operator/pkg/common/diffs"
2423
k8sclient "github.com/eclipse-che/che-operator/pkg/common/k8s-client"
2524

2625
"github.com/eclipse-che/che-operator/pkg/common/constants"
@@ -148,11 +147,11 @@ func (c *CheRoutingSolver) provisionRouting(objs *solvers.RoutingObjects, cheClu
148147

149148
// solvers.RoutingObjects does not currently support ConfigMaps, so we have to actually create it in cluster
150149
for _, cm := range configMaps {
151-
if err = k8sclient.MergeLabelsAnnotationsFromClusterObject(context.TODO(), c.scheme, c.cli, &cm); err != nil {
152-
return err
153-
}
154-
155-
if err = c.cliWrapper.Sync(context.TODO(), &cm, nil, diffs.ConfigMapAll); err != nil {
150+
if err = c.cliWrapper.Sync(
151+
context.TODO(),
152+
&cm,
153+
nil,
154+
&k8sclient.SyncOptions{MergeLabels: true, MergeAnnotations: true}); err != nil {
156155
return err
157156
}
158157
}

‎controllers/workspaceconfig/workspaces_config_controller.go‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -252,11 +252,20 @@ func (r *WorkspacesConfigReconciler) syncNamespace(
252252
// despite the result of the reconciliation
253253
if syncConfig != nil {
254254
if syncConfig.GetResourceVersion() == "" {
255-
if err = r.cliWrapper.Create(ctx, syncConfig, nil); err != nil {
255+
if err = r.cliWrapper.Create(
256+
ctx,
257+
syncConfig,
258+
nil,
259+
); err != nil {
256260
logger.Error(err, "Failed to workspace create sync config", "namespace", dstNamespace)
257261
}
258262
} else {
259-
if err = r.cliWrapper.Sync(ctx, syncConfig, nil, diffs.ConfigMapAllLabels); err != nil {
263+
if err = r.cliWrapper.Sync(
264+
ctx,
265+
syncConfig,
266+
nil,
267+
&k8sclient.SyncOptions{SuppressDiff: true, DiffOpts: diffs.ConfigMapAllLabels},
268+
); err != nil {
260269
logger.Error(err, "Failed to update workspace sync config", "namespace", dstNamespace)
261270
}
262271
}
@@ -569,21 +578,12 @@ func (r *WorkspacesConfigReconciler) doUpdateObject(
569578
dstObj client.Object,
570579
existedDstObj client.Object) error {
571580

572-
// preserve labels and annotations from existed object
573-
dstObj.SetLabels(utils.MergeMaps(
574-
[]map[string]string{
575-
existedDstObj.GetLabels(),
576-
dstObj.GetLabels(),
577-
},
578-
))
579-
dstObj.SetAnnotations(utils.MergeMaps(
580-
[]map[string]string{
581-
existedDstObj.GetAnnotations(),
582-
dstObj.GetAnnotations(),
583-
},
584-
))
585-
586-
if err := r.cliWrapper.Sync(syncContext.ctx, dstObj, nil); err != nil {
581+
if err := r.cliWrapper.Sync(
582+
syncContext.ctx,
583+
dstObj,
584+
nil,
585+
&k8sclient.SyncOptions{MergeAnnotations: true, MergeLabels: true, SuppressDiff: true},
586+
); err != nil {
587587
return err
588588
}
589589

‎pkg/common/diffs/diffs.go‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,25 +13,18 @@
1313
package diffs
1414

1515
import (
16-
"reflect"
16+
"maps"
1717

1818
"github.com/google/go-cmp/cmp"
1919
"github.com/google/go-cmp/cmp/cmpopts"
2020
corev1 "k8s.io/api/core/v1"
2121
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2222
)
2323

24-
var ConfigMapAll = cmp.Options{
25-
cmpopts.IgnoreFields(corev1.ConfigMap{}, "TypeMeta"),
26-
cmp.Comparer(func(x, y metav1.ObjectMeta) bool {
27-
return reflect.DeepEqual(x.Labels, y.Labels) && reflect.DeepEqual(x.Annotations, y.Annotations)
28-
}),
29-
}
30-
3124
var ConfigMapAllLabels = cmp.Options{
3225
cmpopts.IgnoreFields(corev1.ConfigMap{}, "TypeMeta"),
3326
cmp.Comparer(func(x, y metav1.ObjectMeta) bool {
34-
return reflect.DeepEqual(x.Labels, y.Labels)
27+
return maps.Equal(x.Labels, y.Labels)
3528
}),
3629
}
3730

‎pkg/common/diffs/diffs_test.go‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
//
2+
// Copyright (c) 2019-2025 Red Hat, Inc.
3+
// This program and the accompanying materials are made
4+
// available under the terms of the Eclipse Public License 2.0
5+
// which is available at https://www.eclipse.org/legal/epl-2.0/
6+
//
7+
// SPDX-License-Identifier: EPL-2.0
8+
//
9+
// Contributors:
10+
// Red Hat, Inc. - initial API and implementation
11+
//
12+
13+
package diffs
14+
15+
import (
16+
"fmt"
17+
"testing"
18+
19+
"github.com/google/go-cmp/cmp"
20+
"github.com/stretchr/testify/assert"
21+
corev1 "k8s.io/api/core/v1"
22+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
23+
)
24+
25+
func TestConfigMap(t *testing.T) {
26+
type testCase struct {
27+
srcCm *corev1.ConfigMap
28+
dstCm *corev1.ConfigMap
29+
diffs cmp.Options
30+
isEqual bool
31+
}
32+
33+
testCases := []testCase{
34+
{
35+
srcCm: &corev1.ConfigMap{},
36+
dstCm: &corev1.ConfigMap{},
37+
diffs: ConfigMapAllLabels,
38+
isEqual: true,
39+
},
40+
{
41+
srcCm: &corev1.ConfigMap{
42+
ObjectMeta: metav1.ObjectMeta{
43+
Labels: map[string]string{},
44+
Annotations: map[string]string{},
45+
},
46+
},
47+
dstCm: &corev1.ConfigMap{},
48+
diffs: ConfigMapAllLabels,
49+
isEqual: true,
50+
},
51+
}
52+
53+
for i, testCase := range testCases {
54+
t.Run(fmt.Sprintf("Test case %d", i), func(t *testing.T) {
55+
assert.Equal(
56+
t,
57+
testCase.isEqual,
58+
cmp.Equal(testCase.srcCm, testCase.dstCm, testCase.diffs),
59+
)
60+
})
61+
}
62+
}

‎pkg/common/k8s-client/k8s_client.go‎

Lines changed: 34 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -36,26 +36,6 @@ var (
3636
logger = ctrl.Log.WithName("k8s")
3737
)
3838

39-
type K8sClient interface {
40-
// Sync ensures that the object is up to date in the cluster.
41-
// Object is created if it does not exist and updated if it exists but is different.
42-
// Returns nil if object is in sync.
43-
Sync(ctx context.Context, blueprint client.Object, owner metav1.Object, diffOpts ...cmp.Option) error
44-
// Create creates object.
45-
// Returns nil if object is created otherwise returns error.
46-
Create(ctx context.Context, blueprint client.Object, owner metav1.Object, opts ...client.CreateOption) error
47-
// GetIgnoreNotFound gets object.
48-
// Returns true if object exists otherwise returns false.
49-
// Returns nil if object is retrieved or not found otherwise returns error.
50-
GetIgnoreNotFound(ctx context.Context, key client.ObjectKey, objectMeta client.Object, opts ...client.GetOption) (bool, error)
51-
// DeleteByKeyIgnoreNotFound deletes object by key.
52-
// Returns nil if object is deleted or not found otherwise returns error.
53-
DeleteByKeyIgnoreNotFound(ctx context.Context, key client.ObjectKey, objectMeta client.Object, opts ...client.DeleteOption) error
54-
// List returns list of runtime objects.
55-
// Returns nil if list is retrieved otherwise returns error.
56-
List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) ([]runtime.Object, error)
57-
}
58-
5939
func NewK8sClient(cli client.Client, scheme *runtime.Scheme) *K8sClientWrapper {
6040
return &K8sClientWrapper{cli: cli, scheme: scheme}
6141
}
@@ -69,7 +49,7 @@ func (k K8sClientWrapper) Sync(
6949
ctx context.Context,
7050
obj client.Object,
7151
owner metav1.Object,
72-
diffOpts ...cmp.Option,
52+
opts ...SyncOption,
7353
) error {
7454
defer func() {
7555
// ensure GVK is set (for original object) when function returns
@@ -94,9 +74,9 @@ func (k K8sClientWrapper) Sync(
9474
Namespace: obj.GetNamespace(),
9575
}
9676
if exists, err := k.doGetIgnoreNotFound(ctx, key, actual.(client.Object)); exists {
97-
return k.doSync(ctx, actual.(client.Object), obj, diffOpts...)
77+
return k.doSync(ctx, actual.(client.Object), obj, opts...)
9878
} else if err == nil {
99-
return k.doSync(ctx, nil, obj, diffOpts...)
79+
return k.doSync(ctx, nil, obj, opts...)
10080
} else {
10181
return err
10282
}
@@ -256,16 +236,19 @@ func (k K8sClientWrapper) doSync(
256236
ctx context.Context,
257237
actual client.Object,
258238
obj client.Object,
259-
diffOpts ...cmp.Option,
239+
opts ...SyncOption,
260240
) error {
261241
if actual == nil {
262242
return k.doCreate(ctx, obj, false)
263243
}
264244

265-
diff := cmp.Diff(actual, obj, diffOpts...)
245+
syncOptions := SyncOptions{}
246+
syncOptions.ApplyOptions(opts)
247+
248+
diff := cmp.Diff(actual, obj, syncOptions.DiffOpts...)
266249
if len(diff) > 0 {
267-
// don't print difference if there are no diffOpts mainly to avoid huge output
268-
if len(diffOpts) != 0 {
250+
// don't print difference if there are no DiffOpts mainly to avoid huge output
251+
if !syncOptions.SuppressDiff && len(syncOptions.DiffOpts) != 0 {
269252
fmt.Printf("Difference:\n%s", diff)
270253
}
271254

@@ -279,6 +262,30 @@ func (k K8sClientWrapper) doSync(
279262
// to be able to update, we need to set the resource version of the object that we know of
280263
obj.(metav1.Object).SetResourceVersion(actual.GetResourceVersion())
281264

265+
if syncOptions.MergeLabels {
266+
if obj.GetLabels() == nil {
267+
obj.SetLabels(map[string]string{})
268+
}
269+
270+
for k, v := range actual.GetLabels() {
271+
if _, exists := obj.GetLabels()[k]; !exists {
272+
obj.GetLabels()[k] = v
273+
}
274+
}
275+
}
276+
277+
if syncOptions.MergeAnnotations {
278+
if obj.GetAnnotations() == nil {
279+
obj.SetAnnotations(map[string]string{})
280+
}
281+
282+
for k, v := range actual.GetAnnotations() {
283+
if _, exists := obj.GetAnnotations()[k]; !exists {
284+
obj.GetAnnotations()[k] = v
285+
}
286+
}
287+
}
288+
282289
err := k.cli.Update(ctx, obj)
283290
if err == nil {
284291
logger.Info("Object updated", "namespace", actual.GetNamespace(), "kind", GetObjectType(actual), "name", actual.GetName())

‎pkg/common/k8s-client/k8s_client_sync_test.go‎

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/stretchr/testify/assert"
2424
corev1 "k8s.io/api/core/v1"
2525
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26+
"k8s.io/apimachinery/pkg/types"
2627
)
2728

2829
var (
@@ -49,7 +50,7 @@ func TestSync(t *testing.T) {
4950
},
5051
}
5152

52-
err := cli.Sync(context.TODO(), cm, nil, diffs)
53+
err := cli.Sync(context.TODO(), cm, nil, &SyncOptions{DiffOpts: diffs})
5354

5455
assert.NoError(t, err)
5556
assert.Equal(t, "ConfigMap", cm.Kind)
@@ -69,7 +70,7 @@ func TestSync(t *testing.T) {
6970
},
7071
}
7172

72-
err = cli.Sync(context.TODO(), newCm, nil, diffs)
73+
err = cli.Sync(context.TODO(), newCm, nil, &SyncOptions{DiffOpts: diffs})
7374

7475
assert.NoError(t, err)
7576
assert.Equal(t, "ConfigMap", newCm.Kind)
@@ -89,9 +90,68 @@ func TestSync(t *testing.T) {
8990
},
9091
}
9192

92-
err = cli.Sync(context.TODO(), newCm, nil, diffs)
93+
err = cli.Sync(context.TODO(), newCm, nil, &SyncOptions{DiffOpts: diffs})
9394

9495
assert.NoError(t, err)
9596
assert.Equal(t, "ConfigMap", newCm.Kind)
9697
assert.Equal(t, "v1", newCm.APIVersion)
9798
}
99+
100+
func TestSyncAndMergeLabelsAnnotations(t *testing.T) {
101+
fakeClient, _, scheme := testclient.GetTestClients(
102+
&corev1.ConfigMap{
103+
TypeMeta: metav1.TypeMeta{
104+
Kind: "ConfigMap",
105+
APIVersion: "v1",
106+
},
107+
ObjectMeta: metav1.ObjectMeta{
108+
Name: "test",
109+
Namespace: "test",
110+
Labels: map[string]string{
111+
"label_1": "cluster_value_1",
112+
"label_2": "cluster_value_2",
113+
},
114+
Annotations: map[string]string{
115+
"annotation_1": "cluster_value_1",
116+
"annotation_2": "cluster_value_2",
117+
},
118+
},
119+
})
120+
cli := NewK8sClient(fakeClient, scheme)
121+
122+
err := cli.Sync(
123+
context.TODO(),
124+
&corev1.ConfigMap{
125+
TypeMeta: metav1.TypeMeta{
126+
Kind: "ConfigMap",
127+
APIVersion: "v1",
128+
},
129+
ObjectMeta: metav1.ObjectMeta{
130+
Name: "test",
131+
Namespace: "test",
132+
Labels: map[string]string{
133+
"label_1": "value_1",
134+
"label_3": "value_3",
135+
},
136+
Annotations: map[string]string{
137+
"annotation_1": "value_1",
138+
"annotation_3": "value_3",
139+
},
140+
},
141+
},
142+
nil,
143+
&SyncOptions{MergeLabels: true, MergeAnnotations: true},
144+
)
145+
146+
assert.NoError(t, err)
147+
148+
cm := &corev1.ConfigMap{}
149+
err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: "test", Namespace: "test"}, cm)
150+
151+
assert.Equal(t, "value_1", cm.Labels["label_1"])
152+
assert.Equal(t, "cluster_value_2", cm.Labels["label_2"])
153+
assert.Equal(t, "value_3", cm.Labels["label_3"])
154+
assert.Equal(t, "value_1", cm.Annotations["annotation_1"])
155+
assert.Equal(t, "cluster_value_2", cm.Annotations["annotation_2"])
156+
assert.Equal(t, "value_3", cm.Annotations["annotation_3"])
157+
}

0 commit comments

Comments
 (0)