Skip to content

Commit 77df0f4

Browse files
committed
operator: fix endless reconciliation loops
some objects stay on a endless reconciliation loops because we don't configure all the fields and they will differ later when k8s updates them to defaults. this commit fixes such problem.
1 parent e5b7e5d commit 77df0f4

File tree

6 files changed

+548
-108
lines changed

6 files changed

+548
-108
lines changed

Makefile

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,9 +174,14 @@ uninstall: manifests kustomize ## Uninstall CRDs from the K8s cluster specified
174174
deploy: docker-build cluster grpcurl
175175
./hack/deploy_with_helm.sh
176176

177+
.PHONY: deploy-with-operator
177178
deploy-with-operator: docker-build build-operator cluster grpcurl
178179
./hack/deploy_with_operator.sh
179180

181+
.PHONY: operator-logs
182+
operator-logs:
183+
kubectl logs -n jumpstarter-operator-system -l app.kubernetes.io/name=jumpstarter-operator -f
184+
180185
deploy-with-operator-parallel:
181186
make deploy-with-operator -j5 --output-sync=target
182187

deploy/operator/go.mod

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@ module github.com/jumpstarter-dev/jumpstarter-controller/deploy/operator
33
go 1.24.0
44

55
require (
6+
github.com/go-logr/logr v1.4.2
67
github.com/jumpstarter-dev/jumpstarter-controller v0.7.1
78
github.com/onsi/ginkgo/v2 v2.22.2
89
github.com/onsi/gomega v1.36.2
10+
github.com/pmezard/go-difflib v1.0.0
911
k8s.io/api v0.33.0
1012
k8s.io/apimachinery v0.33.0
1113
k8s.io/apiserver v0.33.0
@@ -42,7 +44,6 @@ require (
4244
github.com/gin-gonic/gin v1.10.0 // indirect
4345
github.com/go-chi/chi/v5 v5.2.0 // indirect
4446
github.com/go-jose/go-jose/v4 v4.0.4 // indirect
45-
github.com/go-logr/logr v1.4.2 // indirect
4647
github.com/go-logr/stdr v1.2.2 // indirect
4748
github.com/go-logr/zapr v1.3.0 // indirect
4849
github.com/go-openapi/jsonpointer v0.21.0 // indirect
Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
/*
2+
Copyright 2025.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package jumpstarter
18+
19+
import (
20+
"fmt"
21+
22+
"github.com/go-logr/logr"
23+
"github.com/pmezard/go-difflib/difflib"
24+
appsv1 "k8s.io/api/apps/v1"
25+
corev1 "k8s.io/api/core/v1"
26+
rbacv1 "k8s.io/api/rbac/v1"
27+
"k8s.io/apimachinery/pkg/api/equality"
28+
"sigs.k8s.io/yaml"
29+
)
30+
31+
// deploymentNeedsUpdate checks if a deployment needs to be updated using K8s semantic equality.
32+
func deploymentNeedsUpdate(existing, desired *appsv1.Deployment) bool {
33+
// Compare labels (only if desired.Labels is non-nil)
34+
if desired.Labels != nil && !equality.Semantic.DeepEqual(existing.Labels, desired.Labels) {
35+
return true
36+
}
37+
38+
// Compare annotations (only if desired.Annotations is non-nil)
39+
if desired.Annotations != nil && !equality.Semantic.DeepEqual(existing.Annotations, desired.Annotations) {
40+
return true
41+
}
42+
43+
// Compare the entire Spec using K8s semantic equality (handles nil vs empty automatically)
44+
return !equality.Semantic.DeepEqual(existing.Spec, desired.Spec)
45+
}
46+
47+
// configMapNeedsUpdate checks if a configmap needs to be updated using K8s semantic equality.
48+
func configMapNeedsUpdate(existing, desired *corev1.ConfigMap, log logr.Logger) bool {
49+
// Compare labels (only if desired.Labels is non-nil)
50+
if desired.Labels != nil && !equality.Semantic.DeepEqual(existing.Labels, desired.Labels) {
51+
return true
52+
}
53+
54+
// Compare annotations (only if desired.Annotations is non-nil)
55+
if desired.Annotations != nil && !equality.Semantic.DeepEqual(existing.Annotations, desired.Annotations) {
56+
return true
57+
}
58+
59+
// Compare data (only if desired.Data is non-nil)
60+
if desired.Data != nil && !equality.Semantic.DeepEqual(existing.Data, desired.Data) {
61+
return true
62+
}
63+
64+
// Compare binary data (only if desired.BinaryData is non-nil)
65+
if desired.BinaryData != nil && !equality.Semantic.DeepEqual(existing.BinaryData, desired.BinaryData) {
66+
return true
67+
}
68+
69+
return false
70+
}
71+
72+
// serviceAccountNeedsUpdate checks if a service account needs to be updated using K8s semantic equality.
73+
func serviceAccountNeedsUpdate(existing, desired *corev1.ServiceAccount) bool {
74+
// Compare labels (only if desired.Labels is non-nil)
75+
if desired.Labels != nil && !equality.Semantic.DeepEqual(existing.Labels, desired.Labels) {
76+
return true
77+
}
78+
79+
// Compare annotations (only if desired.Annotations is non-nil)
80+
if desired.Annotations != nil && !equality.Semantic.DeepEqual(existing.Annotations, desired.Annotations) {
81+
return true
82+
}
83+
84+
return false
85+
}
86+
87+
// roleNeedsUpdate checks if a role needs to be updated using K8s semantic equality.
88+
func roleNeedsUpdate(existing, desired *rbacv1.Role) bool {
89+
// Compare labels (only if desired.Labels is non-nil)
90+
if desired.Labels != nil && !equality.Semantic.DeepEqual(existing.Labels, desired.Labels) {
91+
return true
92+
}
93+
94+
// Compare annotations (only if desired.Annotations is non-nil)
95+
if desired.Annotations != nil && !equality.Semantic.DeepEqual(existing.Annotations, desired.Annotations) {
96+
return true
97+
}
98+
99+
// Compare rules (only if non-nil in desired)
100+
if desired.Rules != nil && !equality.Semantic.DeepEqual(existing.Rules, desired.Rules) {
101+
return true
102+
}
103+
104+
return false
105+
}
106+
107+
// roleBindingNeedsUpdate checks if a role binding needs to be updated using K8s semantic equality.
108+
func roleBindingNeedsUpdate(existing, desired *rbacv1.RoleBinding) bool {
109+
// Compare labels (only if desired.Labels is non-nil)
110+
if desired.Labels != nil && !equality.Semantic.DeepEqual(existing.Labels, desired.Labels) {
111+
return true
112+
}
113+
114+
// Compare annotations (only if desired.Annotations is non-nil)
115+
if desired.Annotations != nil && !equality.Semantic.DeepEqual(existing.Annotations, desired.Annotations) {
116+
return true
117+
}
118+
119+
// Compare subjects (only if non-nil in desired)
120+
if desired.Subjects != nil && !equality.Semantic.DeepEqual(existing.Subjects, desired.Subjects) {
121+
return true
122+
}
123+
124+
// Compare role ref (only if non-zero in desired)
125+
if desired.RoleRef.Name != "" && !equality.Semantic.DeepEqual(existing.RoleRef, desired.RoleRef) {
126+
return true
127+
}
128+
129+
return false
130+
}
131+
132+
// generateDiff creates a unified diff between existing and desired resources.
133+
// It works with any Kubernetes resource type.
134+
// Returns the diff string and any error encountered during serialization.
135+
func generateDiff[T any](existing, desired *T) (string, error) {
136+
// Serialize existing resource to YAML
137+
existingYAML, err := yaml.Marshal(existing)
138+
if err != nil {
139+
return "", fmt.Errorf("failed to marshal existing resource: %w", err)
140+
}
141+
142+
// Serialize desired resource to YAML
143+
desiredYAML, err := yaml.Marshal(desired)
144+
if err != nil {
145+
return "", fmt.Errorf("failed to marshal desired resource: %w", err)
146+
}
147+
148+
// Generate unified diff
149+
diff := difflib.UnifiedDiff{
150+
A: difflib.SplitLines(string(existingYAML)),
151+
B: difflib.SplitLines(string(desiredYAML)),
152+
FromFile: "Existing",
153+
ToFile: "Desired",
154+
Context: 3,
155+
}
156+
157+
return difflib.GetUnifiedDiffString(diff)
158+
}

deploy/operator/internal/controller/jumpstarter/endpoints/endpoints.go

Lines changed: 24 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -49,30 +49,26 @@ func NewReconciler(client client.Client, scheme *runtime.Scheme) *Reconciler {
4949
func (r *Reconciler) createOrUpdateService(ctx context.Context, service *corev1.Service, owner metav1.Object) error {
5050
log := logf.FromContext(ctx)
5151

52-
if owner != nil {
53-
if err := controllerutil.SetControllerReference(owner, service, r.Scheme); err != nil {
54-
return err
55-
}
56-
}
57-
5852
existingService := &corev1.Service{}
5953
existingService.Name = service.Name
6054
existingService.Namespace = service.Namespace
6155

56+
if err := controllerutil.SetControllerReference(owner, service, r.Scheme); err != nil {
57+
return err
58+
}
59+
6260
op, err := controllerutil.CreateOrUpdate(ctx, r.Client, existingService, func() error {
6361
// Preserve immutable fields if service already exists
6462
if existingService.CreationTimestamp.IsZero() {
6563
// Service is being created, copy all fields from desired service
6664
existingService.Spec = service.Spec
6765
existingService.Labels = service.Labels
6866
existingService.Annotations = service.Annotations
69-
} else {
70-
// Service exists, preserve immutable fields
71-
preservedClusterIP := existingService.Spec.ClusterIP
72-
preservedClusterIPs := existingService.Spec.ClusterIPs
73-
preservedIPFamilies := existingService.Spec.IPFamilies
74-
preservedIPFamilyPolicy := existingService.Spec.IPFamilyPolicy
67+
if err := controllerutil.SetControllerReference(owner, service, r.Scheme); err != nil {
68+
return err
69+
}
7570

71+
} else {
7672
// Preserve existing NodePorts to prevent "port already allocated" errors
7773
if service.Spec.Type == corev1.ServiceTypeNodePort || service.Spec.Type == corev1.ServiceTypeLoadBalancer {
7874
for i := range existingService.Spec.Ports {
@@ -83,15 +79,18 @@ func (r *Reconciler) createOrUpdateService(ctx context.Context, service *corev1.
8379
}
8480

8581
// Update all mutable fields
86-
existingService.Spec = service.Spec
82+
if service.Spec.LoadBalancerClass != nil && *service.Spec.LoadBalancerClass != "" {
83+
existingService.Spec.LoadBalancerClass = service.Spec.LoadBalancerClass
84+
}
85+
if service.Spec.ExternalTrafficPolicy != "" {
86+
existingService.Spec.ExternalTrafficPolicy = service.Spec.ExternalTrafficPolicy
87+
}
88+
89+
existingService.Spec.Ports = service.Spec.Ports
90+
existingService.Spec.Selector = service.Spec.Selector
91+
existingService.Spec.Type = service.Spec.Type
8792
existingService.Labels = service.Labels
8893
existingService.Annotations = service.Annotations
89-
90-
// Restore immutable fields
91-
existingService.Spec.ClusterIP = preservedClusterIP
92-
existingService.Spec.ClusterIPs = preservedClusterIPs
93-
existingService.Spec.IPFamilies = preservedIPFamilies
94-
existingService.Spec.IPFamilyPolicy = preservedIPFamilyPolicy
9594
}
9695

9796
return nil
@@ -105,25 +104,12 @@ func (r *Reconciler) createOrUpdateService(ctx context.Context, service *corev1.
105104
return err
106105
}
107106

108-
// Log the operation result
109-
switch op {
110-
case controllerutil.OperationResultCreated:
111-
log.Info("Created service",
112-
"name", service.Name,
113-
"namespace", service.Namespace,
114-
"type", service.Spec.Type,
115-
"selector", service.Spec.Selector)
116-
case controllerutil.OperationResultUpdated:
117-
log.Info("Updated service",
118-
"name", service.Name,
119-
"namespace", service.Namespace,
120-
"type", service.Spec.Type,
121-
"selector", service.Spec.Selector)
122-
case controllerutil.OperationResultNone:
123-
log.V(1).Info("Service already up to date",
124-
"name", service.Name,
125-
"namespace", service.Namespace)
126-
}
107+
log.Info("Service reconciled",
108+
"name", service.Name,
109+
"namespace", service.Namespace,
110+
"type", service.Spec.Type,
111+
"selector", service.Spec.Selector,
112+
"operation", op)
127113

128114
return nil
129115
}

0 commit comments

Comments
 (0)