josie.lol
k8s operator for knot hosting
tangled
kubernetes
fix: skip no-op updates to stop reconcile loop crashloop
createOrUpdate was unconditionally writing all resources every cycle,
triggering watch events and cascading re-reconciliations. Compare
managed fields before writing. Bump memory limit to 512Mi.
Signed-off-by: Josephine Pfeiffer <hi@josie.lol>
+311
-19
+1
-1
config/crd/tangled.org_knots.yaml
+1
-1
config/crd/tangled.org_knots.yaml
+2
-2
config/manager/manager.yaml
+2
-2
config/manager/manager.yaml
+74
-13
internal/controller/knot_controller.go
+74
-13
internal/controller/knot_controller.go
···
5
5
"crypto/rand"
6
6
"encoding/hex"
7
7
"fmt"
8
+
"reflect"
8
9
"time"
9
10
10
11
"github.com/go-logr/logr"
···
202
203
203
204
func (r *KnotReconciler) reconcilePVCs(ctx context.Context, knot *tangledv1alpha1.Knot) error {
204
205
repoPVC := r.buildPVC(knot, "repos", knot.Spec.Storage.RepoSize, knot.Spec.Storage.RepoStorageClass)
205
-
if err := r.createOrUpdate(ctx, repoPVC, knot, func(existing, desired client.Object) {}); err != nil {
206
+
if err := r.createOrUpdate(ctx, repoPVC, knot, func(existing, desired client.Object) bool { return false }); err != nil {
206
207
return err
207
208
}
208
209
209
210
dbPVC := r.buildPVC(knot, "db", knot.Spec.Storage.DBSize, knot.Spec.Storage.DBStorageClass)
210
-
return r.createOrUpdate(ctx, dbPVC, knot, func(existing, desired client.Object) {})
211
+
return r.createOrUpdate(ctx, dbPVC, knot, func(existing, desired client.Object) bool { return false })
211
212
}
212
213
213
214
func (r *KnotReconciler) buildPVC(knot *tangledv1alpha1.Knot, suffix, size, storageClass string) *corev1.PersistentVolumeClaim {
···
234
235
return pvc
235
236
}
236
237
237
-
type updateFunc func(existing, desired client.Object)
238
+
type updateFunc func(existing, desired client.Object) bool
238
239
239
240
func (r *KnotReconciler) createOrUpdate(ctx context.Context, obj client.Object, knot *tangledv1alpha1.Knot, update updateFunc) error {
240
241
if err := controllerutil.SetControllerReference(knot, obj, r.Scheme); err != nil {
···
250
251
return err
251
252
}
252
253
253
-
if update != nil {
254
-
update(existing, obj)
254
+
if update != nil && update(existing, obj) {
255
+
return r.Update(ctx, existing)
255
256
}
256
-
return r.Update(ctx, existing)
257
+
return nil
257
258
}
258
259
259
260
func (r *KnotReconciler) reconcileConfigMap(ctx context.Context, knot *tangledv1alpha1.Knot) error {
···
274
275
},
275
276
}
276
277
277
-
return r.createOrUpdate(ctx, cm, knot, func(existing, desired client.Object) {
278
-
existing.(*corev1.ConfigMap).Data = desired.(*corev1.ConfigMap).Data
278
+
return r.createOrUpdate(ctx, cm, knot, func(existing, desired client.Object) bool {
279
+
e := existing.(*corev1.ConfigMap)
280
+
d := desired.(*corev1.ConfigMap)
281
+
if reflect.DeepEqual(e.Data, d.Data) {
282
+
return false
283
+
}
284
+
e.Data = d.Data
285
+
return true
279
286
})
280
287
}
281
288
···
356
363
},
357
364
}
358
365
359
-
return r.createOrUpdate(ctx, svc, knot, func(existing, desired client.Object) {
366
+
return r.createOrUpdate(ctx, svc, knot, func(existing, desired client.Object) bool {
360
367
existingSvc := existing.(*corev1.Service)
361
368
desiredSvc := desired.(*corev1.Service)
369
+
if reflect.DeepEqual(existingSvc.Spec.Ports, desiredSvc.Spec.Ports) &&
370
+
reflect.DeepEqual(existingSvc.Spec.Selector, desiredSvc.Spec.Selector) {
371
+
return false
372
+
}
362
373
existingSvc.Spec.Ports = desiredSvc.Spec.Ports
363
374
existingSvc.Spec.Selector = desiredSvc.Spec.Selector
375
+
return true
364
376
})
365
377
}
366
378
···
409
421
svc.Spec.Ports[0].NodePort = sshSpec.NodePort
410
422
}
411
423
412
-
return r.createOrUpdate(ctx, svc, knot, func(existing, desired client.Object) {
424
+
return r.createOrUpdate(ctx, svc, knot, func(existing, desired client.Object) bool {
413
425
existingSvc := existing.(*corev1.Service)
414
426
desiredSvc := desired.(*corev1.Service)
427
+
if reflect.DeepEqual(existingSvc.Spec.Ports, desiredSvc.Spec.Ports) &&
428
+
existingSvc.Spec.Type == desiredSvc.Spec.Type {
429
+
return false
430
+
}
415
431
existingSvc.Spec.Ports = desiredSvc.Spec.Ports
416
432
existingSvc.Spec.Type = desiredSvc.Spec.Type
433
+
return true
417
434
})
418
435
}
419
436
420
437
func (r *KnotReconciler) reconcileDeployment(ctx context.Context, knot *tangledv1alpha1.Knot) error {
421
438
dep := r.buildDeployment(knot)
422
439
423
-
return r.createOrUpdate(ctx, dep, knot, func(existing, desired client.Object) {
424
-
existing.(*appsv1.Deployment).Spec = desired.(*appsv1.Deployment).Spec
440
+
return r.createOrUpdate(ctx, dep, knot, func(existing, desired client.Object) bool {
441
+
e := existing.(*appsv1.Deployment)
442
+
d := desired.(*appsv1.Deployment)
443
+
changed := false
444
+
if !reflect.DeepEqual(e.Spec.Replicas, d.Spec.Replicas) {
445
+
e.Spec.Replicas = d.Spec.Replicas
446
+
changed = true
447
+
}
448
+
if !reflect.DeepEqual(e.Spec.Template.Spec.Containers, d.Spec.Template.Spec.Containers) {
449
+
e.Spec.Template.Spec.Containers = d.Spec.Template.Spec.Containers
450
+
changed = true
451
+
}
452
+
if !reflect.DeepEqual(e.Spec.Template.Spec.InitContainers, d.Spec.Template.Spec.InitContainers) {
453
+
e.Spec.Template.Spec.InitContainers = d.Spec.Template.Spec.InitContainers
454
+
changed = true
455
+
}
456
+
if !reflect.DeepEqual(e.Spec.Template.Spec.Volumes, d.Spec.Template.Spec.Volumes) {
457
+
e.Spec.Template.Spec.Volumes = d.Spec.Template.Spec.Volumes
458
+
changed = true
459
+
}
460
+
if !reflect.DeepEqual(e.Spec.Template.Spec.NodeSelector, d.Spec.Template.Spec.NodeSelector) {
461
+
e.Spec.Template.Spec.NodeSelector = d.Spec.Template.Spec.NodeSelector
462
+
changed = true
463
+
}
464
+
if !reflect.DeepEqual(e.Spec.Template.Spec.Tolerations, d.Spec.Template.Spec.Tolerations) {
465
+
e.Spec.Template.Spec.Tolerations = d.Spec.Template.Spec.Tolerations
466
+
changed = true
467
+
}
468
+
if !reflect.DeepEqual(e.Spec.Template.Spec.Affinity, d.Spec.Template.Spec.Affinity) {
469
+
e.Spec.Template.Spec.Affinity = d.Spec.Template.Spec.Affinity
470
+
changed = true
471
+
}
472
+
if e.Spec.Template.Spec.ServiceAccountName != d.Spec.Template.Spec.ServiceAccountName {
473
+
e.Spec.Template.Spec.ServiceAccountName = d.Spec.Template.Spec.ServiceAccountName
474
+
changed = true
475
+
}
476
+
if !reflect.DeepEqual(e.Spec.Template.Labels, d.Spec.Template.Labels) {
477
+
e.Spec.Template.Labels = d.Spec.Template.Labels
478
+
changed = true
479
+
}
480
+
return changed
425
481
})
426
482
}
427
483
···
639
695
}
640
696
}
641
697
642
-
return r.createOrUpdate(ctx, ingress, knot, func(existing, desired client.Object) {
698
+
return r.createOrUpdate(ctx, ingress, knot, func(existing, desired client.Object) bool {
643
699
existingIngress := existing.(*networkingv1.Ingress)
644
700
desiredIngress := desired.(*networkingv1.Ingress)
701
+
if reflect.DeepEqual(existingIngress.Spec, desiredIngress.Spec) &&
702
+
reflect.DeepEqual(existingIngress.Annotations, desiredIngress.Annotations) {
703
+
return false
704
+
}
645
705
existingIngress.Spec = desiredIngress.Spec
646
706
existingIngress.Annotations = desiredIngress.Annotations
707
+
return true
647
708
})
648
709
}
649
710
+212
internal/controller/knot_controller_test.go
+212
internal/controller/knot_controller_test.go
···
947
947
})
948
948
}
949
949
950
+
func TestCreateOrUpdateSkipsNoopUpdates(t *testing.T) {
951
+
ctx := context.Background()
952
+
953
+
t.Run("configmap not updated when data unchanged", func(t *testing.T) {
954
+
knot := newMinimalKnot("test-knot", "default")
955
+
knot.Spec.Hostname = "knot.example.com"
956
+
knot.Spec.Owner = "did:plc:test"
957
+
knot.Spec.AppviewEndpoint = "https://tangled.org"
958
+
knot.Spec.Storage.RepoPath = "/data/repos"
959
+
knot.Spec.Storage.DBPath = "/data/db"
960
+
961
+
r := newTestReconciler(knot)
962
+
963
+
err := r.reconcileConfigMap(ctx, knot)
964
+
require.NoError(t, err)
965
+
966
+
cm := &corev1.ConfigMap{}
967
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot-config", Namespace: "default"}, cm)
968
+
require.NoError(t, err)
969
+
rvAfterCreate := cm.ResourceVersion
970
+
971
+
err = r.reconcileConfigMap(ctx, knot)
972
+
require.NoError(t, err)
973
+
974
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot-config", Namespace: "default"}, cm)
975
+
require.NoError(t, err)
976
+
assert.Equal(t, rvAfterCreate, cm.ResourceVersion, "resourceVersion should not change when configmap data is unchanged")
977
+
})
978
+
979
+
t.Run("configmap updated when data changes", func(t *testing.T) {
980
+
knot := newMinimalKnot("test-knot", "default")
981
+
knot.Spec.Hostname = "knot.example.com"
982
+
knot.Spec.Owner = "did:plc:test"
983
+
knot.Spec.AppviewEndpoint = "https://tangled.org"
984
+
knot.Spec.Storage.RepoPath = "/data/repos"
985
+
knot.Spec.Storage.DBPath = "/data/db"
986
+
987
+
r := newTestReconciler(knot)
988
+
989
+
err := r.reconcileConfigMap(ctx, knot)
990
+
require.NoError(t, err)
991
+
992
+
cm := &corev1.ConfigMap{}
993
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot-config", Namespace: "default"}, cm)
994
+
require.NoError(t, err)
995
+
rvAfterCreate := cm.ResourceVersion
996
+
997
+
knot.Spec.Hostname = "new.example.com"
998
+
err = r.reconcileConfigMap(ctx, knot)
999
+
require.NoError(t, err)
1000
+
1001
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot-config", Namespace: "default"}, cm)
1002
+
require.NoError(t, err)
1003
+
assert.NotEqual(t, rvAfterCreate, cm.ResourceVersion, "resourceVersion should change when configmap data changes")
1004
+
assert.Equal(t, "new.example.com", cm.Data["KNOT_SERVER_HOSTNAME"])
1005
+
})
1006
+
1007
+
t.Run("service not updated when ports and selector unchanged", func(t *testing.T) {
1008
+
knot := newMinimalKnot("test-knot", "default")
1009
+
1010
+
r := newTestReconciler(knot)
1011
+
1012
+
err := r.reconcileService(ctx, knot)
1013
+
require.NoError(t, err)
1014
+
1015
+
svc := &corev1.Service{}
1016
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot", Namespace: "default"}, svc)
1017
+
require.NoError(t, err)
1018
+
rvAfterCreate := svc.ResourceVersion
1019
+
1020
+
err = r.reconcileService(ctx, knot)
1021
+
require.NoError(t, err)
1022
+
1023
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot", Namespace: "default"}, svc)
1024
+
require.NoError(t, err)
1025
+
assert.Equal(t, rvAfterCreate, svc.ResourceVersion, "resourceVersion should not change when service is unchanged")
1026
+
})
1027
+
1028
+
t.Run("PVCs never updated", func(t *testing.T) {
1029
+
knot := newMinimalKnot("test-knot", "default")
1030
+
knot.Spec.Storage.RepoSize = "10Gi"
1031
+
knot.Spec.Storage.DBSize = "1Gi"
1032
+
1033
+
r := newTestReconciler(knot)
1034
+
1035
+
err := r.reconcilePVCs(ctx, knot)
1036
+
require.NoError(t, err)
1037
+
1038
+
repoPVC := &corev1.PersistentVolumeClaim{}
1039
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot-repos", Namespace: "default"}, repoPVC)
1040
+
require.NoError(t, err)
1041
+
rvAfterCreate := repoPVC.ResourceVersion
1042
+
1043
+
err = r.reconcilePVCs(ctx, knot)
1044
+
require.NoError(t, err)
1045
+
1046
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot-repos", Namespace: "default"}, repoPVC)
1047
+
require.NoError(t, err)
1048
+
assert.Equal(t, rvAfterCreate, repoPVC.ResourceVersion, "PVC resourceVersion should never change on reconcile")
1049
+
})
1050
+
1051
+
t.Run("deployment not updated when managed fields unchanged", func(t *testing.T) {
1052
+
knot := newMinimalKnot("test-knot", "default")
1053
+
knot.Spec.Image = "test/image:v1"
1054
+
knot.Spec.ImagePullPolicy = corev1.PullAlways
1055
+
knot.Spec.Replicas = 1
1056
+
knot.Spec.Storage.RepoPath = "/data/repos"
1057
+
knot.Spec.Storage.DBPath = "/data/db"
1058
+
1059
+
r := newTestReconciler(knot)
1060
+
1061
+
err := r.reconcileDeployment(ctx, knot)
1062
+
require.NoError(t, err)
1063
+
1064
+
dep := &appsv1.Deployment{}
1065
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot", Namespace: "default"}, dep)
1066
+
require.NoError(t, err)
1067
+
rvAfterCreate := dep.ResourceVersion
1068
+
1069
+
err = r.reconcileDeployment(ctx, knot)
1070
+
require.NoError(t, err)
1071
+
1072
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot", Namespace: "default"}, dep)
1073
+
require.NoError(t, err)
1074
+
assert.Equal(t, rvAfterCreate, dep.ResourceVersion, "resourceVersion should not change when deployment is unchanged")
1075
+
})
1076
+
1077
+
t.Run("deployment updated when image changes", func(t *testing.T) {
1078
+
knot := newMinimalKnot("test-knot", "default")
1079
+
knot.Spec.Image = "test/image:v1"
1080
+
knot.Spec.ImagePullPolicy = corev1.PullAlways
1081
+
knot.Spec.Replicas = 1
1082
+
knot.Spec.Storage.RepoPath = "/data/repos"
1083
+
knot.Spec.Storage.DBPath = "/data/db"
1084
+
1085
+
r := newTestReconciler(knot)
1086
+
1087
+
err := r.reconcileDeployment(ctx, knot)
1088
+
require.NoError(t, err)
1089
+
1090
+
dep := &appsv1.Deployment{}
1091
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot", Namespace: "default"}, dep)
1092
+
require.NoError(t, err)
1093
+
rvAfterCreate := dep.ResourceVersion
1094
+
1095
+
knot.Spec.Image = "test/image:v2"
1096
+
err = r.reconcileDeployment(ctx, knot)
1097
+
require.NoError(t, err)
1098
+
1099
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot", Namespace: "default"}, dep)
1100
+
require.NoError(t, err)
1101
+
assert.NotEqual(t, rvAfterCreate, dep.ResourceVersion, "resourceVersion should change when image changes")
1102
+
assert.Equal(t, "test/image:v2", dep.Spec.Template.Spec.Containers[0].Image)
1103
+
})
1104
+
1105
+
t.Run("deployment updated when replicas change", func(t *testing.T) {
1106
+
knot := newMinimalKnot("test-knot", "default")
1107
+
knot.Spec.Image = "test/image:v1"
1108
+
knot.Spec.Replicas = 1
1109
+
knot.Spec.Storage.RepoPath = "/data/repos"
1110
+
knot.Spec.Storage.DBPath = "/data/db"
1111
+
1112
+
r := newTestReconciler(knot)
1113
+
1114
+
err := r.reconcileDeployment(ctx, knot)
1115
+
require.NoError(t, err)
1116
+
1117
+
dep := &appsv1.Deployment{}
1118
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot", Namespace: "default"}, dep)
1119
+
require.NoError(t, err)
1120
+
rvAfterCreate := dep.ResourceVersion
1121
+
1122
+
knot.Spec.Replicas = 3
1123
+
err = r.reconcileDeployment(ctx, knot)
1124
+
require.NoError(t, err)
1125
+
1126
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot", Namespace: "default"}, dep)
1127
+
require.NoError(t, err)
1128
+
assert.NotEqual(t, rvAfterCreate, dep.ResourceVersion, "resourceVersion should change when replicas change")
1129
+
assert.Equal(t, int32(3), *dep.Spec.Replicas)
1130
+
})
1131
+
1132
+
t.Run("ingress not updated when spec and annotations unchanged", func(t *testing.T) {
1133
+
knot := newMinimalKnot("test-knot", "default")
1134
+
knot.Spec.Hostname = "knot.example.com"
1135
+
knot.Spec.Ingress = &tangledv1alpha1.KnotIngressSpec{
1136
+
Enabled: true,
1137
+
IngressClassName: "nginx",
1138
+
Annotations: map[string]string{
1139
+
"test": "value",
1140
+
},
1141
+
}
1142
+
1143
+
r := newTestReconciler(knot)
1144
+
1145
+
err := r.reconcileIngress(ctx, knot)
1146
+
require.NoError(t, err)
1147
+
1148
+
ingress := &networkingv1.Ingress{}
1149
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot", Namespace: "default"}, ingress)
1150
+
require.NoError(t, err)
1151
+
rvAfterCreate := ingress.ResourceVersion
1152
+
1153
+
err = r.reconcileIngress(ctx, knot)
1154
+
require.NoError(t, err)
1155
+
1156
+
err = r.Get(ctx, types.NamespacedName{Name: "test-knot", Namespace: "default"}, ingress)
1157
+
require.NoError(t, err)
1158
+
assert.Equal(t, rvAfterCreate, ingress.ResourceVersion, "resourceVersion should not change when ingress is unchanged")
1159
+
})
1160
+
}
1161
+
950
1162
func TestLabelsForKnot(t *testing.T) {
951
1163
knot := newMinimalKnot("my-knot", "default")
952
1164
r := &KnotReconciler{}
+22
-3
internal/controller/openshift.go
+22
-3
internal/controller/openshift.go
···
3
3
import (
4
4
"context"
5
5
"fmt"
6
+
"reflect"
6
7
7
8
"k8s.io/apimachinery/pkg/api/errors"
8
9
"k8s.io/apimachinery/pkg/api/meta"
···
106
107
return err
107
108
}
108
109
110
+
if reflect.DeepEqual(existing.Object["spec"], route.Object["spec"]) &&
111
+
reflect.DeepEqual(existing.GetAnnotations(), route.GetAnnotations()) {
112
+
return nil
113
+
}
109
114
existing.Object["spec"] = route.Object["spec"]
110
115
existing.SetAnnotations(route.GetAnnotations())
111
116
logger.V(1).Info("updating route", "route", route.GetName())
···
185
190
return err
186
191
}
187
192
193
+
needsUpdate := false
188
194
for k, v := range sccData {
195
+
if !reflect.DeepEqual(existing.Object[k], v) {
196
+
needsUpdate = true
197
+
}
189
198
if err := unstructured.SetNestedField(existing.Object, v, k); err != nil {
190
199
return err
191
200
}
192
201
}
202
+
desiredAnnotations := map[string]string{
203
+
"tangled.org/managed-by": fmt.Sprintf("%s/%s", knot.Namespace, knot.Name),
204
+
}
205
+
if !reflect.DeepEqual(existing.GetLabels(), r.labelsForKnot(knot)) {
206
+
needsUpdate = true
207
+
}
208
+
if !reflect.DeepEqual(existing.GetAnnotations(), desiredAnnotations) {
209
+
needsUpdate = true
210
+
}
211
+
if !needsUpdate {
212
+
return nil
213
+
}
193
214
existing.SetLabels(r.labelsForKnot(knot))
194
-
existing.SetAnnotations(map[string]string{
195
-
"tangled.org/managed-by": fmt.Sprintf("%s/%s", knot.Namespace, knot.Name),
196
-
})
215
+
existing.SetAnnotations(desiredAnnotations)
197
216
existing.SetOwnerReferences(nil)
198
217
logger.V(1).Info("updating scc", "scc", sccName)
199
218
return r.Update(ctx, existing)