fix: cns to use controller runtime (cached) clients in reconcilers

cns/kubecontroller/clustersubnetstate/reconciler.go

@@ -3,6 +3,7 @@ package clustersubnetstate
 import (
 	"context"
 
+	"github.com/Azure/azure-container-networking/crd/clustersubnetstate"
 	"github.com/Azure/azure-container-networking/crd/clustersubnetstate/api/v1alpha1"
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
@@ -16,22 +17,29 @@ type cssClient interface {
 }
 
 type Reconciler struct {
-	Cli  cssClient
-	Sink chan<- v1alpha1.ClusterSubnetState
+	cli  cssClient
+	sink chan<- v1alpha1.ClusterSubnetState
+}
+
+func New(sink chan<- v1alpha1.ClusterSubnetState) *Reconciler {
+	return &Reconciler{
+		sink: sink,
+	}
 }
 
 func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
-	css, err := r.Cli.Get(ctx, req.NamespacedName)
+	css, err := r.cli.Get(ctx, req.NamespacedName)
 	if err != nil {
 		cssReconcilerErrorCount.With(prometheus.Labels{cssReconcilerCRDWatcherStateLabel: "failed"}).Inc()
 		return reconcile.Result{}, errors.Wrapf(err, "failed to get css %s", req.String())
 	}
 	cssReconcilerErrorCount.With(prometheus.Labels{cssReconcilerCRDWatcherStateLabel: "succeeded"}).Inc()
-	r.Sink <- *css
+	r.sink <- *css
 	return reconcile.Result{}, nil
 }
 
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
+	r.cli = clustersubnetstate.NewClient(mgr.GetClient())
 	err := ctrl.NewControllerManagedBy(mgr).
 		For(&v1alpha1.ClusterSubnetState{}).
 		Complete(r)

cns/kubecontroller/nodenetworkconfig/reconciler.go

