Performance enhacments (#87)

Signed-off-by: Xiangjing Li <[email protected]>
(cherry picked from commit 1b8ace6ccad67d3a52985d4aca94e6a780ade04b)

Makefile

@@ -34,7 +34,7 @@ export GOPACKAGES   = $(shell go list ./... | grep -v /manager | grep -v /bindat
 
 TEST_TMP :=/tmp
 export KUBEBUILDER_ASSETS ?=$(TEST_TMP)/kubebuilder/bin
-K8S_VERSION ?=1.19.2
+K8S_VERSION ?=1.28.3
 GOHOSTOS ?=$(shell go env GOHOSTOS)
 GOHOSTARCH ?= $(shell go env GOHOSTARCH)
 KB_TOOLS_ARCHIVE_NAME :=kubebuilder-tools-$(K8S_VERSION)-$(GOHOSTOS)-$(GOHOSTARCH).tar.gz
@@ -55,6 +55,13 @@ build:
 build-images:
 	@docker build -t ${IMAGE_NAME_AND_VERSION} -f build/Dockerfile .
 
+# build local linux/amd64 images on non-amd64 hosts such as Apple M3
+build-images-non-amd64:
+	docker buildx create --use
+	docker buildx inspect --bootstrap
+	@docker buildx build --platform linux/amd64 -t ${IMAGE_NAME_AND_VERSION} -f build/Dockerfile --load .
+
+
 .PHONY: lint
 
 lint: lint-all
@@ -72,14 +79,12 @@ lint-go:
 
 # download the kubebuilder-tools to get kube-apiserver binaries from it
 ensure-kubebuilder-tools:
-ifeq "" "$(wildcard $(KUBEBUILDER_ASSETS))"
 	$(info Downloading kube-apiserver into '$(KUBEBUILDER_ASSETS)')
+	rm -fr '$(KUBEBUILDER_ASSETS)'
 	mkdir -p '$(KUBEBUILDER_ASSETS)'
 	curl -s -f -L https://storage.googleapis.com/kubebuilder-tools/$(KB_TOOLS_ARCHIVE_NAME) -o '$(KB_TOOLS_ARCHIVE_PATH)'
 	tar -C '$(KUBEBUILDER_ASSETS)' --strip-components=2 -zvxf '$(KB_TOOLS_ARCHIVE_PATH)'
-else
-	$(info Using existing kube-apiserver from "$(KUBEBUILDER_ASSETS)")
-endif
+
 .PHONY: ensure-kubebuilder-tools
 
 test: ensure-kubebuilder-tools

build/Dockerfile

@@ -1,6 +1,6 @@
 FROM registry.ci.openshift.org/stolostron/builder:go1.21-linux AS builder
 
-WORKDIR go/src/open-cluster-management.io/multicloud-operators-channel
+WORKDIR /go/src/open-cluster-management.io/multicloud-operators-channel
 COPY . .
 RUN make -f Makefile build
 

cmd/manager/exec/manager.go

@@ -155,6 +155,10 @@ func RunManager() {
 		RenewDeadline:           &options.LeaderElectionRenewDeadline,
 		RetryPeriod:             &options.LeaderElectionRetryPeriod,
 		WebhookServer:           webhookServer,
+		// create non cache client
+		NewClient: func(config *rest.Config, options client.Options) (client.Client, error) {
+			return client.New(config, client.Options{})
+		},
 	})
 
 	if err != nil {

pkg/apis/apis.go

@@ -20,6 +20,7 @@ import (
 
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
+	admissionv1 "k8s.io/api/admissionregistration/v1"
 	spokeClusterV1 "open-cluster-management.io/api/cluster/v1"
 )
 
@@ -35,5 +36,10 @@ func AddToScheme(s *runtime.Scheme) error {
 		return err
 	}
 
+	if err = admissionv1.AddToScheme(s); err != nil {
+		logf.Log.Error(err, "unable add k8s admission registration APIs to scheme")
+		return err
+	}
+
 	return AddToSchemes.AddToScheme(s)
 }

pkg/controller/channel/channel_controller.go

@@ -115,52 +115,14 @@ func add(mgr manager.Manager, r reconcile.Reconciler, logger logr.Logger) error
 		return err
 	}
 
