Skip to content

Commit c97f5ac

Browse files
committed
config: Migrate to Server-Side Apply for reconciliation
Replace the Get/Create and Get/Update reconciliation pattern with Server-Side Apply (SSA) using client.Apply patches. This simplifies resource management by handling both creation and updates in a single operation while providing better conflict resolution and field ownership tracking. Changes: - Add TypeMeta fields to all Kubernetes objects for proper SSA support - Change assureResource logic to use r.Patch with client.Apply - Add test interceptor to handle SSA patches in fake client Signed-off-by: Andreas Karis <[email protected]>
1 parent 0b557d9 commit c97f5ac

File tree

2 files changed

+76
-29
lines changed

2 files changed

+76
-29
lines changed

controllers/bpfman-operator/config.go

Lines changed: 47 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,15 @@ func (r *BpfmanConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request
149149
}
150150

151151
func (r *BpfmanConfigReconciler) reconcileCM(ctx context.Context, bpfmanConfig *v1alpha1.Config) error {
152-
cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{
153-
Name: internal.BpfmanCmName,
154-
Namespace: bpfmanConfig.Spec.Namespace},
152+
cm := &corev1.ConfigMap{
153+
TypeMeta: metav1.TypeMeta{
154+
Kind: "ConfigMap",
155+
APIVersion: "v1",
156+
},
157+
ObjectMeta: metav1.ObjectMeta{
158+
Name: internal.BpfmanCmName,
159+
Namespace: bpfmanConfig.Spec.Namespace,
160+
},
155161
Data: map[string]string{
156162
internal.BpfmanTOML: bpfmanConfig.Spec.Configuration,
157163
internal.BpfmanAgentLogLevel: bpfmanConfig.Spec.Agent.LogLevel,
@@ -164,7 +170,13 @@ func (r *BpfmanConfigReconciler) reconcileCM(ctx context.Context, bpfmanConfig *
164170
}
165171

166172
func (r *BpfmanConfigReconciler) reconcileCSIDriver(ctx context.Context, bpfmanConfig *v1alpha1.Config) error {
167-
csiDriver := &storagev1.CSIDriver{ObjectMeta: metav1.ObjectMeta{Name: internal.BpfmanCsiDriverName}}
173+
csiDriver := &storagev1.CSIDriver{
174+
TypeMeta: metav1.TypeMeta{
175+
Kind: "CSIDriver",
176+
APIVersion: "storage.k8s.io/v1",
177+
},
178+
ObjectMeta: metav1.ObjectMeta{Name: internal.BpfmanCsiDriverName},
179+
}
168180
r.Logger.Info("Loading object", "object", csiDriver.Name, "path", r.CsiDriverDS)
169181
csiDriver, err := load(csiDriver, r.CsiDriverDS, csiDriver.Name)
170182
if err != nil {
@@ -178,6 +190,10 @@ func (r *BpfmanConfigReconciler) reconcileCSIDriver(ctx context.Context, bpfmanC
178190
func (r *BpfmanConfigReconciler) reconcileSCC(ctx context.Context, bpfmanConfig *v1alpha1.Config) error {
179191
if r.IsOpenshift {
180192
bpfmanRestrictedSCC := &osv1.SecurityContextConstraints{
193+
TypeMeta: metav1.TypeMeta{
194+
Kind: "SecurityContextConstraints",
195+
APIVersion: "security.openshift.io/v1",
196+
},
181197
ObjectMeta: metav1.ObjectMeta{
182198
Name: internal.BpfmanRestrictedSccName,
183199
},
@@ -190,16 +206,22 @@ func (r *BpfmanConfigReconciler) reconcileSCC(ctx context.Context, bpfmanConfig
190206
return assureResource(ctx, r, bpfmanConfig, bpfmanRestrictedSCC, func(existing, desired *osv1.SecurityContextConstraints) bool {
191207
existingCopy := existing.DeepCopy()
192208
desiredCopy := desired.DeepCopy()
193-
existingCopy.ObjectMeta = metav1.ObjectMeta{}
194-
desiredCopy.ObjectMeta = metav1.ObjectMeta{}
209+
existingCopy.TypeMeta = desired.TypeMeta
210+
existingCopy.ObjectMeta = desired.ObjectMeta
195211
return !equality.Semantic.DeepEqual(existingCopy, desiredCopy)
196212
})
197213
}
198214
return nil
199215
}
200216

201217
func (r *BpfmanConfigReconciler) reconcileStandardDS(ctx context.Context, bpfmanConfig *v1alpha1.Config) error {
202-
bpfmanDS := &appsv1.DaemonSet{ObjectMeta: metav1.ObjectMeta{Name: internal.BpfmanDsName}}
218+
bpfmanDS := &appsv1.DaemonSet{
219+
TypeMeta: metav1.TypeMeta{
220+
Kind: "DaemonSet",
221+
APIVersion: "apps/v1",
222+
},
223+
ObjectMeta: metav1.ObjectMeta{Name: internal.BpfmanDsName},
224+
}
203225
r.Logger.Info("Loading object", "object", bpfmanDS.Name, "path", r.BpfmanStandardDS)
204226
bpfmanDS, err := load(bpfmanDS, r.BpfmanStandardDS, bpfmanDS.Name)
205227
if err != nil {
@@ -213,7 +235,13 @@ func (r *BpfmanConfigReconciler) reconcileStandardDS(ctx context.Context, bpfman
213235

214236
func (r *BpfmanConfigReconciler) reconcileMetricsProxyDS(ctx context.Context, bpfmanConfig *v1alpha1.Config) error {
215237
// Reconcile metrics-proxy daemonset
216-
metricsProxyDS := &appsv1.DaemonSet{ObjectMeta: metav1.ObjectMeta{Name: internal.BpfmanMetricsProxyDsName}}
238+
metricsProxyDS := &appsv1.DaemonSet{
239+
TypeMeta: metav1.TypeMeta{
240+
Kind: "DaemonSet",
241+
APIVersion: "apps/v1",
242+
},
243+
ObjectMeta: metav1.ObjectMeta{Name: internal.BpfmanMetricsProxyDsName},
244+
}
217245
r.Logger.Info("Loading object", "object", metricsProxyDS.Name, "path", r.BpfmanMetricsProxyDS)
218246
metricsProxyDS, err := load(metricsProxyDS, r.BpfmanMetricsProxyDS, metricsProxyDS.Name)
219247
if err != nil {
@@ -370,39 +398,32 @@ func load[T client.Object](t T, path, name string) (T, error) {
370398
}
371399

372400
// assureResource ensures a Kubernetes resource exists and is up to date.
373-
// Creates the resource if it doesn't exist, otherwise updates it to match the desired state.
401+
// SSA patch creates the resource if it doesn't exist, otherwise updates it to match the desired state.
374402
func assureResource[T client.Object](ctx context.Context, r *BpfmanConfigReconciler,
375403
bpfmanConfig *v1alpha1.Config, resource T, needsUpdate func(existing T, desired T) bool) error {
376404
if err := ctrl.SetControllerReference(bpfmanConfig, resource, r.Scheme); err != nil {
377405
return err
378406
}
379407

380-
objectKey := types.NamespacedName{Namespace: resource.GetNamespace(), Name: resource.GetName()}
381408
r.Logger.Info("Getting object",
382409
"type", resource.GetObjectKind(), "namespace", resource.GetNamespace(), "name", resource.GetName())
410+
objectKey := types.NamespacedName{Namespace: resource.GetNamespace(), Name: resource.GetName()}
383411
existingResource := resource.DeepCopyObject().(T)
412+
found := true
384413
if err := r.Get(ctx, objectKey, existingResource); err != nil {
385-
if errors.IsNotFound(err) {
386-
r.Logger.Info("Creating object",
414+
if !errors.IsNotFound(err) {
415+
r.Logger.Error(err, "Failed to get object",
387416
"type", resource.GetObjectKind(), "namespace", resource.GetNamespace(), "name", resource.GetName())
388-
if err := r.Create(ctx, resource); err != nil {
389-
r.Logger.Error(err, "Failed to create object",
390-
"type", resource.GetObjectKind(), "namespace", resource.GetNamespace(), "name", resource.GetName())
391-
return err
392-
}
393-
return nil
417+
return err
394418
}
395-
r.Logger.Error(err, "Failed to get object",
396-
"type", resource.GetObjectKind(), "namespace", resource.GetNamespace(), "name", resource.GetName())
397-
return err
419+
found = false
398420
}
399421

400-
if needsUpdate(existingResource, resource) {
401-
r.Logger.Info("Updating object",
422+
if !found || needsUpdate(existingResource, resource) {
423+
r.Logger.Info("Patching object",
402424
"type", resource.GetObjectKind(), "namespace", resource.GetNamespace(), "name", resource.GetName())
403-
resource.SetResourceVersion(existingResource.GetResourceVersion())
404-
if err := r.Update(ctx, resource); err != nil {
405-
r.Logger.Error(err, "Failed updating object",
425+
if err := r.Patch(ctx, resource, client.Apply, client.ForceOwnership, client.FieldOwner(bpfmanConfig.Name)); err != nil {
426+
r.Logger.Error(err, "Failed patching object",
406427
"type", resource.GetObjectKind(), "namespace", resource.GetNamespace(), "name", resource.GetName())
407428
return err
408429
}

controllers/bpfman-operator/config_test.go

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,15 @@ import (
2929
appsv1 "k8s.io/api/apps/v1"
3030
corev1 "k8s.io/api/core/v1"
3131
storagev1 "k8s.io/api/storage/v1"
32+
"k8s.io/apimachinery/pkg/api/errors"
3233
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3334
"k8s.io/apimachinery/pkg/runtime"
3435
"k8s.io/apimachinery/pkg/types"
3536
"k8s.io/client-go/kubernetes/scheme"
3637
"k8s.io/utils/ptr"
3738
"sigs.k8s.io/controller-runtime/pkg/client"
3839
"sigs.k8s.io/controller-runtime/pkg/client/fake"
40+
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
3941
logf "sigs.k8s.io/controller-runtime/pkg/log"
4042
"sigs.k8s.io/controller-runtime/pkg/log/zap"
4143
"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -207,12 +209,36 @@ func setupTestEnvironment(isOpenShift bool) (*BpfmanConfigReconciler, *v1alpha1.
207209
s.AddKnownTypes(storagev1.SchemeGroupVersion, &storagev1.CSIDriver{})
208210
s.AddKnownTypes(osv1.GroupVersion, &osv1.SecurityContextConstraints{})
209211

210-
// Create a fake client to mock API calls.
211-
cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build()
212-
213212
// Set development Logger so we can see all logs in tests.
214213
logf.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{Development: true})))
215214

215+
// Create an interceptor that fakes Server-Side Apply Patch calls.
216+
// This is needed until the upgrade to a version of sigs.k8s.io/controller-runtime/pkg/client/fake that fully
217+
// supports r.Patch(..., client.Apply, ...) and/or r.Apply().
218+
// See: https://github.com/kubernetes-sigs/controller-runtime/issues/2341#issuecomment-1560118000
219+
// https://github.com/kubernetes/kubernetes/issues/115598
220+
fns := interceptor.Funcs{
221+
Patch: func(ctx context.Context, c client.WithWatch, obj client.Object, patch client.Patch, opts ...client.PatchOption) error {
222+
if patch.Type() != types.ApplyPatchType {
223+
return c.Patch(ctx, obj, patch, opts...)
224+
}
225+
existing, ok := obj.DeepCopyObject().(client.Object)
226+
if !ok {
227+
return fmt.Errorf("could not check for object in fake client")
228+
}
229+
if err := c.Get(ctx, client.ObjectKeyFromObject(obj), existing); errors.IsNotFound(err) {
230+
if err := c.Create(ctx, existing); err != nil {
231+
return fmt.Errorf("could not create object: %w", err)
232+
}
233+
}
234+
obj.SetResourceVersion(existing.GetResourceVersion())
235+
return c.Update(ctx, obj)
236+
},
237+
}
238+
239+
// Create a fake client to mock API calls.
240+
cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).WithInterceptorFuncs(fns).Build()
241+
216242
rc := ReconcilerCommon[v1alpha1.ClusterBpfApplicationState, v1alpha1.ClusterBpfApplicationStateList]{
217243
Client: cl,
218244
Scheme: s,

0 commit comments

Comments
 (0)