Use the GeneralClient when accessing operatorv1.DNS resources

...instead of the ScopedClient. The ScopedClient should only be used for accessing resources in the operator namespace. Also, since it caches internally, it requires list and watch permissions. Also use MustUpdate instead of CreateOrUpdate to make the update-only semantics clear. Signed-off-by: Tom Pantelis <[email protected]>
skitt · May 28, 2024 · aba84de · aba84de
1 parent 1b2a272
commit aba84de
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 19 deletions.
diff --git a/controllers/servicediscovery/servicediscovery_controller.go b/controllers/servicediscovery/servicediscovery_controller.go
@@ -554,10 +554,9 @@ func (r *Reconciler) configureOpenshiftClusterDNSOperator(ctx context.Context, i
 func (r *Reconciler) updateLighthouseConfigInOpenshiftDNSOperator(ctx context.Context, instance *submarinerv1alpha1.ServiceDiscovery,
 	clusterIP string,
 ) error {
-	//nolint:wrapcheck // No need to wrap errors here
 	retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 		dnsOperator := &operatorv1.DNS{}
-		if err := r.ScopedClient.Get(ctx, types.NamespacedName{Name: defaultOpenShiftDNSController}, dnsOperator); err != nil {
+		if err := r.GeneralClient.Get(ctx, types.NamespacedName{Name: defaultOpenShiftDNSController}, dnsOperator); err != nil {
 			// microshift uses the coredns image, but the DNS operator and CRDs are off
 			if resource.IsNotFoundErr(err) {
 				err = r.configureDNSConfigMap(ctx, instance, microshiftDNSNamespace, microshiftDNSConfigMap)
@@ -575,20 +574,17 @@ func (r *Reconciler) updateLighthouseConfigInOpenshiftDNSOperator(ctx context.Co
 
 		dnsOperator.Spec.Servers = updatedForwardServers
 
-		toUpdate := &operatorv1.DNS{ObjectMeta: metav1.ObjectMeta{
-			Name:   dnsOperator.Name,
-			Labels: dnsOperator.Labels,
-		}}
+		err := util.MustUpdate[*operatorv1.DNS](ctx, resource.ForControllerClient(r.GeneralClient, "", dnsOperator), dnsOperator,
+			func(existing *operatorv1.DNS) (*operatorv1.DNS, error) {
+				existing.Spec = dnsOperator.Spec
+				for k, v := range dnsOperator.Labels {
+					existing.Labels[k] = v
+				}
 
-		result, err := controllerutil.CreateOrUpdate(ctx, r.ScopedClient, toUpdate, func() error {
-			toUpdate.Spec = dnsOperator.Spec
-			for k, v := range dnsOperator.Labels {
-				toUpdate.Labels[k] = v
-			}
-			return nil
-		})
+				return existing, nil
+			})
 
-		if result == controllerutil.OperationResultUpdated {
+		if err == nil {
 			log.Info("Updated Cluster DNS Operator", "DnsOperator.Name", dnsOperator.Name)
 		}
 		return err

diff --git a/controllers/servicediscovery/servicediscovery_controller_test.go b/controllers/servicediscovery/servicediscovery_controller_test.go
@@ -50,7 +50,8 @@ func testReconciliation() {
 	When("the openshift DNS config exists", func() {
 		Context("and the lighthouse config isn't present", func() {
 			BeforeEach(func() {
-				t.InitScopedClientObjs = append(t.InitScopedClientObjs, newDNSConfig(""), newDNSService(clusterIP))
+				t.InitScopedClientObjs = append(t.InitScopedClientObjs, newDNSService(clusterIP))
+				t.InitGeneralClientObjs = append(t.InitGeneralClientObjs, newDNSConfig(""))
 			})
 
 			It("should add it", func() {
@@ -64,7 +65,8 @@ func testReconciliation() {
 			updatedClusterIP := "10.10.10.11"
 
 			BeforeEach(func() {
-				t.InitScopedClientObjs = append(t.InitScopedClientObjs, newDNSConfig(clusterIP), newDNSService(updatedClusterIP))
+				t.InitScopedClientObjs = append(t.InitScopedClientObjs, newDNSService(updatedClusterIP))
+				t.InitGeneralClientObjs = append(t.InitGeneralClientObjs, newDNSConfig(clusterIP))
 			})
 
 			It("should update the lighthouse config", func() {
@@ -76,7 +78,7 @@ func testReconciliation() {
 
 		Context("and the lighthouse DNS service doesn't exist", func() {
 			BeforeEach(func() {
-				t.InitScopedClientObjs = append(t.InitScopedClientObjs, newDNSConfig(""))
+				t.InitGeneralClientObjs = append(t.InitGeneralClientObjs, newDNSConfig(""))
 			})
 
 			It("should create the service and add the lighthouse config", func() {
@@ -233,7 +235,7 @@ func testCoreDNSCleanup() {
 
 	When("the openshift DNS config exists", func() {
 		BeforeEach(func() {
-			t.InitScopedClientObjs = append(t.InitScopedClientObjs, newDNSConfig(clusterIP))
+			t.InitGeneralClientObjs = append(t.InitGeneralClientObjs, newDNSConfig(clusterIP))
 		})
 
 		It("should remove the lighthouse config", func() {

diff --git a/controllers/servicediscovery/servicediscovery_suite_test.go b/controllers/servicediscovery/servicediscovery_suite_test.go
@@ -123,7 +123,7 @@ func (t *testDriver) assertUninstallServiceDiscoveryDeployment() *appsv1.Deploym
 
 func (t *testDriver) getDNSConfig() (*operatorv1.DNS, error) {
 	foundDNSConfig := &operatorv1.DNS{}
-	err := t.ScopedClient.Get(context.TODO(), types.NamespacedName{Name: openShiftDNSConfigName}, foundDNSConfig)
+	err := t.GeneralClient.Get(context.TODO(), types.NamespacedName{Name: openShiftDNSConfigName}, foundDNSConfig)
 
 	return foundDNSConfig, err
 }