From 440abc6fdc12eee768721a309b2fd53c15b6e61a Mon Sep 17 00:00:00 2001 From: Miguel Angel Ajo Date: Fri, 3 Dec 2021 10:10:32 +0100 Subject: [PATCH] Split mDNS service in host resolver, and route resolver The mDNS services are now split in route announcer and host announcer, in such way that worker nodes can still announce their names, but they won't announce routes. Signed-off-by: Miguel Angel Ajo --- pkg/cmd/run.go | 9 ++- pkg/mdns/controller.go | 17 +++--- pkg/mdns/{routes.go => route_controller.go} | 61 +++++++++++++------ ...outes_test.go => route_controller_test.go} | 30 ++++----- 4 files changed, 75 insertions(+), 42 deletions(-) rename pkg/mdns/{routes.go => route_controller.go} (66%) rename pkg/mdns/{routes_test.go => route_controller_test.go} (78%) diff --git a/pkg/cmd/run.go b/pkg/cmd/run.go index 8603e59e07..9aefccec57 100644 --- a/pkg/cmd/run.go +++ b/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))) } diff --git a/pkg/mdns/controller.go b/pkg/mdns/controller.go index e494437f29..58f87b50ea 100644 --- a/pkg/mdns/controller.go +++ b/pkg/mdns/controller.go @@ -5,7 +5,6 @@ import ( "net" "path/filepath" "strings" - "sync" "github.com/openshift/microshift/pkg/config" "github.com/openshift/microshift/pkg/mdns/server" @@ -13,13 +12,11 @@ import ( ) 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(): diff --git a/pkg/mdns/routes.go b/pkg/mdns/route_controller.go similarity index 66% rename from pkg/mdns/routes.go rename to pkg/mdns/route_controller.go index 662c165c67..fc118d7230 100644 --- a/pkg/mdns/routes.go +++ b/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 { diff --git a/pkg/mdns/routes_test.go b/pkg/mdns/route_controller_test.go similarity index 78% rename from pkg/mdns/routes_test.go rename to pkg/mdns/route_controller_test.go index 57d292eba1..2fbd2404c8 100644 --- a/pkg/mdns/routes_test.go +++ b/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") } }