-	if utils.IsReadyClusterRegistry(mgr.GetAPIReader()) {
-		cmapper := &clusterMapper{Client: mgr.GetClient(), logger: logger}
-		err = c.Watch(
-			source.Kind(mgr.GetCache(), &spokeClusterV1.ManagedCluster{}),
-			handler.EnqueueRequestsFromMapFunc(cmapper.Map),
-			utils.ClusterPredicateFunc,
-		)
-	}
+	// Watching managed cluster changes has been removed.
+	// For any managed cluster reconcile events(create, update, delete), don't trigger the automatic role/rolebing update for all channels.
+	// It turns out an expensive action if there are > 1000 managed clusters. The OOMKilled issue would hit easiy due to tons of duplicate reconciliations
+	// In this case, users should manually update channel cr to trigger the role/rolebinding update.
 
 	return err
 }
 
-type clusterMapper struct {
-	client.Client
-	logger logr.Logger
-}
-
-// Map triggers all placements
-func (mapper *clusterMapper) Map(ctx context.Context, obj client.Object) []reconcile.Request {
-	cname := obj.GetName()
-
-	mapper.logger.Info(fmt.Sprintf("In cluster Mapper for %v", cname))
-
-	plList := &chv1.ChannelList{}
-
-	listopts := &client.ListOptions{}
-	err := mapper.List(context.TODO(), plList, listopts)
-
-	if err != nil {
-		mapper.logger.Error(err, "failed to list channels")
-	}
-
-	var requests []reconcile.Request
-
-	for _, pl := range plList.Items {
-		objkey := types.NamespacedName{
-			Name:      pl.GetName(),
-			Namespace: pl.GetNamespace(),
-		}
-
-		requests = append(requests, reconcile.Request{NamespacedName: objkey})
-	}
-
-	return requests
-}
-
 var _ reconcile.Reconciler = &ReconcileChannel{}
 
 // ReconcileChannel reconciles a Channel object

pkg/utils/placement.go

@@ -17,53 +17,14 @@ package utils
 import (
 	"context"
 	"os"
-	"reflect"
 	"time"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/klog"
 	spokeClusterV1 "open-cluster-management.io/api/cluster/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/event"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
 )
 
