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) {