[Multicluster] Fix cache options for controller Manager

A few issues were introduced by antrea-io#5843 because of changes in the sigs.k8s.io/controller-runtime interface. The biggest issue was that the call to ctrl.NewManager was not using the Options object populated earlier in the setupManagerAndCertController function. Instead it was creating and using a new, incomplete Options object. Fixes antrea-io#6149 Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
antoninbas · Mar 26, 2024 · f7e8260 · f7e8260
1 parent 91f374b
commit f7e8260
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 34 deletions.
diff --git a/multicluster/cmd/multicluster-controller/controller.go b/multicluster/cmd/multicluster-controller/controller.go
@@ -25,7 +25,6 @@ import (
 	apiextensionclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/fields"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	genericoptions "k8s.io/apiserver/pkg/server/options"
 	clientset "k8s.io/client-go/kubernetes"
@@ -139,6 +138,15 @@ func getWebhookLabel(isLeader bool, controllerNs string) *metav1.LabelSelector {
 func setupManagerAndCertController(isLeader bool, o *Options) (manager.Manager, error) {
 	ctrl.SetLogger(klog.NewKlogr())
 
+	podNamespace := env.GetPodNamespace()
+
+	var caConfig *certificate.CAConfig
+	if isLeader {
+		caConfig = getCaConfig(isLeader, podNamespace)
+	} else {
+		caConfig = getCaConfig(isLeader, "")
+	}
+
 	// build up cert controller to manage certificate for MC Controller
 	k8sConfig := ctrl.GetConfigOrDie()
 	k8sConfig.QPS = common.ResourceExchangeQPS
@@ -149,40 +157,48 @@ func setupManagerAndCertController(isLeader bool, o *Options) (manager.Manager,
 	}
 
 	secureServing := genericoptions.NewSecureServingOptions().WithLoopback()
-	caCertController, err := certificate.ApplyServerCert(o.SelfSignedCert, client, aggregatorClient, apiExtensionClient,
-		secureServing, getCaConfig(isLeader, o.Namespace))
+	caCertController, err := certificate.ApplyServerCert(o.SelfSignedCert, client, aggregatorClient, apiExtensionClient, secureServing, caConfig)
 	if err != nil {
 		return nil, fmt.Errorf("error applying server cert: %v", err)
 	}
 	if err := caCertController.RunOnce(context.TODO()); err != nil {
 		return nil, err
 	}
 
+	options := o.options
 	if o.SelfSignedCert {
-		o.options.Metrics.CertDir = selfSignedCertDir
+		options.Metrics.CertDir = selfSignedCertDir
 		o.WebhookConfig.CertDir = selfSignedCertDir
 	} else {
-		o.options.Metrics.CertDir = certDir
+		options.Metrics.CertDir = certDir
 		o.WebhookConfig.CertDir = certDir
 	}
-	o.options.WebhookServer = webhook.NewServer(webhook.Options{
+	options.WebhookServer = webhook.NewServer(webhook.Options{
 		Port:    *o.WebhookConfig.Port,
 		Host:    o.WebhookConfig.Host,
 		CertDir: o.WebhookConfig.CertDir,
 	})
 
-	namespaceFieldSelector := fields.SelectorFromSet(fields.Set{"metadata.namespace": env.GetPodNamespace()})
-	o.options.Cache.DefaultFieldSelector = namespaceFieldSelector
-	o.options.Cache.ByObject = map[controllerruntimeclient.Object]cache.ByObject{
-		&mcv1alpha1.Gateway{}: {
-			Field: namespaceFieldSelector,
-		},
-		&mcv1alpha2.ClusterSet{}: {
-			Field: namespaceFieldSelector,
-		},
-		&mcv1alpha1.MemberClusterAnnounce{}: {
-			Field: namespaceFieldSelector,
-		},
+	cacheOptions := &options.Cache
+	if isLeader {
+		// For the leader, restrict the cache to the controller's Namespace.
+		cacheOptions.DefaultNamespaces = map[string]cache.Config{
+			podNamespace: {},
+		}
+	} else {
+		// For a member, restict the cache to the controller's Namespace for the following objects.
+		cacheOptions.ByObject = map[controllerruntimeclient.Object]cache.ByObject{
+			&mcv1alpha1.Gateway{}: {
+				Namespaces: map[string]cache.Config{
+					podNamespace: {},
+				},
+			},
+			&mcv1alpha2.ClusterSet{}: {
+				Namespaces: map[string]cache.Config{
+					podNamespace: {},
+				},
+			},
+		}
 	}
 
 	// EndpointSlice is enabled in AntreaProxy by default since v1.11, so Antrea MC
@@ -206,11 +222,7 @@ func setupManagerAndCertController(isLeader bool, o *Options) (manager.Manager,
 	}
 	o.ClusterCalimCRDAvailable = clusterClaimCRDAvailable
 
-	mgr, err := ctrl.NewManager(k8sConfig, manager.Options{
-		Scheme:                 o.options.Scheme,
-		Metrics:                o.options.Metrics,
-		HealthProbeBindAddress: o.options.HealthProbeBindAddress,
-	})
+	mgr, err := ctrl.NewManager(k8sConfig, options)
 	if err != nil {
 		return nil, fmt.Errorf("error creating manager: %v", err)
 	}

diff --git a/multicluster/cmd/multicluster-controller/leader.go b/multicluster/cmd/multicluster-controller/leader.go
@@ -50,9 +50,7 @@ func newLeaderCommand() *cobra.Command {
 }
 
 func runLeader(o *Options) error {
-	// on the leader we want the reconciler to run for a given Namespace instead of cluster scope
 	podNamespace := env.GetPodNamespace()
-	o.Namespace = podNamespace
 	stopCh := signals.RegisterSignalHandlers()
 
 	mgr, err := setupManagerAndCertControllerFunc(true, o)

diff --git a/multicluster/cmd/multicluster-controller/options.go b/multicluster/cmd/multicluster-controller/options.go
@@ -35,8 +35,8 @@ type Options struct {
 	// The path of configuration file.
 	configFile     string
 	SelfSignedCert bool
-	options        ctrl.Options
-	Namespace      string
+	// options store some base controller Manager options (initialized from the provided config).
+	options ctrl.Options
 	// The Service ClusterIP range used in the member cluster.
 	ServiceCIDR string
 	// PodCIDRs is the Pod IP address CIDRs of the member cluster.
@@ -68,14 +68,12 @@ func newOptions() *Options {
 func (o *Options) complete(args []string) error {
 	var err error
 	o.setDefaults()
-	options := ctrl.Options{Scheme: scheme}
 	ctrlConfig := &mcsv1alpha1.MultiClusterConfig{}
 	if len(o.configFile) > 0 {
 		klog.InfoS("Loading config", "file", o.configFile)
 		if err = o.loadConfigFromFile(ctrlConfig); err != nil {
 			return err
 		}
-		o.options = options
 		if ctrlConfig.ServiceCIDR != "" {
 			if _, _, err := net.ParseCIDR(ctrlConfig.ServiceCIDR); err != nil {
 				return fmt.Errorf("failed to parse serviceCIDR, invalid CIDR string %s", ctrlConfig.ServiceCIDR)

diff --git a/multicluster/cmd/multicluster-controller/options_test.go b/multicluster/cmd/multicluster-controller/options_test.go
@@ -19,7 +19,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	ctrl "sigs.k8s.io/controller-runtime"
 )
 
 func TestComplete(t *testing.T) {
@@ -34,7 +33,6 @@ func TestComplete(t *testing.T) {
 			o: Options{
 				configFile:          "./testdata/antrea-mc-config-with-valid-podcidrs.yml",
 				SelfSignedCert:      false,
-				options:             ctrl.Options{},
 				ServiceCIDR:         "",
 				PodCIDRs:            nil,
 				GatewayIPPrecedence: "",
@@ -47,7 +45,6 @@ func TestComplete(t *testing.T) {
 			o: Options{
 				configFile:          "./testdata/antrea-mc-config-with-empty-podcidrs.yml",
 				SelfSignedCert:      false,
-				options:             ctrl.Options{},
 				ServiceCIDR:         "",
 				PodCIDRs:            nil,
 				GatewayIPPrecedence: "",
@@ -60,7 +57,6 @@ func TestComplete(t *testing.T) {
 			o: Options{
 				configFile:          "./testdata/antrea-mc-config-with-invalid-podcidrs.yml",
 				SelfSignedCert:      false,
-				options:             ctrl.Options{},
 				ServiceCIDR:         "10.100.0.0/16",
 				PodCIDRs:            nil,
 				GatewayIPPrecedence: "",
@@ -73,7 +69,6 @@ func TestComplete(t *testing.T) {
 			o: Options{
 				configFile:          "./testdata/antrea-mc-config-with-invalid-endpointiptype.yml",
 				SelfSignedCert:      false,
-				options:             ctrl.Options{},
 				ServiceCIDR:         "10.100.0.0/16",
 				PodCIDRs:            nil,
 				GatewayIPPrecedence: "",