feature: add dns controller to sync cluster node dns records #270

SataQiu · 2021-04-20T10:08:44Z

Ⅰ. Describe what this PR does

In the edge computing scenario, the IP addresses of the edge nodes are likely to be the same. So we can not rely on the node IP to forward the request but should use the node hostname(unique in one cluster).
This PR provides the ability for the tunnel-server to handle requests in the form of scheme://[hostname]:[port]/[req_path].

How to use it

The dns controller will generate a ConfigMap named yurt-tunnel-nodes in the kube-system namespace.
You need to manually config CoreDNS to use it according to the steps like the following:
First, mount the ConfigMap as volume of CoreDNS Pod, for example

        volumeMounts:
        - mountPath: /etc/edge
          name: hosts
          readOnly: true
...

      volumes:
      - configMap:
          defaultMode: 420
          name: yurt-tunnel-nodes
        name: hosts

Second, update CoreDNS ConfigMap (kube-system/coredns), add the hosts section

apiVersion: v1
data:
  Corefile: |
    .:53 {
        errors
        health :10260
        hosts /etc/edge/tunnel-nodes {
            reload 300ms
            fallthrough
        }
        kubernetes cluster.local in-addr.arpa ip6.arpa {
           pods insecure
           upstream
           fallthrough in-addr.arpa ip6.arpa
           endpoint http://169.254.2.1:10261
        }
        prometheus :9153
        proxy . /etc/resolv.conf
        cache 30
        reload
    }
kind: ConfigMap
metadata:
  name: coredns
  namespace: kube-system

Ⅱ. Does this pull request fix one issue?

Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

pkg/yurttunnel/dns/node.go

pkg/yurttunnel/dns/dns.go

pkg/yurttunnel/dns/service.go

pkg/yurttunnel/dns/node.go

go.mod

rambohe-ch · 2021-04-20T12:23:10Z

@SataQiu Thank you for making the pull request.
would you add more detail info like background, reason, solution etc to explain why we need this pull request?

rambohe-ch · 2021-04-25T08:41:54Z

pkg/yurttunnel/dns/dns.go

+				continue
+			}
+		}
+		records = append(records, formatDNSRecord(ip, node.Name))


records for edge nodes are forgotten?

We will record whether it is an edge node or not, but the destination ip is different.

rambohe-ch · 2021-04-25T08:44:21Z

pkg/yurttunnel/dns/dns.go

+	}
+
+	// parse tunnel server tcp port
+	tunnelServerAgentPort, _ := strconv.Atoi(constants.YurttunnelServerAgentPort)


tunnelServerAgentPort for insecure request is not 10262, is 10264(YurttunnelServerMasterInsecurePort)

rambohe-ch · 2021-04-25T09:06:30Z

pkg/yurttunnel/dns/util.go

+			found = true
+			if v != record {
+				result[i] = record
+				changed = true


how about add a break to exit for loop?

rambohe-ch · 2021-04-25T09:13:21Z

pkg/yurttunnel/util/util.go

+		} else {
+			klog.Errorf("fail to get configmap %s/%s: %v",
+				YurttunnelServerDnatConfigMapNs,
+				YurttunnelServerDnatConfigMapName, err)


I think that we need to add error return value for func GetConfiguredDnatPorts, and when we failed to get yurt-server-tunnel-cfg configmap, we can return error instead of empty slice.

rambohe-ch · 2021-04-25T09:17:00Z

pkg/yurttunnel/dns/dns.go

+	go wait.Until(func() {
+		if err := dnsctl.syncTunnelServerService(); err != nil {
+			klog.Errorf("failed to sync tunnel server service, %v", err)
+		}


why we need to add syncTunnelServerService here?

Based on past experience, we should not rely solely on the watch mechanism, as there is a risk of lost events. We need to make sure that the information is synchronized through a periodic list function.

SataQiu · 2021-04-26T09:17:46Z

config/setup/yurt-tunnel-server.yaml

@@ -127,6 +127,7 @@ spec:
        - yurt-tunnel-server
        args:
        - --bind-address=$(NODE_IP)
+        - --insecure-bind-address=$(NODE_IP)


Ensure that the tunnel-server listening address can be accessed by other cloud nodes in the cluster.

yeah, tunnel-server need to listen on node ip for insecure requests.

rambohe-ch · 2021-04-26T09:44:16Z

pkg/yurttunnel/dns/dns.go

+		}
+		updatedSvcPorts = append(updatedSvcPorts, port)
+	}
+	svc.Spec.Ports = updatedSvcPorts


how about add a flag to check service port need to be updated or not?

rambohe-ch

LGTM

feature: add dns controller to sync cluster node dns records