Change remote.NewClusterClient to return a controller-runtime client.Client

Author: noamran

controllers/machine_controller.go

@@ -19,6 +19,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"k8s.io/apimachinery/pkg/runtime"
 	"sync"
 	"time"
 
@@ -31,6 +32,7 @@ import (
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/klog"
@@ -67,6 +69,7 @@ type MachineReconciler struct {
 	controller       controller.Controller
 	recorder         record.EventRecorder
 	externalWatchers sync.Map
+	scheme *runtime.Scheme
 }
 
 func (r *MachineReconciler) SetupWithManager(mgr ctrl.Manager, options controller.Options) error {
@@ -75,9 +78,16 @@ func (r *MachineReconciler) SetupWithManager(mgr ctrl.Manager, options controlle
 		WithOptions(options).
 		Build(r)
 
+	if err!=nil{
+		return err
+	}
+
 	r.controller = c
 	r.recorder = mgr.GetEventRecorderFor("machine-controller")
 	r.config = mgr.GetConfig()
+
+	r.scheme = runtime.NewScheme()
+	err = scheme.AddToScheme(r.scheme)
 	return err
 }
 
@@ -277,12 +287,8 @@ func (r *MachineReconciler) drainNode(cluster *clusterv1.Cluster, nodeName strin
 		}
 	} else {
 		// Otherwise, proceed to get the remote cluster client and get the Node.
-		remoteClient, err := remote.NewClusterClient(r.Client, cluster)
-		if err != nil {
-			logger.Error(err, "Error creating a remote client while deleting Machine, won't retry")
-			return nil
-		}
-		kubeClient, err = kubernetes.NewForConfig(remoteClient.RESTConfig())
+		restConfig , err := remote.RESTConfig(r.Client, cluster)
+		kubeClient, err = kubernetes.NewForConfig(restConfig)
 		if err != nil {
 			logger.Error(err, "Error creating a remote client while deleting Machine, won't retry")
 			return nil
@@ -339,32 +345,26 @@ func (r *MachineReconciler) drainNode(cluster *clusterv1.Cluster, nodeName strin
 
 func (r *MachineReconciler) deleteNode(ctx context.Context, cluster *clusterv1.Cluster, name string) error {
 	logger := r.Log.WithValues("machine", name)
-
+	var c client.Client
+	var err error
 	if cluster == nil {
 		// Try to retrieve the Node from the local cluster, if no Cluster reference is found.
-		var node corev1.Node
-		if err := r.Client.Get(ctx, client.ObjectKey{Name: name}, &node); err != nil {
-			return err
+		c = r.Client
+	}else{
+		logger = logger.WithValues("cluster", cluster.Name, "namespace", cluster.Namespace)
+		// Otherwise, proceed to get the remote cluster client to get the Node.
+		c, err = remote.NewClusterClient(r.Client, cluster, r.scheme)
+		if err != nil {
+			logger.Error(err, "Error creating a remote client for cluster while deleting Machine, won't retry")
+			return nil
 		}
-		return r.Client.Delete(ctx, &node)
-	}
-
-	logger = logger.WithValues("cluster", cluster.Name, "namespace", cluster.Namespace)
-
-	// Otherwise, proceed to get the remote cluster client and get the Node.
-	remoteClient, err := remote.NewClusterClient(r.Client, cluster)
-	if err != nil {
-		logger.Error(err, "Error creating a remote client for cluster while deleting Machine, won't retry")
-		return nil
 	}
 
-	corev1Remote, err := remoteClient.CoreV1()
-	if err != nil {
-		logger.Error(err, "Error creating a remote client for cluster while deleting Machine, won't retry")
-		return nil
+	var node corev1.Node
+	if err := c.Get(ctx, client.ObjectKey{Name: name}, &node); err != nil {
+		return err
 	}
-
-	return corev1Remote.Nodes().Delete(name, &metav1.DeleteOptions{})
+	return c.Delete(ctx, &node)
 }
 
 // reconcileDeleteExternal tries to delete external references, returning true if it cannot find any.

controllers/machine_controller_noderef.go

@@ -18,12 +18,12 @@ package controllers
 
 import (
 	"context"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"time"
 
 	"github.com/pkg/errors"
 	apicorev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
 	"sigs.k8s.io/cluster-api/controllers/noderefutil"
 	"sigs.k8s.io/cluster-api/controllers/remote"
@@ -65,18 +65,13 @@ func (r *MachineReconciler) reconcileNodeRef(_ context.Context, cluster *cluster
 		return err
 	}
 
-	clusterClient, err := remote.NewClusterClient(r.Client, cluster)
-	if err != nil {
-		return err
-	}
-
-	corev1Client, err := clusterClient.CoreV1()
+	clusterClient, err := remote.NewClusterClient(r.Client, cluster, r.scheme)
 	if err != nil {
 		return err
 	}
 
 	// Get the Node reference.
-	nodeRef, err := r.getNodeReference(corev1Client, providerID)
+	nodeRef, err := r.getNodeReference(clusterClient, providerID)
 	if err != nil {
 		if err == ErrNodeNotFound {
 			return errors.Wrapf(&capierrors.RequeueAfterError{RequeueAfter: 10 * time.Second},
@@ -94,13 +89,14 @@ func (r *MachineReconciler) reconcileNodeRef(_ context.Context, cluster *cluster
 	return nil
 }
 
-func (r *MachineReconciler) getNodeReference(client corev1.NodesGetter, providerID *noderefutil.ProviderID) (*apicorev1.ObjectReference, error) {
+func (r *MachineReconciler) getNodeReference(client client.Client, providerID *noderefutil.ProviderID) (*apicorev1.ObjectReference, error) {
 	logger := r.Log.WithValues("providerID", providerID)
 
 	listOpt := metav1.ListOptions{}
 
 	for {
-		nodeList, err := client.Nodes().List(listOpt)
+		nodeList := apicorev1.NodeList{}
+		err := client.List(context.Background(), &nodeList)
 		if err != nil {
 			return nil, err
 		}
@@ -129,4 +125,4 @@ func (r *MachineReconciler) getNodeReference(client corev1.NodesGetter, provider
 	}
 
 	return nil, ErrNodeNotFound
-}
+}

controllers/machineset_controller.go

@@ -19,6 +19,8 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/kubernetes/scheme"
 	"strings"
 	"sync"
 	"time"
@@ -65,6 +67,7 @@ type MachineSetReconciler struct {
 	Log    logr.Logger
 
 	recorder record.EventRecorder
+	scheme *runtime.Scheme
 }
 
 func (r *MachineSetReconciler) SetupWithManager(mgr ctrl.Manager, options controller.Options) error {
@@ -78,7 +81,14 @@ func (r *MachineSetReconciler) SetupWithManager(mgr ctrl.Manager, options contro
 		WithOptions(options).
 		Complete(r)
 
+	if err!=nil{
+		return err
+	}
+
 	r.recorder = mgr.GetEventRecorderFor("machineset-controller")
+
+	r.scheme = runtime.NewScheme()
+	err = scheme.AddToScheme(r.scheme)
 	return err
 }
 

controllers/machineset_status.go

@@ -138,23 +138,19 @@ func updateMachineSetStatus(c client.Client, ms *clusterv1.MachineSet, newStatus
 }
 
 func (r *MachineSetReconciler) getMachineNode(cluster *clusterv1.Cluster, machine *clusterv1.Machine) (*corev1.Node, error) {
+	var c client.Client
+	var err error
 	if cluster == nil {
 		// Try to retrieve the Node from the local cluster, if no Cluster reference is found.
-		node := &corev1.Node{}
-		err := r.Client.Get(context.Background(), client.ObjectKey{Name: machine.Status.NodeRef.Name}, node)
-		return node, err
-	}
-
-	// Otherwise, proceed to get the remote cluster client and get the Node.
-	remoteClient, err := remote.NewClusterClient(r.Client, cluster)
-	if err != nil {
-		return nil, err
-	}
-
-	corev1Remote, err := remoteClient.CoreV1()
-	if err != nil {
-		return nil, err
+		c = r.Client
+	} else {
+		// Otherwise, proceed to get the remote cluster client and get the Node.
+		c, err = remote.NewClusterClient(r.Client, cluster, r.scheme)
+		if err != nil {
+			return nil, err
+		}
 	}
-
-	return corev1Remote.Nodes().Get(machine.Status.NodeRef.Name, metav1.GetOptions{})
+	node := &corev1.Node{}
+	err = c.Get(context.Background(), client.ObjectKey{Name: machine.Status.NodeRef.Name}, node)
+	return node, err
 }

controllers/remote/cluster.go

@@ -18,53 +18,41 @@ package remote
 
 import (
 	"github.com/pkg/errors"
-	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
 	kcfg "sigs.k8s.io/cluster-api/util/kubeconfig"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-// ClusterClient is an interface encapsulating methods
-// to access a remote cluster.
-type ClusterClient interface {
-	RESTConfig() *restclient.Config
-	CoreV1() (corev1.CoreV1Interface, error)
-}
-
-// clusterClient is a helper struct to connect to remote workload clusters.
-type clusterClient struct {
-	restConfig *restclient.Config
-	cluster    *clusterv1.Cluster
-}
+// NewClusterClient returns a Client for interacting with a remote Cluster
+func NewClusterClient(c client.Client, cluster *clusterv1.Cluster, scheme *runtime.Scheme) (client.Client, error){
 
-// NewClusterClient creates a new ClusterClient.
-func NewClusterClient(c client.Client, cluster *clusterv1.Cluster) (ClusterClient, error) {
-	kubeconfig, err := kcfg.FromSecret(c, cluster)
+	restConfig, err := RESTConfig(c, cluster)
 	if err != nil {
-		return nil, errors.Wrapf(err, "failed to retrieve kubeconfig secret for Cluster %q in namespace %q",
-			cluster.Name, cluster.Namespace)
+		return nil, err
 	}
 
-	restConfig, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig)
+	ret, err := client.New(restConfig, client.Options{Scheme:scheme})
 	if err != nil {
-		return nil, errors.Wrapf(err, "failed to create client configuration for Cluster %q in namespace %q",
-			cluster.Name, cluster.Namespace)
+		return nil, errors.Wrapf(err, "failed to create client for Cluster %s/%s", cluster.Namespace, cluster.Name)
 	}
 
-	return &clusterClient{
-		restConfig: restConfig,
-		cluster:    cluster,
-	}, nil
+	return ret, nil
 }
 
 // RESTConfig returns a configuration instance to be used with a Kubernetes client.
-func (c *clusterClient) RESTConfig() *restclient.Config {
-	return c.restConfig
-}
+func  RESTConfig(c client.Client, cluster *clusterv1.Cluster) (*restclient.Config, error) {
+	kubeConfig, err := kcfg.FromSecret(c, cluster)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to retrieve kubeconfig secret for Cluster %s/%s", cluster.Namespace, cluster.Name)
+	}
+
+	restConfig, err := clientcmd.RESTConfigFromKubeConfig(kubeConfig)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to create REST configuration for Cluster %s/%s", cluster.Namespace, cluster.Name)
+	}
 
-// CoreV1 returns a new Kubernetes CoreV1 client.
-func (c *clusterClient) CoreV1() (corev1.CoreV1Interface, error) {
-	return corev1.NewForConfig(c.RESTConfig())
+	return restConfig, nil
 }

docs/book/src/providers/v1alpha2-to-v1alpha3.md

@@ -36,3 +36,15 @@
 ## Cluster and Machine controllers now set `cluster.x-k8s.io/cluster-name` to external objects.
 
 - In addition to the OwnerReference back to the Cluster, a label is now added as well to any external objects, for example objects such as KubeadmConfig (bootstrap provider), AWSCluster (infrastructure provider), AWSMachine (infrastructure provider), etc.
+
+## Changes to `sigs.k8s.io/cluster-api/controllers/remote`
+
+-  The `ClusterClient` interface has been removed.
+- `remote.NewClusterClient` now returns a `sigs.k8s.io/controller-runtime/pkg/client` Client. The signature changed from 
+
+    `func NewClusterClient(c client.Client, cluster *clusterv1.Cluster) (ClusterClient, error)`
+
+    to
+    
+    `func NewClusterClient(c client.Client, cluster *clusterv1.Cluster, scheme runtime.Scheme) (client.Client, error)`
+