From f7e82602a6d7c9588134d9555adb46d54be69122 Mon Sep 17 00:00:00 2001 From: Antonin Bas Date: Tue, 26 Mar 2024 14:27:23 -0700 Subject: [PATCH] [Multicluster] Fix cache options for controller Manager A few issues were introduced by #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 #6149 Signed-off-by: Antonin Bas --- .../cmd/multicluster-controller/controller.go | 58 +++++++++++-------- .../cmd/multicluster-controller/leader.go | 2 - .../cmd/multicluster-controller/options.go | 6 +- .../multicluster-controller/options_test.go | 5 -- 4 files changed, 37 insertions(+), 34 deletions(-) diff --git a/multicluster/cmd/multicluster-controller/controller.go b/multicluster/cmd/multicluster-controller/controller.go index 00c39a4145b..cb586b3e742 100644 --- 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,8 +157,7 @@ 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) } @@ -158,31 +165,40 @@ func setupManagerAndCertController(isLeader bool, o *Options) (manager.Manager, 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 index a9ad3a01cf4..42aa2467204 100644 --- 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 index f28e6439e52..ec1c782b148 100644 --- 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 index 716fba64916..d053446330e 100644 --- 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: "",