Use patcher in addon manager code (#198)

Signed-off-by: Jian Qiu <jqiu@redhat.com>

pkg/addon/controllers/addonconfiguration/addon_configuration_reconciler.go

@@ -2,18 +2,13 @@ package addonconfiguration
 
 import (
 	"context"
-	"encoding/json"
-	"fmt"
 
-	jsonpatch "github.com/evanphx/json-patch"
-	"k8s.io/apimachinery/pkg/api/equality"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
-	"k8s.io/klog/v2"
 
 	addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
 	addonv1alpha1client "open-cluster-management.io/api/client/addon/clientset/versioned"
+
+	"open-cluster-management.io/ocm/pkg/common/patcher"
 )
 
 type managedClusterAddonConfigurationReconciler struct {
@@ -26,7 +21,10 @@ func (d *managedClusterAddonConfigurationReconciler) reconcile(
 
 	for _, addon := range graph.addonToUpdate() {
 		mca := d.mergeAddonConfig(addon.mca, addon.desiredConfigs)
-		err := d.patchAddonStatus(ctx, mca, addon.mca)
+		patcher := patcher.NewPatcher[
+			*addonv1alpha1.ManagedClusterAddOn, addonv1alpha1.ManagedClusterAddOnSpec, addonv1alpha1.ManagedClusterAddOnStatus](
+			d.addonClient.AddonV1alpha1().ManagedClusterAddOns(mca.Namespace))
+		_, err := patcher.PatchStatus(ctx, mca, mca.Status, addon.mca.Status)
 		if err != nil {
 			errs = append(errs, err)
 		}
@@ -72,43 +70,3 @@ func (d *managedClusterAddonConfigurationReconciler) mergeAddonConfig(
 	mcaCopy.Status.ConfigReferences = mergedConfigs
 	return mcaCopy
 }
-
-func (d *managedClusterAddonConfigurationReconciler) patchAddonStatus(ctx context.Context, new, old *addonv1alpha1.ManagedClusterAddOn) error {
-	if equality.Semantic.DeepEqual(new.Status, old.Status) {
-		return nil
-	}
-
-	oldData, err := json.Marshal(&addonv1alpha1.ManagedClusterAddOn{
-		Status: addonv1alpha1.ManagedClusterAddOnStatus{
-			Namespace:        old.Status.Namespace,
-			ConfigReferences: old.Status.ConfigReferences,
-		},
-	})
-	if err != nil {
-		return err
-	}
-
-	newData, err := json.Marshal(&addonv1alpha1.ManagedClusterAddOn{
-		ObjectMeta: metav1.ObjectMeta{
-			UID:             new.UID,
-			ResourceVersion: new.ResourceVersion,
-		},
-		Status: addonv1alpha1.ManagedClusterAddOnStatus{
-			Namespace:        new.Status.Namespace,
-			ConfigReferences: new.Status.ConfigReferences,
-		},
-	})
-	if err != nil {
-		return err
-	}
-
-	patchBytes, err := jsonpatch.CreateMergePatch(oldData, newData)
-	if err != nil {
-		return fmt.Errorf("failed to create patch for addon %s: %w", new.Name, err)
-	}
-
-	klog.V(2).Infof("Patching addon %s/%s status with %s", new.Namespace, new.Name, string(patchBytes))
-	_, err = d.addonClient.AddonV1alpha1().ManagedClusterAddOns(new.Namespace).Patch(
-		ctx, new.Name, types.MergePatchType, patchBytes, metav1.PatchOptions{}, "status")
-	return err
-}

pkg/addon/controllers/addonconfiguration/controller.go

@@ -20,6 +20,8 @@ import (
 	clusterinformersv1beta1 "open-cluster-management.io/api/client/cluster/informers/externalversions/cluster/v1beta1"
 	clusterlisterv1beta1 "open-cluster-management.io/api/client/cluster/listers/cluster/v1beta1"
 	clusterv1beta1 "open-cluster-management.io/api/cluster/v1beta1"
+
+	"open-cluster-management.io/ocm/pkg/common/patcher"
 )
 
 // addonConfigurationController is a controller to update configuration of mca with the following order
@@ -70,7 +72,9 @@ func NewAddonConfigurationController(
 			addonClient: addonClient,
 		},
 		&clusterManagementAddonProgressingReconciler{
-			addonClient: addonClient,
+			patcher: patcher.NewPatcher[
+				*addonv1alpha1.ClusterManagementAddOn, addonv1alpha1.ClusterManagementAddOnSpec, addonv1alpha1.ClusterManagementAddOnStatus](
+				addonClient.AddonV1alpha1().ClusterManagementAddOns()),
 		},
 	}
 

pkg/addon/controllers/addonconfiguration/mgmt_addon_progressing_reconciler.go

@@ -2,23 +2,21 @@ package addonconfiguration
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 
-	jsonpatch "github.com/evanphx/json-patch"
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
-	"k8s.io/klog/v2"
 
 	addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
-	addonv1alpha1client "open-cluster-management.io/api/client/addon/clientset/versioned"
+
+	"open-cluster-management.io/ocm/pkg/common/patcher"
 )
 
 type clusterManagementAddonProgressingReconciler struct {
-	addonClient addonv1alpha1client.Interface
+	patcher patcher.Patcher[
+		*addonv1alpha1.ClusterManagementAddOn, addonv1alpha1.ClusterManagementAddOnSpec, addonv1alpha1.ClusterManagementAddOnStatus]
 }
 
 func (d *clusterManagementAddonProgressingReconciler) reconcile(
@@ -51,51 +49,13 @@ func (d *clusterManagementAddonProgressingReconciler) reconcile(
 		)
 	}
 
-	err := d.patchMgmtAddonStatus(ctx, cmaCopy, cma)
+	_, err := d.patcher.PatchStatus(ctx, cmaCopy, cmaCopy.Status, cma.Status)
 	if err != nil {
 		errs = append(errs, err)
 	}
 	return cmaCopy, reconcileContinue, utilerrors.NewAggregate(errs)
 }
 
-func (d *clusterManagementAddonProgressingReconciler) patchMgmtAddonStatus(ctx context.Context, new, old *addonv1alpha1.ClusterManagementAddOn) error {
-	if equality.Semantic.DeepEqual(new.Status, old.Status) {
-		return nil
-	}
-
-	oldData, err := json.Marshal(&addonv1alpha1.ClusterManagementAddOn{
-		Status: addonv1alpha1.ClusterManagementAddOnStatus{
-			InstallProgressions: old.Status.InstallProgressions,
-		},
-	})
-	if err != nil {
-		return err
-	}
-
-	newData, err := json.Marshal(&addonv1alpha1.ClusterManagementAddOn{
-		ObjectMeta: metav1.ObjectMeta{
-			UID:             new.UID,
-			ResourceVersion: new.ResourceVersion,
-		},
-		Status: addonv1alpha1.ClusterManagementAddOnStatus{
-			InstallProgressions: new.Status.InstallProgressions,
-		},
-	})
-	if err != nil {
-		return err
-	}
-
-	patchBytes, err := jsonpatch.CreateMergePatch(oldData, newData)
-	if err != nil {
-		return fmt.Errorf("failed to create patch for addon %s: %w", new.Name, err)
-	}
-
-	klog.V(2).Infof("Patching clustermanagementaddon %s status with %s", new.Name, string(patchBytes))
-	_, err = d.addonClient.AddonV1alpha1().ClusterManagementAddOns().Patch(
-		ctx, new.Name, types.MergePatchType, patchBytes, metav1.PatchOptions{}, "status")
-	return err
-}
-
 func setAddOnInstallProgressionsAndLastApplied(installProgression *addonv1alpha1.InstallProgression, isUpgrade bool, progressing, done, total int) {
 	// always update progressing condition when there is no config
 	// skip update progressing condition when last applied config already the same as desired

pkg/addon/controllers/addonconfiguration/mgmt_addon_progressing_reconciler_test.go

@@ -20,6 +20,8 @@ import (
 	fakecluster "open-cluster-management.io/api/client/cluster/clientset/versioned/fake"
 	clusterv1informers "open-cluster-management.io/api/client/cluster/informers/externalversions"
 	clusterv1beta1 "open-cluster-management.io/api/cluster/v1beta1"
+
+	"open-cluster-management.io/ocm/pkg/common/patcher"
 )
 
 func TestMgmtAddonProgressingReconcile(t *testing.T) {
@@ -605,7 +607,9 @@ func TestMgmtAddonProgressingReconcile(t *testing.T) {
 			}
 
 			reconcile := &clusterManagementAddonProgressingReconciler{
-				addonClient: fakeAddonClient,
+				patcher.NewPatcher[
+					*addonv1alpha1.ClusterManagementAddOn, addonv1alpha1.ClusterManagementAddOnSpec, addonv1alpha1.ClusterManagementAddOnStatus](
+					fakeAddonClient.AddonV1alpha1().ClusterManagementAddOns()),
 			}
 
 			for _, obj := range c.clusterManagementAddon {

pkg/addon/controllers/addonprogressing/controller.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"strings"
 
-	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/openshift/library-go/pkg/controller/factory"
 	"github.com/openshift/library-go/pkg/operator/events"
 	"k8s.io/apimachinery/pkg/api/equality"
@@ -16,7 +15,6 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/selection"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/klog/v2"
 
@@ -29,6 +27,8 @@ import (
 	workinformers "open-cluster-management.io/api/client/work/informers/externalversions/work/v1"
 	worklister "open-cluster-management.io/api/client/work/listers/work/v1"
 	workapiv1 "open-cluster-management.io/api/work/v1"
+
+	"open-cluster-management.io/ocm/pkg/common/patcher"
 )
 
 const (
@@ -111,10 +111,15 @@ func (c *addonProgressingController) sync(ctx context.Context, syncCtx factory.S
 	}
 
 	// update progressing condition and last applied config
-	return c.updateAddonProgressingAndLastApplied(ctx, addon.DeepCopy(), addon)
+	_, err = c.updateAddonProgressingAndLastApplied(ctx, addon.DeepCopy(), addon)
+	return err
 }
 
-func (c *addonProgressingController) updateAddonProgressingAndLastApplied(ctx context.Context, newaddon, oldaddon *addonapiv1alpha1.ManagedClusterAddOn) error {
+func (c *addonProgressingController) updateAddonProgressingAndLastApplied(
+	ctx context.Context, newaddon, oldaddon *addonapiv1alpha1.ManagedClusterAddOn) (bool, error) {
+	patcher := patcher.NewPatcher[
+		*addonapiv1alpha1.ManagedClusterAddOn, addonapiv1alpha1.ManagedClusterAddOnSpec, addonapiv1alpha1.ManagedClusterAddOnStatus](
+		c.addonClient.AddonV1alpha1().ManagedClusterAddOns(newaddon.Namespace))
 	// check config references
 	if supported, config := isConfigurationSupported(newaddon); !supported {
 		meta.SetStatusCondition(&newaddon.Status.Conditions, metav1.Condition{
@@ -123,7 +128,8 @@ func (c *addonProgressingController) updateAddonProgressingAndLastApplied(ctx co
 			Reason:  addonapiv1alpha1.ProgressingReasonConfigurationUnsupported,
 			Message: fmt.Sprintf("Configuration with gvr %s/%s is not supported for this addon", config.Group, config.Resource),
 		})
-		return c.patchAddOnProgressingAndLastApplied(ctx, newaddon, oldaddon)
+
+		return patcher.PatchStatus(ctx, newaddon, newaddon.Status, oldaddon.Status)
 	}
 
 	// wait until addon has ManifestApplied condition
@@ -134,7 +140,7 @@ func (c *addonProgressingController) updateAddonProgressingAndLastApplied(ctx co
 			Reason:  "WaitingForManifestApplied",
 			Message: "Waiting for ManagedClusterAddOn ManifestApplied condition",
 		})
-		return c.patchAddOnProgressingAndLastApplied(ctx, newaddon, oldaddon)
+		return patcher.PatchStatus(ctx, newaddon, newaddon.Status, oldaddon.Status)
 	}
 
 	// set upgrade flag
@@ -153,12 +159,12 @@ func (c *addonProgressingController) updateAddonProgressingAndLastApplied(ctx co
 	addonWorks, err := c.workLister.ManifestWorks(newaddon.Namespace).List(selector)
 	if err != nil {
 		setAddOnProgressingAndLastApplied(isUpgrade, ProgressingFailed, err.Error(), newaddon)
-		return c.patchAddOnProgressingAndLastApplied(ctx, newaddon, oldaddon)
+		return patcher.PatchStatus(ctx, newaddon, newaddon.Status, oldaddon.Status)
 	}
 
 	if len(addonWorks) == 0 {
 		setAddOnProgressingAndLastApplied(isUpgrade, ProgressingDoing, "no addon works", newaddon)
-		return c.patchAddOnProgressingAndLastApplied(ctx, newaddon, oldaddon)
+		return patcher.PatchStatus(ctx, newaddon, newaddon.Status, oldaddon.Status)
 	}
 
 	// check addon manifestworks
@@ -171,60 +177,19 @@ func (c *addonProgressingController) updateAddonProgressingAndLastApplied(ctx co
 		// check if work configs matches addon configs
 		if !workConfigsMatchesAddon(work, newaddon) {
 			setAddOnProgressingAndLastApplied(isUpgrade, ProgressingDoing, "configs mismatch", newaddon)
-			return c.patchAddOnProgressingAndLastApplied(ctx, newaddon, oldaddon)
+			return patcher.PatchStatus(ctx, newaddon, newaddon.Status, oldaddon.Status)
 		}
 
 		// check if work is ready
 		if !workIsReady(work) {
 			setAddOnProgressingAndLastApplied(isUpgrade, ProgressingDoing, "work is not ready", newaddon)
-			return c.patchAddOnProgressingAndLastApplied(ctx, newaddon, oldaddon)
+			return patcher.PatchStatus(ctx, newaddon, newaddon.Status, oldaddon.Status)
 		}
 	}
 
 	// set lastAppliedConfig when all the work matches addon and are ready.
 	setAddOnProgressingAndLastApplied(isUpgrade, ProgressingSucceed, "", newaddon)
-	return c.patchAddOnProgressingAndLastApplied(ctx, newaddon, oldaddon)
-}
-
-func (c *addonProgressingController) patchAddOnProgressingAndLastApplied(ctx context.Context, new, old *addonapiv1alpha1.ManagedClusterAddOn) error {
-	if equality.Semantic.DeepEqual(new.Status, old.Status) {
-		return nil
-	}
-
-	oldData, err := json.Marshal(&addonapiv1alpha1.ManagedClusterAddOn{
-		Status: addonapiv1alpha1.ManagedClusterAddOnStatus{
-			ConfigReferences: old.Status.ConfigReferences,
-			Conditions:       old.Status.Conditions,
-		},
-	})
-	if err != nil {
-		return err
-	}
-
-	newData, err := json.Marshal(&addonapiv1alpha1.ManagedClusterAddOn{
-		ObjectMeta: metav1.ObjectMeta{
-			UID:             new.UID,
-			ResourceVersion: new.ResourceVersion,
-		},
-		Status: addonapiv1alpha1.ManagedClusterAddOnStatus{
-			ConfigReferences: new.Status.ConfigReferences,
-			Conditions:       new.Status.Conditions,
-		},
-	})
-	if err != nil {
-		return err
-	}
-
-	patchBytes, err := jsonpatch.CreateMergePatch(oldData, newData)
-	if err != nil {
-		return fmt.Errorf("failed to create patch for addon %s: %w", new.Name, err)
-	}
-
-	klog.V(2).Infof("Patching addon %s/%s condition and last applied config with %s", new.Namespace, new.Name, string(patchBytes))
-	addon, err := c.addonClient.AddonV1alpha1().ManagedClusterAddOns(new.Namespace).Patch(
-		ctx, new.Name, types.MergePatchType, patchBytes, metav1.PatchOptions{}, "status")
-	fmt.Printf("%v", addon)
-	return err
+	return patcher.PatchStatus(ctx, newaddon, newaddon.Status, oldaddon.Status)
 }
 
 func isConfigurationSupported(addon *addonapiv1alpha1.ManagedClusterAddOn) (bool, addonapiv1alpha1.ConfigGroupResource) {

pkg/addon/controllers/managementaddoninstallprogression/controller.go

@@ -2,30 +2,27 @@ package managementaddoninstallprogression
 
 import (
 	"context"
-	"encoding/json"
-	"fmt"
 
-	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/openshift/library-go/pkg/controller/factory"
 	"github.com/openshift/library-go/pkg/operator/events"
-	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/klog/v2"
 
 	addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
 	addonv1alpha1client "open-cluster-management.io/api/client/addon/clientset/versioned"
 	addoninformerv1alpha1 "open-cluster-management.io/api/client/addon/informers/externalversions/addon/v1alpha1"
 	addonlisterv1alpha1 "open-cluster-management.io/api/client/addon/listers/addon/v1alpha1"
+
+	"open-cluster-management.io/ocm/pkg/common/patcher"
 )
 
 // managementAddonInstallProgressionController reconciles instances of clustermanagementaddon the hub
 // based to update related object and status condition.
 type managementAddonInstallProgressionController struct {
-	addonClient                  addonv1alpha1client.Interface
+	patcher patcher.Patcher[
+		*addonv1alpha1.ClusterManagementAddOn, addonv1alpha1.ClusterManagementAddOnSpec, addonv1alpha1.ClusterManagementAddOnStatus]
 	managedClusterAddonLister    addonlisterv1alpha1.ManagedClusterAddOnLister
 	clusterManagementAddonLister addonlisterv1alpha1.ClusterManagementAddOnLister
 	addonFilterFunc              factory.EventFilterFunc
@@ -39,7 +36,9 @@ func NewManagementAddonInstallProgressionController(
 	recorder events.Recorder,
 ) factory.Controller {
 	c := &managementAddonInstallProgressionController{
-		addonClient:                  addonClient,
+		patcher: patcher.NewPatcher[
+			*addonv1alpha1.ClusterManagementAddOn, addonv1alpha1.ClusterManagementAddOnSpec, addonv1alpha1.ClusterManagementAddOnStatus](
+			addonClient.AddonV1alpha1().ClusterManagementAddOns()),
 		managedClusterAddonLister:    addonInformers.Lister(),
 		clusterManagementAddonLister: clusterManagementAddonInformers.Lister(),
 		addonFilterFunc:              addonFilterFunc,
@@ -84,12 +83,14 @@ func (c *managementAddonInstallProgressionController) sync(ctx context.Context,
 	// update default config reference when type is manual
 	if mgmtAddonCopy.Spec.InstallStrategy.Type == "" || mgmtAddonCopy.Spec.InstallStrategy.Type == addonv1alpha1.AddonInstallStrategyManual {
 		mgmtAddonCopy.Status.InstallProgressions = []addonv1alpha1.InstallProgression{}
-		return c.patchMgmtAddonStatus(ctx, mgmtAddonCopy, mgmtAddon)
+		_, err = c.patcher.PatchStatus(ctx, mgmtAddonCopy, mgmtAddonCopy.Status, mgmtAddon.Status)
+		return err
 	}
 
 	// only update default config references and skip updating install progression for self-managed addon
 	if !c.addonFilterFunc(clusterManagementAddon) {
-		return c.patchMgmtAddonStatus(ctx, mgmtAddonCopy, mgmtAddon)
+		_, err = c.patcher.PatchStatus(ctx, mgmtAddonCopy, mgmtAddonCopy.Status, mgmtAddon.Status)
+		return err
 	}
 
 	// set install progression
@@ -97,46 +98,7 @@ func (c *managementAddonInstallProgressionController) sync(ctx context.Context,
 		mgmtAddonCopy.Spec.InstallStrategy.Placements, mgmtAddonCopy.Status.InstallProgressions)
 
 	// update cma status
-	return c.patchMgmtAddonStatus(ctx, mgmtAddonCopy, mgmtAddon)
-}
-
-func (c *managementAddonInstallProgressionController) patchMgmtAddonStatus(ctx context.Context, new, old *addonv1alpha1.ClusterManagementAddOn) error {
-	if equality.Semantic.DeepEqual(new.Status, old.Status) {
-		return nil
-	}
-
-	oldData, err := json.Marshal(&addonv1alpha1.ClusterManagementAddOn{
-		Status: addonv1alpha1.ClusterManagementAddOnStatus{
-			DefaultConfigReferences: old.Status.DefaultConfigReferences,
-			InstallProgressions:     old.Status.InstallProgressions,
-		},
-	})
-	if err != nil {
-		return err
-	}
-
-	newData, err := json.Marshal(&addonv1alpha1.ClusterManagementAddOn{
-		ObjectMeta: metav1.ObjectMeta{
-			UID:             new.UID,
-			ResourceVersion: new.ResourceVersion,
-		},
-		Status: addonv1alpha1.ClusterManagementAddOnStatus{
-			DefaultConfigReferences: new.Status.DefaultConfigReferences,
-			InstallProgressions:     new.Status.InstallProgressions,
-		},
-	})
-	if err != nil {
-		return err
-	}
-
-	patchBytes, err := jsonpatch.CreateMergePatch(oldData, newData)
-	if err != nil {
-		return fmt.Errorf("failed to create patch for addon %s: %w", new.Name, err)
-	}
-
-	klog.V(2).Infof("Patching clustermanagementaddon %s status with %s", new.Name, string(patchBytes))
-	_, err = c.addonClient.AddonV1alpha1().ClusterManagementAddOns().Patch(
-		ctx, new.Name, types.MergePatchType, patchBytes, metav1.PatchOptions{}, "status")
+	_, err = c.patcher.PatchStatus(ctx, mgmtAddonCopy, mgmtAddonCopy.Status, mgmtAddon.Status)
 	return err
 }