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") } }