Repository: submariner-io/submariner-operator (go)

Issue:
Allow to set the name of the custom coredns configmap
**What would you like to be added**:

The ability to set the name of the custom configmap to coredns.

**Why is this needed**:

Similar to #1054, in our environment we can provide custom configmaps to coredns in kube-system but the name of the configmap must be coredns-extra-configs. Currently we have to deploy this manually:

```
apiVersion: v1
data:
  Corefile: |
    clusterset.local:53 {
        forward . <<lighthouse-coredns-clusterip>>
    }
kind: ConfigMap
metadata:
  name: coredns-extra-configs
  namespace: kube-system
```

It would be nice to have this deployed by the operator. Would also really aprecciate to have this backported to 0.8.1.

Pull request description:
Allow user to set name of CoreDNS custom ConfigMap
Currently we use `coredns-custom` as only possible name for custom
CoreDNS configMap. Not all deployments use this name, so give users
an option to configure this at the time of join.

Fixes #1166

Signed-off-by: Vishal Thapar <5137689+vthapar@users.noreply.github.com>

Patch:
diff --git a/apis/submariner/v1alpha1/servicediscovery_types.go b/apis/submariner/v1alpha1/servicediscovery_types.go
index b39e9bdb..b95c2e61 100644
--- a/apis/submariner/v1alpha1/servicediscovery_types.go
+++ b/apis/submariner/v1alpha1/servicediscovery_types.go
@@ -29,16 +29,17 @@ import (
 // ServiceDiscoverySpec defines the desired state of ServiceDiscovery
 // +k8s:openapi-gen=true
 type ServiceDiscoverySpec struct {
-	BrokerK8sApiServer       string `json:"brokerK8sApiServer"`
-	BrokerK8sApiServerToken  string `json:"brokerK8sApiServerToken"`
-	BrokerK8sCA              string `json:"brokerK8sCA"`
-	BrokerK8sRemoteNamespace string `json:"brokerK8sRemoteNamespace"`
-	ClusterID                string `json:"clusterID"`
-	Namespace                string `json:"namespace"`
-	Repository               string `json:"repository,omitempty"`
-	Version                  string `json:"version,omitempty"`
-	Debug                    bool   `json:"debug"`
-	GlobalnetEnabled         bool   `json:"globalnetEnabled,omitempty"`
+	BrokerK8sApiServer       string               `json:"brokerK8sApiServer"`
+	BrokerK8sApiServerToken  string               `json:"brokerK8sApiServerToken"`
+	BrokerK8sCA              string               `json:"brokerK8sCA"`
+	BrokerK8sRemoteNamespace string               `json:"brokerK8sRemoteNamespace"`
+	ClusterID                string               `json:"clusterID"`
+	Namespace                string               `json:"namespace"`
+	Repository               string               `json:"repository,omitempty"`
+	Version                  string               `json:"version,omitempty"`
+	Debug                    bool                 `json:"debug"`
+	GlobalnetEnabled         bool                 `json:"globalnetEnabled,omitempty"`
+	CoreDNSCustomConfig      *CoreDNSCustomConfig `json:"coreDNSCustomConfig,omitempty"`
 	// +listType=set
 	CustomDomains  []string          `json:"customDomains,omitempty"`
 	ImageOverrides map[string]string `json:"imageOverrides,omitempty"`
@@ -47,6 +48,11 @@ type ServiceDiscoverySpec struct {
 	// Add custom validation using kubebuilder tags: https://book-v1.book.kubebuilder.io/beyond_basics/generating_crd.html
 }
 
+type CoreDNSCustomConfig struct {
+	ConfigMapName string `json:"configMapName,omitempty"`
+	Namespace     string `json:"namespace,omitempty"`
+}
+
 // ServiceDiscoveryStatus defines the observed state of ServiceDiscovery
 // +k8s:openapi-gen=true
 type ServiceDiscoveryStatus struct {
diff --git a/apis/submariner/v1alpha1/submariner_types.go b/apis/submariner/v1alpha1/submariner_types.go
index ce6dbafe..cbfd8ff6 100644
--- a/apis/submariner/v1alpha1/submariner_types.go
+++ b/apis/submariner/v1alpha1/submariner_types.go
@@ -34,27 +34,28 @@ import (
 // SubmarinerSpec defines the desired state of Submariner
 // +k8s:openapi-gen=true
 type SubmarinerSpec struct {
-	Broker                   string `json:"broker"`
-	BrokerK8sApiServer       string `json:"brokerK8sApiServer"`
-	BrokerK8sApiServerToken  string `json:"brokerK8sApiServerToken"`
-	BrokerK8sCA              string `json:"brokerK8sCA"`
-	BrokerK8sRemoteNamespace string `json:"brokerK8sRemoteNamespace"`
-	CableDriver              string `json:"cableDriver,omitempty"`
-	CeIPSecPSK               string `json:"ceIPSecPSK"`
-	ClusterCIDR              string `json:"clusterCIDR"`
-	ClusterID                string `json:"clusterID"`
-	ColorCodes               string `json:"colorCodes,omitempty"`
-	Repository               string `json:"repository,omitempty"`
-	ServiceCIDR              string `json:"serviceCIDR"`
-	GlobalCIDR               string `json:"globalCIDR,omitempty"`
-	Namespace                string `json:"namespace"`
-	Version                  string `json:"version,omitempty"`
-	CeIPSecIKEPort           int    `json:"ceIPSecIKEPort,omitempty"`
-	CeIPSecNATTPort          int    `json:"ceIPSecNATTPort,omitempty"`
-	CeIPSecDebug             bool   `json:"ceIPSecDebug"`
-	Debug                    bool   `json:"debug"`
-	NatEnabled               bool   `json:"natEnabled"`
-	ServiceDiscoveryEnabled  bool   `json:"serviceDiscoveryEnabled,omitempty"`
+	Broker                   string               `json:"broker"`
+	BrokerK8sApiServer       string               `json:"brokerK8sApiServer"`
+	BrokerK8sApiServerToken  string               `json:"brokerK8sApiServerToken"`
+	BrokerK8sCA              string               `json:"brokerK8sCA"`
+	BrokerK8sRemoteNamespace string               `json:"brokerK8sRemoteNamespace"`
+	CableDriver              string               `json:"cableDriver,omitempty"`
+	CeIPSecPSK               string               `json:"ceIPSecPSK"`
+	ClusterCIDR              string               `json:"clusterCIDR"`
+	ClusterID                string               `json:"clusterID"`
+	ColorCodes               string               `json:"colorCodes,omitempty"`
+	Repository               string               `json:"repository,omitempty"`
+	ServiceCIDR              string               `json:"serviceCIDR"`
+	GlobalCIDR               string               `json:"globalCIDR,omitempty"`
+	Namespace                string               `json:"namespace"`
+	Version                  string               `json:"version,omitempty"`
+	CeIPSecIKEPort           int                  `json:"ceIPSecIKEPort,omitempty"`
+	CeIPSecNATTPort          int                  `json:"ceIPSecNATTPort,omitempty"`
+	CeIPSecDebug             bool                 `json:"ceIPSecDebug"`
+	Debug                    bool                 `json:"debug"`
+	NatEnabled               bool                 `json:"natEnabled"`
+	ServiceDiscoveryEnabled  bool                 `json:"serviceDiscoveryEnabled,omitempty"`
+	CoreDNSCustomConfig      *CoreDNSCustomConfig `json:"coreDNSCustomConfig,omitempty"`
 	// +listType=set
 	CustomDomains  []string          `json:"customDomains,omitempty"`
 	ImageOverrides map[string]string `json:"imageOverrides,omitempty"`
diff --git a/apis/submariner/v1alpha1/zz_generated.deepcopy.go b/apis/submariner/v1alpha1/zz_generated.deepcopy.go
index 0aa0c3ad..bccda0c5 100644
--- a/apis/submariner/v1alpha1/zz_generated.deepcopy.go
+++ b/apis/submariner/v1alpha1/zz_generated.deepcopy.go
@@ -127,6 +127,21 @@ func (in *BrokerStatus) DeepCopy() *BrokerStatus {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *CoreDNSCustomConfig) DeepCopyInto(out *CoreDNSCustomConfig) {
+	*out = *in
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CoreDNSCustomConfig.
+func (in *CoreDNSCustomConfig) DeepCopy() *CoreDNSCustomConfig {
+	if in == nil {
+		return nil
+	}
+	out := new(CoreDNSCustomConfig)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *DaemonSetStatus) DeepCopyInto(out *DaemonSetStatus) {
 	*out = *in
@@ -235,6 +250,11 @@ func (in *ServiceDiscoveryList) DeepCopyObject() runtime.Object {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ServiceDiscoverySpec) DeepCopyInto(out *ServiceDiscoverySpec) {
 	*out = *in
+	if in.CoreDNSCustomConfig != nil {
+		in, out := &in.CoreDNSCustomConfig, &out.CoreDNSCustomConfig
+		*out = new(CoreDNSCustomConfig)
+		**out = **in
+	}
 	if in.CustomDomains != nil {
 		in, out := &in.CustomDomains, &out.CustomDomains
 		*out = make([]string, len(*in))
@@ -336,6 +356,11 @@ func (in *SubmarinerList) DeepCopyObject() runtime.Object {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *SubmarinerSpec) DeepCopyInto(out *SubmarinerSpec) {
 	*out = *in
+	if in.CoreDNSCustomConfig != nil {
+		in, out := &in.CoreDNSCustomConfig, &out.CoreDNSCustomConfig
+		*out = new(CoreDNSCustomConfig)
+		**out = **in
+	}
 	if in.CustomDomains != nil {
 		in, out := &in.CustomDomains, &out.CustomDomains
 		*out = make([]string, len(*in))
diff --git a/config/crd/bases/submariner.io_servicediscoveries.yaml b/config/crd/bases/submariner.io_servicediscoveries.yaml
index 741b6896..f71924da 100644
--- a/config/crd/bases/submariner.io_servicediscoveries.yaml
+++ b/config/crd/bases/submariner.io_servicediscoveries.yaml
@@ -46,6 +46,13 @@ spec:
                 type: string
               clusterID:
                 type: string
+              coreDNSCustomConfig:
+                properties:
+                  configMapName:
+                    type: string
+                  namespace:
+                    type: string
+                type: object
               customDomains:
                 items:
                   type: string
diff --git a/config/crd/bases/submariner.io_submariners.yaml b/config/crd/bases/submariner.io_submariners.yaml
index 35172a53..f8113aeb 100644
--- a/config/crd/bases/submariner.io_submariners.yaml
+++ b/config/crd/bases/submariner.io_submariners.yaml
@@ -76,6 +76,13 @@ spec:
                     format: int64
                     type: integer
                 type: object
+              coreDNSCustomConfig:
+                properties:
+                  configMapName:
+                    type: string
+                  namespace:
+                    type: string
+                type: object
               customDomains:
                 items:
                   type: string
diff --git a/controllers/servicediscovery/servicediscovery_controller.go b/controllers/servicediscovery/servicediscovery_controller.go
index 20c2814b..a2785453 100644
--- a/controllers/servicediscovery/servicediscovery_controller.go
+++ b/controllers/servicediscovery/servicediscovery_controller.go
@@ -62,9 +62,8 @@ const (
 	lighthouseCoreDNSName         = "submariner-lighthouse-coredns"
 	defaultOpenShiftDNSController = "default"
 	lighthouseForwardPluginName   = "lighthouse"
-	coreDNSNamespace              = "kube-system"
+	defaultCoreDNSNamespace       = "kube-system"
 	coreDNSName                   = "coredns"
-	customCoreDNSName             = "coredns-custom"
 )
 
 // NewReconciler returns a new ServiceDiscoveryReconciler
@@ -173,12 +172,14 @@ func (r *ServiceDiscoveryReconciler) Reconcile(request reconcile.Request) (recon
 	if err != nil {
 		return reconcile.Result{}, err
 	}
-	err = updateDNSCustomConfigMap(r.client, r.k8sClientSet, instance, reqLogger)
-	if errors.IsNotFound(err) {
+	if instance.Spec.CoreDNSCustomConfig != nil && instance.Spec.CoreDNSCustomConfig.ConfigMapName != "" {
+		err = updateDNSCustomConfigMap(r.client, r.k8sClientSet, instance, reqLogger)
+		if err != nil {
+			reqLogger.Error(err, "Error updating the 'custom-coredns' ConfigMap")
+			return reconcile.Result{}, err
+		}
+	} else {
 		err = updateDNSConfigMap(r.client, r.k8sClientSet, instance, reqLogger)
-	} else if err != nil {
-		reqLogger.Error(err, "Error updating the 'custom-coredns' ConfigMap")
-		return reconcile.Result{}, err
 	}
 
 	if errors.IsNotFound(err) {
@@ -276,6 +277,19 @@ prometheus :9153
 	}
 }
 
+func newCoreDNSCustomConfigMap(cr *submarinerv1alpha1.ServiceDiscovery) *corev1.ConfigMap {
+	namespace := defaultCoreDNSNamespace
+	if cr.Spec.CoreDNSCustomConfig.Namespace != "" {
+		namespace = cr.Spec.CoreDNSCustomConfig.Namespace
+	}
+	return &corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      cr.Spec.CoreDNSCustomConfig.ConfigMapName,
+			Namespace: namespace,
+		},
+	}
+}
+
 func newLighthouseCoreDNSDeployment(cr *submarinerv1alpha1.ServiceDiscovery) *appsv1.Deployment {
 	replicas := int32(2)
 	labels := map[string]string{
@@ -379,10 +393,22 @@ func newLighthouseCoreDNSService(cr *submarinerv1alpha1.ServiceDiscovery) *corev
 
 func updateDNSCustomConfigMap(client controllerClient.Client, k8sclientSet clientset.Interface, cr *submarinerv1alpha1.ServiceDiscovery,
 	reqLogger logr.Logger) error {
+	var configFunc func(*corev1.ConfigMap) (*corev1.ConfigMap, error)
+	customCoreDNSName := cr.Spec.CoreDNSCustomConfig.ConfigMapName
+	coreDNSNamespace := defaultCoreDNSNamespace
+	if cr.Spec.CoreDNSCustomConfig.Namespace != "" {
+		coreDNSNamespace = cr.Spec.CoreDNSCustomConfig.Namespace
+	}
+
 	retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 		configMap, err := k8sclientSet.CoreV1().ConfigMaps(coreDNSNamespace).Get(customCoreDNSName, metav1.GetOptions{})
-		if err != nil {
+		if errors.IsNotFound(err) {
+			configFunc = k8sclientSet.CoreV1().ConfigMaps(coreDNSNamespace).Create
+			configMap = newCoreDNSCustomConfigMap(cr)
+		} else if err != nil {
 			return err
+		} else {
+			configFunc = k8sclientSet.CoreV1().ConfigMaps(coreDNSNamespace).Update
 		}
 
 		lighthouseDNSService := &corev1.Service{}
@@ -409,7 +435,7 @@ func updateDNSCustomConfigMap(client controllerClient.Client, k8sclientSet clien
 		log.Info("Updating coredns-custom ConfigMap for lighthouse.server: " + coreFile)
 		configMap.Data["lighthouse.server"] = coreFile
 		// Potentially retried
-		_, err = k8sclientSet.CoreV1().ConfigMaps(coreDNSNamespace).Update(configMap)
+		_, err = configFunc(configMap)
 		return err
 	})
 	return retryErr
@@ -417,6 +443,7 @@ func updateDNSCustomConfigMap(client controllerClient.Client, k8sclientSet clien
 
 func updateDNSConfigMap(client controllerClient.Client, k8sclientSet clientset.Interface, cr *submarinerv1alpha1.ServiceDiscovery,
 	reqLogger logr.Logger) error {
+	coreDNSNamespace := defaultCoreDNSNamespace
 	retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 		configMap, err := k8sclientSet.CoreV1().ConfigMaps(coreDNSNamespace).Get(coreDNSName, metav1.GetOptions{})
 		if err != nil {
diff --git a/controllers/submariner/submariner_controller.go b/controllers/submariner/submariner_controller.go
index 302834f5..b680d97a 100644
--- a/controllers/submariner/submariner_controller.go
+++ b/controllers/submariner/submariner_controller.go
@@ -367,6 +367,10 @@ func (r *SubmarinerReconciler) serviceDiscoveryReconciler(submariner *submopv1a1
 					GlobalnetEnabled:         submariner.Spec.GlobalCIDR != "",
 					ImageOverrides:           submariner.Spec.ImageOverrides,
 				}
+				if submariner.Spec.CoreDNSCustomConfig != nil {
+					sd.Spec.CoreDNSCustomConfig.ConfigMapName = submariner.Spec.CoreDNSCustomConfig.ConfigMapName
+					sd.Spec.CoreDNSCustomConfig.Namespace = submariner.Spec.CoreDNSCustomConfig.Namespace
+				}
 				if len(submariner.Spec.CustomDomains) > 0 {
 					sd.Spec.CustomDomains = submariner.Spec.CustomDomains
 				}
diff --git a/pkg/subctl/cmd/join.go b/pkg/subctl/cmd/join.go
index 306b124f..819b8499 100644
--- a/pkg/subctl/cmd/join.go
+++ b/pkg/subctl/cmd/join.go
@@ -74,6 +74,7 @@ var (
 	healthCheckEnable             bool
 	healthCheckInterval           uint64
 	healthCheckMaxPacketLossCount uint64
+	corednsCustomConfigMap        string
 )
 
 func init() {
@@ -130,6 +131,9 @@ func addJoinFlags(cmd *cobra.Command) {
 		"maximum number of packets lost before the connection is marked as down")
 	cmd.Flags().BoolVar(&globalnetEnabled, "globalnet", true,
 		"enable/disable Globalnet for this cluster")
+	cmd.Flags().StringVar(&corednsCustomConfigMap, "coredns-custom-configmap", "",
+		"Name of the custom CoreDNS configmap to configure forwarding to lighthouse. It should be in "+
+			"<namespace>/<name> format where <namespace> is optional and defaults to kube-system")
 }
 
 const (
@@ -150,6 +154,8 @@ var joinCmd = &cobra.Command{
 		exitOnError("Error loading the broker information from the given file", err)
 		fmt.Printf("* %s says broker is at: %s\n", args[0], subctlData.BrokerURL)
 		exitOnError("Error connecting to broker cluster", err)
+		err = isValidCustomCoreDNSConfig()
+		exitOnError("Invalid Custom CoreDNS configuration", err)
 		config := getClientConfig(kubeConfig, kubeContext)
 		joinSubmarinerCluster(config, kubeContext, subctlData)
 	},
@@ -499,6 +505,13 @@ func populateSubmarinerSpec(subctlData *datafile.SubctlData, netconfig globalnet
 	if netconfig.GlobalnetCIDR != "" {
 		submarinerSpec.GlobalCIDR = netconfig.GlobalnetCIDR
 	}
+	if corednsCustomConfigMap != "" {
+		namespace, name := getCustomCoreDNSParams()
+		submarinerSpec.CoreDNSCustomConfig = &submariner.CoreDNSCustomConfig{
+			ConfigMapName: name,
+			Namespace:     namespace,
+		}
+	}
 	if len(customDomains) > 0 {
 		submarinerSpec.CustomDomains = customDomains
 	}
@@ -564,6 +577,14 @@ func populateServiceDiscoverySpec(subctlData *datafile.SubctlData) *submariner.S
 		ImageOverrides:           getImageOverrides(),
 	}
 
+	if corednsCustomConfigMap != "" {
+		namespace, name := getCustomCoreDNSParams()
+		serviceDiscoverySpec.CoreDNSCustomConfig = &submariner.CoreDNSCustomConfig{
+			ConfigMapName: name,
+			Namespace:     namespace,
+		}
+	}
+
 	if len(customDomains) > 0 {
 		serviceDiscoverySpec.CustomDomains = customDomains
 	}
@@ -597,3 +618,22 @@ func getImageOverrides() map[string]string {
 	}
 	return nil
 }
+
+func isValidCustomCoreDNSConfig() error {
+	if corednsCustomConfigMap != "" && strings.Count(corednsCustomConfigMap, "/") > 1 {
+		return fmt.Errorf("coredns-custom-configmap should be in <namespace>/<name> format, namespace is optional")
+	}
+	return nil
+}
+
+func getCustomCoreDNSParams() (namespace, name string) {
+	if corednsCustomConfigMap != "" {
+		name = corednsCustomConfigMap
+		paramList := strings.Split(corednsCustomConfigMap, "/")
+		if len(paramList) > 1 {
+			namespace = paramList[0]
+			name = paramList[1]
+		}
+	}
+	return namespace, name
+}


Test patch:
diff --git a/controllers/servicediscovery/servicediscovery_controller_test.go b/controllers/servicediscovery/servicediscovery_controller_test.go
index e8e50798..8eb6f922 100644
--- a/controllers/servicediscovery/servicediscovery_controller_test.go
+++ b/controllers/servicediscovery/servicediscovery_controller_test.go
@@ -258,7 +258,7 @@ func newConfigMap(lighthouseConfig string) *corev1.ConfigMap {
 	return &corev1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      coreDNSName,
-			Namespace: coreDNSNamespace,
+			Namespace: defaultCoreDNSNamespace,
 		},
 		Data: map[string]string{
 			"Corefile": corefile,
@@ -318,7 +318,7 @@ func expectDNSConfigUpdated(name string, client controllerClient.Client) *operat
 }
 
 func expectCoreMapUpdated(client clientset.Interface) *corev1.ConfigMap {
-	foundCoreMap, err := client.CoreV1().ConfigMaps(coreDNSNamespace).Get(coreDNSName, metav1.GetOptions{})
+	foundCoreMap, err := client.CoreV1().ConfigMaps(defaultCoreDNSNamespace).Get(coreDNSName, metav1.GetOptions{})
 	Expect(err).To(Succeed())
 	return foundCoreMap
 }