-var ClusterPredicateFunc = predicate.Funcs{
-	UpdateFunc: func(e event.UpdateEvent) bool {
-		oldcl := e.ObjectOld.(*spokeClusterV1.ManagedCluster)
-		newcl := e.ObjectNew.(*spokeClusterV1.ManagedCluster)
-
-		//if managed cluster is being deleted
-		if !reflect.DeepEqual(oldcl.DeletionTimestamp, newcl.DeletionTimestamp) {
-			return true
-		}
-
-		if !reflect.DeepEqual(oldcl.Labels, newcl.Labels) {
-			return true
-		}
-
-		oldcondMap := make(map[string]metav1.ConditionStatus)
-		for _, cond := range oldcl.Status.Conditions {
-			oldcondMap[cond.Type] = cond.Status
-		}
-		for _, cond := range newcl.Status.Conditions {
-			oldcondst, ok := oldcondMap[cond.Type]
-			if !ok || oldcondst != cond.Status {
-				return true
-			}
-			delete(oldcondMap, cond.Type)
-		}
-
-		if len(oldcondMap) > 0 {
-			return true
-		}
-
-		klog.V(1).Info("Out Cluster Predicate Func ", oldcl.Name, " with false possitive")
-		return false
-	},
-}
-
 // IsReadyClusterRegistry check if Cluster API service is ready or not.
 func IsReadyClusterRegistry(clReader client.Reader) bool {
 	cllist := &spokeClusterV1.ManagedClusterList{}

pkg/utils/placement_test.go

@@ -20,149 +20,10 @@ import (
 	"time"
 
 	"github.com/onsi/gomega"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	spokeClusterV1 "open-cluster-management.io/api/cluster/v1"
-	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 )
 
-var (
-	oldClusterCond1 = metav1.Condition{
-		Type:   "ManagedClusterConditionAvailable",
-		Status: "true",
-	}
-
-	oldClusterCond2 = metav1.Condition{
-		Type:   "HubAcceptedManagedCluster",
-		Status: "true",
-	}
-
-	oldCluster = &spokeClusterV1.ManagedCluster{
-		TypeMeta: metav1.TypeMeta{
-			Kind:       "ManagedCluster",
-			APIVersion: "cluster.open-cluster-management.io/v1",
-		},
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "cluster1",
-			Labels: map[string]string{
-				"name": "cluster1",
-				"key1": "c1v1",
-				"key2": "c1v2",
-			},
-		},
-		Status: spokeClusterV1.ManagedClusterStatus{
-			Conditions: []metav1.Condition{oldClusterCond1, oldClusterCond2},
-		},
-	}
-
-	newClusterCond = metav1.Condition{
-		Type:   "ManagedClusterConditionAvailable",
-		Status: "true",
-	}
-
-	newCluster = &spokeClusterV1.ManagedCluster{
-		TypeMeta: metav1.TypeMeta{
-			Kind:       "ManagedCluster",
-			APIVersion: "cluster.open-cluster-management.io/v1",
-		},
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "cluster1",
-			Labels: map[string]string{
-				"name": "cluster1",
-				"key1": "c1v1",
-				"key2": "c1v2",
-			},
-		},
-		Status: spokeClusterV1.ManagedClusterStatus{
-			Conditions: []metav1.Condition{newClusterCond},
-		},
-	}
-
-	// Different labels
-	oldCluster2 = &spokeClusterV1.ManagedCluster{
-		TypeMeta: metav1.TypeMeta{
-			Kind:       "ManagedCluster",
-			APIVersion: "cluster.open-cluster-management.io/v1",
-		},
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "cluster1",
-			Labels: map[string]string{
-				"name": "cluster1",
-				"key1": "c1v1",
-				"key2": "c1v2",
-			},
-		},
-	}
-
-	newCluster2 = &spokeClusterV1.ManagedCluster{
-		TypeMeta: metav1.TypeMeta{
-			Kind:       "ManagedCluster",
-			APIVersion: "cluster.open-cluster-management.io/v1",
-		},
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "cluster1",
-			Labels: map[string]string{
-				"name": "cluster1",
-				"key1": "diff",
-				"key2": "diff",
-			},
-		},
-	}
-
-	// Different deletiontimestamps
-	oldCluster3 = &spokeClusterV1.ManagedCluster{
-		TypeMeta: metav1.TypeMeta{
-			Kind:       "ManagedCluster",
-			APIVersion: "cluster.open-cluster-management.io/v1",
-		},
-		ObjectMeta: metav1.ObjectMeta{
-			Name:              "cluster1",
-			DeletionTimestamp: &metav1.Time{},
-		},
-	}
-
-	newCluster3 = &spokeClusterV1.ManagedCluster{
-		TypeMeta: metav1.TypeMeta{
-			Kind:       "ManagedCluster",
-			APIVersion: "cluster.open-cluster-management.io/v1",
-		},
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "cluster1",
-		},
-	}
-)
-
-func TestPredicate(t *testing.T) {
-	g := gomega.NewGomegaWithT(t)
-
-	// Test ClusterPredicateFunc
-	instance := ClusterPredicateFunc
-
-	updateEvt := event.UpdateEvent{
-		ObjectOld: oldCluster,
-		ObjectNew: newCluster,
-	}
-	ret := instance.Update(updateEvt)
-	g.Expect(ret).To(gomega.Equal(true))
-
-	// Different deletiontimestamp
-	updateEvt = event.UpdateEvent{
-		ObjectOld: oldCluster3,
-		ObjectNew: newCluster3,
-	}
-	ret = instance.Update(updateEvt)
-	g.Expect(ret).To(gomega.Equal(true))
-
-	// Different labels
-	updateEvt = event.UpdateEvent{
-		ObjectOld: oldCluster2,
-		ObjectNew: newCluster2,
-	}
-	ret = instance.Update(updateEvt)
-	g.Expect(ret).To(gomega.Equal(true))
-}
-
 func TestIsReadyClusterRegistry(t *testing.T) {
 	g := gomega.NewGomegaWithT(t)
 

pkg/webhook/wireupwebhook.go

@@ -363,6 +363,11 @@ func setWebhookOwnerReferences(c client.Client, logger logr.Logger, obj metav1.O
 		return
 	}
 
+	// The TypeMeta fields (APIVersion and Kind) are not automatically populated
+	// when you fetch a Kubernetes resource using the client-go or controller-runtime client
+	owner.APIVersion = "apiextensions.k8s.io/v1"
+	owner.Kind = "CustomResourceDefinition"
+
 	obj.SetOwnerReferences([]metav1.OwnerReference{
 		{
 			APIVersion: owner.APIVersion,