@@ -8,6 +8,7 @@ import (
 	"github.com/Azure/azure-container-networking/cns/logger"
 	"github.com/Azure/azure-container-networking/cns/restserver"
 	cnstypes "github.com/Azure/azure-container-networking/cns/types"
+	"github.com/Azure/azure-container-networking/crd/nodenetworkconfig"
 	"github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha"
 	"github.com/pkg/errors"
 	v1 "k8s.io/api/core/v1"
@@ -47,11 +48,10 @@ type Reconciler struct {
 // apiserver for NNC events.
 // Provided nncListeners are passed the NNC after the Reconcile preprocesses it. Note: order matters! The
 // passed Listeners are notified in the order provided.
-func NewReconciler(cnscli cnsClient, nnccli nncGetter, ipampoolmonitorcli nodeNetworkConfigListener, nodeIP string) *Reconciler {
+func NewReconciler(cnscli cnsClient, ipampoolmonitorcli nodeNetworkConfigListener, nodeIP string) *Reconciler {
 	return &Reconciler{
 		cnscli:             cnscli,
 		ipampoolmonitorcli: ipampoolmonitorcli,
-		nnccli:             nnccli,
 		started:            make(chan interface{}),
 		nodeIP:             nodeIP,
 	}
@@ -146,6 +146,7 @@ func (r *Reconciler) Started(ctx context.Context) bool {
 
 // SetupWithManager Sets up the reconciler with a new manager, filtering using NodeNetworkConfigFilter on nodeName.
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager, node *v1.Node) error {
+	r.nnccli = nodenetworkconfig.NewClient(mgr.GetClient())
 	err := ctrl.NewControllerManagedBy(mgr).
 		For(&v1alpha.NodeNetworkConfig{}).
 		WithEventFilter(predicate.Funcs{

cns/kubecontroller/nodenetworkconfig/reconciler_test.go

@@ -174,7 +174,8 @@ func TestReconcile(t *testing.T) {
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			r := NewReconciler(&tt.cnsClient, &tt.ncGetter, &tt.cnsClient, tt.nodeIP)
+			r := NewReconciler(&tt.cnsClient, &tt.cnsClient, tt.nodeIP)
+			r.nnccli = &tt.ncGetter
 			got, err := r.Reconcile(context.Background(), tt.in)
 			if tt.wantErr {
 				require.Error(t, err)

cns/service/main.go

@@ -40,7 +40,6 @@ import (
 	"github.com/Azure/azure-container-networking/cns/wireserver"
 	acn "github.com/Azure/azure-container-networking/common"
 	"github.com/Azure/azure-container-networking/crd"
-	"github.com/Azure/azure-container-networking/crd/clustersubnetstate"
 	"github.com/Azure/azure-container-networking/crd/clustersubnetstate/api/v1alpha1"
 	"github.com/Azure/azure-container-networking/crd/nodenetworkconfig"
 	"github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha"
@@ -1075,7 +1074,11 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn
 		})
 	}
 	// create scoped kube clients.
-	nnccli, err := nodenetworkconfig.NewClient(kubeConfig)
+	directcli, err := client.New(kubeConfig, client.Options{Scheme: nodenetworkconfig.Scheme})
+	if err != nil {
+		return errors.Wrap(err, "failed to create ctrl client")
+	}
+	nnccli := nodenetworkconfig.NewClient(directcli)
 	if err != nil {
 		return errors.Wrap(err, "failed to create NNC client")
 	}
@@ -1169,23 +1172,15 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn
 	nodeIP := configuration.NodeIP()
 
 	// NodeNetworkConfig reconciler
-	nncReconciler := nncctrl.NewReconciler(httpRestServiceImplementation, nnccli, poolMonitor, nodeIP)
+	nncReconciler := nncctrl.NewReconciler(httpRestServiceImplementation, poolMonitor, nodeIP)
 	// pass Node to the Reconciler for Controller xref
 	if err := nncReconciler.SetupWithManager(manager, node); err != nil { //nolint:govet // intentional shadow
 		return errors.Wrapf(err, "failed to setup nnc reconciler with manager")
 	}
 
 	if cnsconfig.EnableSubnetScarcity {
-		cssCli, err := clustersubnetstate.NewClient(kubeConfig)
-		if err != nil {
-			return errors.Wrapf(err, "failed to init css client")
-		}
-
 		// ClusterSubnetState reconciler
-		cssReconciler := cssctrl.Reconciler{
-			Cli:  cssCli,
-			Sink: clusterSubnetStateChan,
-		}
+		cssReconciler := cssctrl.New(clusterSubnetStateChan)
 		if err := cssReconciler.SetupWithManager(manager); err != nil {
 			return errors.Wrapf(err, "failed to setup css reconciler with manager")
 		}

crd/client.go

@@ -7,10 +7,16 @@ import (
 	"k8s.io/client-go/rest"
 )
 
-func NewCRDClient(config *rest.Config) (v1.CustomResourceDefinitionInterface, error) {
+// NewCRDCLientFromConfig creates a CRD-scoped client from the provided kubeconfig.
+func NewCRDClientFromConfig(config *rest.Config) (v1.CustomResourceDefinitionInterface, error) {
 	c, err := clientset.NewForConfig(config)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to init CRD client")
 	}
+	return NewCRDClientFromClientset(c)
+}
+
+// NewCRDCLientFromConfig creates a CRD-scoped client from the provided kube clientset.
+func NewCRDClientFromClientset(c *clientset.Clientset) (v1.CustomResourceDefinitionInterface, error) {
 	return c.ApiextensionsV1().CustomResourceDefinitions(), nil
 }

crd/clustersubnetstate/client.go

@@ -13,117 +13,93 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
-	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
-	ctrlcli "sigs.k8s.io/controller-runtime/pkg/client"
-	ctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 // Scheme is a runtime scheme containing the client-go scheme and the ClusterSubnetStatus scheme.
 var Scheme = runtime.NewScheme()
 
 func init() {
-	_ = clientgoscheme.AddToScheme(Scheme)
+	_ = scheme.AddToScheme(Scheme)
 	_ = v1alpha1.AddToScheme(Scheme)
 }
 
-// Client is provided to interface with the ClusterSubnetState CRDs.
-type Client struct {
-	csscli ctrlcli.Client
-	crdcli typedv1.CustomResourceDefinitionInterface
+// Installer provides methods to manage the lifecycle of the ClusterSubnetState resource definition.
+type Installer struct {
+	cli typedv1.CustomResourceDefinitionInterface
 }
 
-// NewClient creates a new ClusterSubnetState client from the passed k8s Config.
-func NewClient(c *rest.Config) (*Client, error) {
-	crdCli, err := crd.NewCRDClient(c)
+func NewInstaller(c *rest.Config) (*Installer, error) {
+	cli, err := crd.NewCRDClientFromConfig(c)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to init crd client")
 	}
-	opts := ctrlcli.Options{
-		Scheme: Scheme,
-	}
-	cssCli, err := ctrlcli.New(c, opts)
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to init css client")
-	}
-	return &Client{
-		crdcli: crdCli,
-		csscli: cssCli,
+	return &Installer{
+		cli: cli,
 	}, nil
 }
 
-func (c *Client) create(ctx context.Context, res *v1.CustomResourceDefinition) (*v1.CustomResourceDefinition, error) {
-	res, err := c.crdcli.Create(ctx, res, metav1.CreateOptions{})
+func (i *Installer) create(ctx context.Context, res *v1.CustomResourceDefinition) (*v1.CustomResourceDefinition, error) {
+	res, err := i.cli.Create(ctx, res, metav1.CreateOptions{})
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to create css crd")
 	}
 	return res, nil
 }
 
-// Get returns the ClusterSubnetState identified by the NamespacedName.
-func (c *Client) Get(ctx context.Context, key types.NamespacedName) (*v1alpha1.ClusterSubnetState, error) {
-	clusterSubnetState := &v1alpha1.ClusterSubnetState{}
-	err := c.csscli.Get(ctx, key, clusterSubnetState)
-	return clusterSubnetState, errors.Wrapf(err, "failed to get css %v", key)
-}
-
 // Install installs the embedded ClusterSubnetState CRD definition in the cluster.
-func (c *Client) Install(ctx context.Context) (*v1.CustomResourceDefinition, error) {
+func (i *Installer) Install(ctx context.Context) (*v1.CustomResourceDefinition, error) {
 	css, err := GetClusterSubnetStates()
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get embedded css crd")
 	}
-	return c.create(ctx, css)
+	return i.create(ctx, css)
 }
 
 // InstallOrUpdate installs the embedded ClusterSubnetState CRD definition in the cluster or updates it if present.
-func (c *Client) InstallOrUpdate(ctx context.Context) (*v1.CustomResourceDefinition, error) {
+func (i *Installer) InstallOrUpdate(ctx context.Context) (*v1.CustomResourceDefinition, error) {
 	css, err := GetClusterSubnetStates()
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get embedded css crd")
 	}
-	current, err := c.create(ctx, css)
+	current, err := i.create(ctx, css)
 	if !apierrors.IsAlreadyExists(err) {
 		return current, err
 	}
 	if current == nil {
-		current, err = c.crdcli.Get(ctx, css.Name, metav1.GetOptions{})
+		current, err = i.cli.Get(ctx, css.Name, metav1.GetOptions{})
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to get existing css crd")
 		}
 	}
 	if !reflect.DeepEqual(css.Spec.Versions, current.Spec.Versions) {
 		css.SetResourceVersion(current.GetResourceVersion())
 		previous := *current
-		current, err = c.crdcli.Update(ctx, css, metav1.UpdateOptions{})
+		current, err = i.cli.Update(ctx, css, metav1.UpdateOptions{})
 		if err != nil {
 			return &previous, errors.Wrap(err, "failed to update existing css crd")
 		}
 	}
 	return current, nil
 }
 
-// SetOwnerRef sets the owner of the ClusterSubnetStatus to the given object, using HTTP Patch
-func (c *Client) SetOwnerRef(ctx context.Context, key types.NamespacedName, owner metav1.Object, fieldManager string) (*v1alpha1.ClusterSubnetState, error) {
-	obj := genPatchSkel(key)
-	if err := ctrlutil.SetControllerReference(owner, obj, Scheme); err != nil {
-		return nil, errors.Wrapf(err, "failed to set controller reference for css")
-	}
-	if err := c.csscli.Patch(ctx, obj, ctrlcli.Apply, ctrlcli.ForceOwnership, ctrlcli.FieldOwner(fieldManager)); err != nil {
-		return nil, errors.Wrapf(err, "failed to patch css")
-	}
-	return obj, nil
+// Client provides methods to interact with instances of the ClusterSubnetState custom resource.
+type Client struct {
+	cli client.Client
 }
 
-func genPatchSkel(key types.NamespacedName) *v1alpha1.ClusterSubnetState {
-	return &v1alpha1.ClusterSubnetState{
-		TypeMeta: metav1.TypeMeta{
-			APIVersion: v1alpha1.GroupVersion.String(),
-			Kind:       "ClusterSubnetState",
-		},
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      key.Name,
-			Namespace: key.Namespace,
-		},
+// NewClient creates a new ClusterSubnetState client from the passed ctrlcli.Client.
+func NewClient(cli client.Client) *Client {
+	return &Client{
+		cli: cli,
 	}
 }
+
+// Get returns the ClusterSubnetState identified by the NamespacedName.
+func (c *Client) Get(ctx context.Context, key types.NamespacedName) (*v1alpha1.ClusterSubnetState, error) {
+	clusterSubnetState := &v1alpha1.ClusterSubnetState{}
+	err := c.cli.Get(ctx, key, clusterSubnetState)
+	return clusterSubnetState, errors.Wrapf(err, "failed to get css %v", key)
+}