Repository: submariner-io/lighthouse
Pull request #1027 (2023-01-03 16:42:53)

Remove unused kubeClientSet in agent Controller

submariner-bot: 🤖 Created branch: z_pr1027/tpantelis/rem_kubeClientSet 
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

Patch:
diff --git a/pkg/agent/controller/agent.go b/pkg/agent/controller/agent.go
index 66ac82a..1d08b7b 100644
--- a/pkg/agent/controller/agent.go
+++ b/pkg/agent/controller/agent.go
@@ -39,7 +39,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	validations "k8s.io/apimachinery/pkg/util/validation"
-	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/util/retry"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	mcsv1a1 "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1"
@@ -59,9 +58,7 @@ type AgentConfig struct {
 var logger = log.Logger{Logger: logf.Log.WithName("agent")}
 
 //nolint:gocritic // (hugeParam) This function modifies syncerConf so we don't want to pass by pointer.
-func New(spec *AgentSpecification, syncerConf broker.SyncerConfig, kubeClientSet kubernetes.Interface,
-	syncerMetricNames AgentConfig,
-) (*Controller, error) {
+func New(spec *AgentSpecification, syncerConf broker.SyncerConfig, syncerMetricNames AgentConfig) (*Controller, error) {
 	if errs := validations.IsDNS1123Label(spec.ClusterID); len(errs) > 0 {
 		return nil, errors.Errorf("%s is not a valid ClusterID %v", spec.ClusterID, errs)
 	}
@@ -70,7 +67,6 @@ func New(spec *AgentSpecification, syncerConf broker.SyncerConfig, kubeClientSet
 		clusterID:        spec.ClusterID,
 		namespace:        spec.Namespace,
 		globalnetEnabled: spec.GlobalnetEnabled,
-		kubeClientSet:    kubeClientSet,
 	}
 
 	_, gvr, err := util.ToUnstructuredResource(&mcsv1a1.ServiceExport{}, syncerConf.RestMapper)
diff --git a/pkg/agent/controller/types.go b/pkg/agent/controller/types.go
index 995d2d3..26511ab 100644
--- a/pkg/agent/controller/types.go
+++ b/pkg/agent/controller/types.go
@@ -28,14 +28,12 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/dynamic"
-	"k8s.io/client-go/kubernetes"
 )
 
 type Controller struct {
 	clusterID               string
 	globalnetEnabled        bool
 	namespace               string
-	kubeClientSet           kubernetes.Interface
 	serviceExportClient     dynamic.NamespaceableResourceInterface
 	serviceExportSyncer     syncer.Interface
 	serviceImportSyncer     *broker.Syncer
diff --git a/pkg/agent/main.go b/pkg/agent/main.go
index 011ff26..6e860a9 100644
--- a/pkg/agent/main.go
+++ b/pkg/agent/main.go
@@ -35,7 +35,6 @@ import (
 	"github.com/submariner-io/admiral/pkg/util"
 	"github.com/submariner-io/lighthouse/pkg/agent/controller"
 	"k8s.io/client-go/dynamic"
-	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/klog/v2"
@@ -106,9 +105,6 @@ func main() {
 	cfg, err := clientcmd.BuildConfigFromFlags(masterURL, kubeConfig)
 	exitOnError(err, "Error building kubeconfig")
 
-	kubeClientSet, err := kubernetes.NewForConfig(cfg)
-	exitOnError(err, "Error building clientset")
-
 	restMapper, err := util.BuildRestMapper(cfg)
 	exitOnError(err, "Error building rest mapper")
 
@@ -125,11 +121,10 @@ func main() {
 		LocalClient:     localClient,
 		RestMapper:      restMapper,
 		Scheme:          scheme.Scheme,
-	}, kubeClientSet,
-		controller.AgentConfig{
-			ServiceImportCounterName: "submariner_service_import",
-			ServiceExportCounterName: "submariner_service_export",
-		})
+	}, controller.AgentConfig{
+		ServiceImportCounterName: "submariner_service_import",
+		ServiceExportCounterName: "submariner_service_export",
+	})
 	exitOnError(err, "Failed to create lighthouse agent")
 
 	if agentSpec.Uninstall {


Test patch:
diff --git a/pkg/agent/controller/controller_suite_test.go b/pkg/agent/controller/controller_suite_test.go
index c6be734..7ddd222 100644
--- a/pkg/agent/controller/controller_suite_test.go
+++ b/pkg/agent/controller/controller_suite_test.go
@@ -48,8 +48,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/client-go/dynamic"
-	"k8s.io/client-go/kubernetes"
-	fakeKubeClient "k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/tools/cache"
 	mcsv1a1 "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1"
@@ -97,8 +95,6 @@ type cluster struct {
 	localServiceImportClient *fake.DynamicResourceClient
 	localIngressIPClient     *fake.DynamicResourceClient
 	localEndpointSliceClient dynamic.ResourceInterface
-	localKubeClient          kubernetes.Interface
-	endpointsReactor         *fake.FailingReactor
 	agentController          *controller.Controller
 	chanByCondType           map[mcsv1a1.ServiceExportConditionType]chan mcsv1a1.ServiceExportCondition
 }
@@ -280,10 +276,6 @@ func (c *cluster) init(syncerConfig *broker.SyncerConfig) {
 	c.localIngressIPClient = c.localDynClient.Resource(*test.GetGroupVersionResourceFor(syncerConfig.RestMapper,
 		controller.GetGlobalIngressIPObj())).Namespace(serviceNamespace).(*fake.DynamicResourceClient)
 
-	fakeCS := fakeKubeClient.NewSimpleClientset()
-	c.endpointsReactor = fake.NewFailingReactorForResource(&fakeCS.Fake, "endpoints")
-	c.localKubeClient = fakeCS
-
 	c.chanByCondType = map[mcsv1a1.ServiceExportConditionType]chan mcsv1a1.ServiceExportCondition{}
 	c.chanByCondType[mcsv1a1.ServiceExportValid] = make(chan mcsv1a1.ServiceExportCondition, 100)
 	c.chanByCondType[constants.ServiceExportSynced] = make(chan mcsv1a1.ServiceExportCondition, 100)
@@ -348,7 +340,7 @@ func (c *cluster) start(t *testDriver, syncerConfig broker.SyncerConfig) {
 
 	serviceExportCounterName := "submariner_service_export" + bigint.String()
 
-	c.agentController, err = controller.New(&c.agentSpec, syncerConfig, c.localKubeClient,
+	c.agentController, err = controller.New(&c.agentSpec, syncerConfig,
 		controller.AgentConfig{
 			ServiceImportCounterName: serviceImportCounterName,
 			ServiceExportCounterName: serviceExportCounterName,
@@ -595,30 +587,18 @@ func (t *testDriver) awaitUpdatedEndpointSlice(expectedIPs []string) {
 }
 
 func (t *testDriver) createService() {
-	_, err := t.cluster1.localKubeClient.CoreV1().Services(t.service.Namespace).Create(context.TODO(), t.service, metav1.CreateOptions{})
-	Expect(err).To(Succeed())
-
 	test.CreateResource(t.cluster1.dynamicServiceClient().Namespace(t.service.Namespace), t.service)
 }
 
 func (t *testDriver) updateService() {
-	_, err := t.cluster1.localKubeClient.CoreV1().Services(t.service.Namespace).Update(context.TODO(), t.service, metav1.UpdateOptions{})
-	Expect(err).To(Succeed())
-
 	test.UpdateResource(t.cluster1.dynamicServiceClient().Namespace(t.service.Namespace), t.service)
 }
 
 func (t *testDriver) createEndpoints() {
-	_, err := t.cluster1.localKubeClient.CoreV1().Endpoints(t.endpoints.Namespace).Create(context.TODO(), t.endpoints, metav1.CreateOptions{})
-	Expect(err).To(Succeed())
-
 	test.CreateResource(t.dynamicEndpointsClient(), t.endpoints)
 }
 
 func (t *testDriver) updateEndpoints() {
-	_, err := t.cluster1.localKubeClient.CoreV1().Endpoints(t.endpoints.Namespace).Update(context.TODO(), t.endpoints, metav1.UpdateOptions{})
-	Expect(err).To(Succeed())
-
 	test.UpdateResource(t.dynamicEndpointsClient(), t.endpoints)
 }
 
@@ -653,9 +633,6 @@ func (t *testDriver) deleteServiceExport() {
 func (t *testDriver) deleteService() {
 	Expect(t.cluster1.dynamicServiceClient().Namespace(t.service.Namespace).Delete(context.TODO(), t.service.Name,
 		metav1.DeleteOptions{})).To(Succeed())
-
-	Expect(t.cluster1.localKubeClient.CoreV1().Services(t.service.Namespace).Delete(context.TODO(), t.service.Name,
-		metav1.DeleteOptions{})).To(Succeed())
 }
 
 func (t *testDriver) createGlobalIngressIP(ingressIP *unstructured.Unstructured) {
@@ -791,11 +768,10 @@ func (t *testDriver) awaitHeadlessServiceUnexported() {
 	t.awaitNoEndpointSlice(t.brokerEndpointSliceClient)
 	t.awaitNoEndpointSlice(t.cluster2.localEndpointSliceClient)
 
-	// Ensure the service's Endpoints are no longer being watched by updating the Endpoints and verifyjng the
+	// Ensure the service's Endpoints are no longer being watched by updating the Endpoints and verifying the
 	// EndpointSlice isn't recreated.
 	t.endpoints.Subsets[0].Addresses = append(t.endpoints.Subsets[0].Addresses, corev1.EndpointAddress{IP: "192.168.5.10"})
-	_, err := t.cluster1.localKubeClient.CoreV1().Endpoints(t.endpoints.Namespace).Update(context.TODO(), t.endpoints, metav1.UpdateOptions{})
-	Expect(err).To(Succeed())
+	t.updateEndpoints()
 
 	time.Sleep(200 * time.Millisecond)
 	t.awaitNoEndpointSlice(t.cluster1.localEndpointSliceClient)