Split mDNS service in host resolver, and route resolver

pkg/cmd/run.go

@@ -78,6 +78,12 @@ func RunMicroshift(cfg *config.MicroshiftConfig, flags *pflag.FlagSet) error {
 	}
 
 	m := servicemanager.NewServiceManager()
+
+	mDNSController := mdns.NewMicroShiftmDNSController(cfg)
+
+	// all nodes must make themselves available via mDNS
+	util.Must(m.AddService(mDNSController))
+
 	if config.StringInList("controlplane", cfg.Roles) {
 		util.Must(m.AddService(controllers.NewEtcd(cfg)))
 		util.Must(m.AddService(controllers.NewKubeAPIServer(cfg)))
@@ -87,9 +93,8 @@ func RunMicroshift(cfg *config.MicroshiftConfig, flags *pflag.FlagSet) error {
 		util.Must(m.AddService(controllers.NewOpenShiftCRDManager(cfg)))
 		util.Must(m.AddService(controllers.NewOpenShiftAPIServer(cfg)))
 		util.Must(m.AddService(controllers.NewOpenShiftOAuth(cfg)))
-
 		util.Must(m.AddService(controllers.NewOpenShiftDefaultSCCManager(cfg)))
-		util.Must(m.AddService(mdns.NewMicroShiftmDNSController(cfg)))
+		util.Must(m.AddService(mDNSController.NewmDNSRouteController()))
 		util.Must(m.AddService(controllers.NewInfrastructureServices(cfg)))
 		util.Must(m.AddService(kustomize.NewKustomizer(cfg)))
 	}

pkg/mdns/controller.go

@@ -5,21 +5,18 @@ import (
 	"net"
 	"path/filepath"
 	"strings"
-	"sync"
 
 	"github.com/openshift/microshift/pkg/config"
 	"github.com/openshift/microshift/pkg/mdns/server"
 	"k8s.io/klog/v2"
 )
 
 type MicroShiftmDNSController struct {
-	sync.Mutex
 	NodeName   string
 	NodeIP     string
 	KubeConfig string
 	myIPs      []string
 	resolver   *server.Resolver
-	hostCount  map[string]int
 	stopCh     chan struct{}
 }
 
@@ -28,13 +25,19 @@ func NewMicroShiftmDNSController(cfg *config.MicroshiftConfig) *MicroShiftmDNSCo
 		NodeIP:     cfg.NodeIP,
 		NodeName:   cfg.NodeName,
 		KubeConfig: filepath.Join(cfg.DataDir, "resources", "kubeadmin", "kubeconfig"),
-		hostCount:  make(map[string]int),
 	}
 }
 
 func (s *MicroShiftmDNSController) Name() string { return "microshift-mdns-controller" }
 func (s *MicroShiftmDNSController) Dependencies() []string {
-	return []string{"openshift-default-scc-manager"}
+	return []string{}
+}
+
+func (s *MicroShiftmDNSController) NewmDNSRouteController() *MicroShiftmDNSRouteController {
+	return &MicroShiftmDNSRouteController{
+		parent:    s,
+		hostCount: make(map[string]int),
+	}
 }
 
 func (c *MicroShiftmDNSController) Run(ctx context.Context, ready chan<- struct{}, stopped chan<- struct{}) error {
@@ -70,14 +73,12 @@ func (c *MicroShiftmDNSController) Run(ctx context.Context, ready chan<- struct{
 		}
 
 		klog.InfoS("Host FQDN will be announced via mDNS", "fqdn", c.NodeName, "ips", ips)
-		c.resolver.AddDomain(c.NodeName, ips)
+		c.resolver.AddDomain(c.NodeName+".", ips)
 		c.myIPs = ips
 	}
 
 	close(ready)
 
-	go c.startRouteInformer(c.stopCh)
-
 	select {
 	case <-ctx.Done():
 

pkg/mdns/routes.go → pkg/mdns/route_controller.go

@@ -3,6 +3,7 @@ package mdns
 import (
 	"context"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/openshift/microshift/pkg/mdns/server"
@@ -22,11 +23,37 @@ import (
 
 const defaultResyncTime = time.Hour * 1
 
-func (c *MicroShiftmDNSController) restConfig() (*rest.Config, error) {
-	return clientcmd.BuildConfigFromFlags("", c.KubeConfig)
+type MicroShiftmDNSRouteController struct {
+	sync.Mutex
+	parent    *MicroShiftmDNSController
+	hostCount map[string]int
 }
 
-func (c *MicroShiftmDNSController) startRouteInformer(stopCh chan struct{}) error {
+func (s *MicroShiftmDNSRouteController) Name() string { return "microshift-mdns-route-controller" }
+func (s *MicroShiftmDNSRouteController) Dependencies() []string {
+	return []string{"openshift-api-components-manager"}
+}
+
+func (c *MicroShiftmDNSRouteController) Run(ctx context.Context, ready chan<- struct{}, stopped chan<- struct{}) error {
+	stopCh := make(chan struct{})
+	defer close(stopCh)
+	defer close(stopped)
+
+	go c.startRouteInformer(stopCh, ready)
+
+	select {
+	case <-ctx.Done():
+		return ctx.Err()
+	}
+
+	return nil
+}
+
+func (c *MicroShiftmDNSRouteController) restConfig() (*rest.Config, error) {
+	return clientcmd.BuildConfigFromFlags("", c.parent.KubeConfig)
+}
+
+func (c *MicroShiftmDNSRouteController) startRouteInformer(stopCh chan struct{}, ready chan<- struct{}) error {
 	klog.InfoS("Starting MicroShift mDNS route watcher")
 	cfg, err := c.restConfig()
 	if err != nil {
@@ -38,10 +65,10 @@ func (c *MicroShiftmDNSController) startRouteInformer(stopCh chan struct{}) erro
 		return errors.Wrap(err, "error creating dynamic informer")
 	}
 
-	return c.run(stopCh, dc)
+	return c.runInformers(stopCh, dc, ready)
 }
 
-func (c *MicroShiftmDNSController) run(stopCh chan struct{}, dc dynamic.Interface) error {
+func (c *MicroShiftmDNSRouteController) runInformers(stopCh chan struct{}, dc dynamic.Interface, ready chan<- struct{}) error {
 	informerFactory := dynamicinformer.NewFilteredDynamicSharedInformerFactory(dc, defaultResyncTime, v1.NamespaceAll, nil)
 	routersGVR, _ := schema.ParseResourceArg("routes.v1.route.openshift.io")
 
@@ -56,13 +83,13 @@ func (c *MicroShiftmDNSController) run(stopCh chan struct{}, dc dynamic.Interfac
 
 	informer.AddEventHandler(handlers)
 	informer.Run(stopCh)
-
+	close(ready)
 	return nil
 }
 
 // TODO: The need for this indicates that the openshift-default-scc-manager is declaring itself ready before
 //       it really is. If we don't wait for the route API the informer will start throwing ugly errors.
-func (c *MicroShiftmDNSController) waitForRouterAPI(dc dynamic.Interface, routersGVR *schema.GroupVersionResource) {
+func (c *MicroShiftmDNSRouteController) waitForRouterAPI(dc dynamic.Interface, routersGVR *schema.GroupVersionResource) {
 	backoff := wait.Backoff{
 		Cap:      3 * time.Minute,
 		Duration: 10 * time.Second,
@@ -88,7 +115,7 @@ func (c *MicroShiftmDNSController) waitForRouterAPI(dc dynamic.Interface, router
 	}
 }
 
-func (c *MicroShiftmDNSController) addedRoute(obj interface{}) {
+func (c *MicroShiftmDNSRouteController) addedRoute(obj interface{}) {
 	u := obj.(*unstructured.Unstructured)
 
 	host, found, err := unstructured.NestedString(u.UnstructuredContent(), "spec", "host")
@@ -100,7 +127,7 @@ func (c *MicroShiftmDNSController) addedRoute(obj interface{}) {
 	c.exposeHost(host)
 }
 
-func (c *MicroShiftmDNSController) updatedRoute(oldObj, newObj interface{}) {
+func (c *MicroShiftmDNSRouteController) updatedRoute(oldObj, newObj interface{}) {
 	oldU := oldObj.(*unstructured.Unstructured)
 
 	oldHost, _, _ := unstructured.NestedString(oldU.UnstructuredContent(), "spec", "host")
@@ -114,7 +141,7 @@ func (c *MicroShiftmDNSController) updatedRoute(oldObj, newObj interface{}) {
 	}
 }
 
-func (c *MicroShiftmDNSController) deletedRoute(obj interface{}) {
+func (c *MicroShiftmDNSRouteController) deletedRoute(obj interface{}) {
 	u := obj.(*unstructured.Unstructured)
 	host, found, err := unstructured.NestedString(u.UnstructuredContent(), "spec", "host")
 	if !found || err != nil {
@@ -125,33 +152,33 @@ func (c *MicroShiftmDNSController) deletedRoute(obj interface{}) {
 	c.unexposeHost(host)
 }
 
-func (c *MicroShiftmDNSController) exposeHost(host string) {
+func (c *MicroShiftmDNSRouteController) exposeHost(host string) {
 	if !strings.HasSuffix(host, server.DefaultmDNSTLD) {
 		klog.V(2).InfoS("mDNS ignoring host without mDNS suffix", "host", host)
 		return
 	}
 
-	klog.InfoS("mDNS: route found for", "host", host, "ips", c.myIPs)
+	klog.InfoS("mDNS: route found for", "host", host, "ips", c.parent.myIPs)
 
 	// TODO(multi-node) look up for the exact router service Endpoints instead of assuming our own IP (ok for single-node)
 	c.incHost(host)
-	c.resolver.AddDomain(host+".", c.myIPs)
+	c.parent.resolver.AddDomain(host+".", c.parent.myIPs)
 }
 
-func (c *MicroShiftmDNSController) unexposeHost(oldHost string) {
+func (c *MicroShiftmDNSRouteController) unexposeHost(oldHost string) {
 	if c.decHost(oldHost) == 0 {
 		klog.InfoS("mDNS removing, no more router references", "host", oldHost)
-		c.resolver.DeleteDomain(oldHost + ".")
+		c.parent.resolver.DeleteDomain(oldHost + ".")
 	}
 }
 
-func (c *MicroShiftmDNSController) incHost(name string) {
+func (c *MicroShiftmDNSRouteController) incHost(name string) {
 	c.Lock()
 	defer c.Unlock()
 	c.hostCount[name]++
 }
 
-func (c *MicroShiftmDNSController) decHost(name string) int {
+func (c *MicroShiftmDNSRouteController) decHost(name string) int {
 	c.Lock()
 	defer c.Unlock()
 	if c.hostCount[name] > 0 {

pkg/mdns/routes_test.go → pkg/mdns/route_controller_test.go

@@ -13,14 +13,14 @@ const testNodeName = "test-node.local"
 const testRouteHost = "test-route-host.cluster.local"
 const testRouteHost2 = "test-route-host2.cluster.local"
 
-func newTestController() *MicroShiftmDNSController {
-	return &MicroShiftmDNSController{
-		NodeIP:    testIP,
-		NodeName:  testNodeName,
-		resolver:  server.NewResolver(),
-		hostCount: make(map[string]int),
-		myIPs:     []string{testIP, testIPv6},
+func newTestController() *MicroShiftmDNSRouteController {
+	ctl := &MicroShiftmDNSController{
+		NodeIP:   testIP,
+		NodeName: testNodeName,
+		resolver: server.NewResolver(),
+		myIPs:    []string{testIP, testIPv6},
 	}
+	return ctl.NewmDNSRouteController()
 }
 
 func Test_addedRoute(t *testing.T) {
@@ -29,7 +29,7 @@ func Test_addedRoute(t *testing.T) {
 	unstructured.SetNestedField(route.Object, testRouteHost, "spec", "host")
 
 	ctl.addedRoute(route)
-	if !ctl.resolver.HasDomain(testRouteHost + ".") {
+	if !ctl.parent.resolver.HasDomain(testRouteHost + ".") {
 		t.Errorf("When a host is added, the mDNS resolver should expose it")
 	}
 }
@@ -42,12 +42,12 @@ func Test_deletedRoute(t *testing.T) {
 	ctl.addedRoute(route)
 	ctl.addedRoute(route)
 	ctl.deletedRoute(route)
-	if !ctl.resolver.HasDomain(testRouteHost + ".") {
+	if !ctl.parent.resolver.HasDomain(testRouteHost + ".") {
 		t.Errorf("When multiple routes share a hostname, deleting one route shouldn't stop exposing the host")
 	}
 
 	ctl.deletedRoute(route)
-	if ctl.resolver.HasDomain(testRouteHost + ".") {
+	if ctl.parent.resolver.HasDomain(testRouteHost + ".") {
 		t.Errorf("Deleting all routes exposing a hostname should stop exposing the host")
 	}
 }
@@ -63,11 +63,11 @@ func Test_updatedRoute(t *testing.T) {
 	ctl.addedRoute(routeOld)
 	ctl.updatedRoute(routeOld, routeNew)
 
-	if ctl.resolver.HasDomain(testRouteHost + ".") {
+	if ctl.parent.resolver.HasDomain(testRouteHost + ".") {
 		t.Errorf("Old domain must have updated")
 	}
 
-	if !ctl.resolver.HasDomain(testRouteHost2 + ".") {
+	if !ctl.parent.resolver.HasDomain(testRouteHost2 + ".") {
 		t.Errorf("The updated domain must resolve at this point")
 	}
 }
@@ -84,16 +84,16 @@ func Test_updatedRouteDupHost(t *testing.T) {
 	ctl.addedRoute(routeOld) // two routes with the same hostname
 	ctl.updatedRoute(routeOld, routeNew)
 
-	if !ctl.resolver.HasDomain(testRouteHost + ".") {
+	if !ctl.parent.resolver.HasDomain(testRouteHost + ".") {
 		t.Errorf("Old domain must have persisted, there is another route using it")
 	}
 
-	if !ctl.resolver.HasDomain(testRouteHost2 + ".") {
+	if !ctl.parent.resolver.HasDomain(testRouteHost2 + ".") {
 		t.Errorf("The updated domain must resolve at this point")
 	}
 
 	ctl.deletedRoute(routeOld) // deleted the second route we had with the host
-	if ctl.resolver.HasDomain(testRouteHost + ".") {
+	if ctl.parent.resolver.HasDomain(testRouteHost + ".") {
 		t.Errorf("Old domain must have be gone after deleting the 2nd route")
 	}
 }