pillar: enhance NTP server handling

* allow setting more NTP servers through controller
* allow setting NTP server by domain instead of just IP address
  (problem: we only resolve the domains once at the beginning)

Signed-off-by: Christoph Ostarek <[email protected]>

pkg/pillar/cmd/diag/diag.go

@@ -961,8 +961,8 @@ func printOutput(ctx *diagContext, caller string) {
 			}
 			ctx.ph.Print("INFO: %s: Static Domain Name: %s\n",
 				ifname, port.DomainName)
-			ctx.ph.Print("INFO: %s: Static NTP server: %s\n",
-				ifname, port.NtpServer.String())
+			ctx.ph.Print("INFO: %s: Static NTP server: %+v\n",
+				ifname, port.ConfiguredNtpServers)
 		}
 		printProxy(ctx, port, ifname)
 		if port.HasError() {

pkg/pillar/cmd/zedagent/handlemetrics.go

@@ -1137,8 +1137,7 @@ func PublishAppInfoToZedCloud(ctx *zedagentContext, uuid string,
 			niStatus := appIfnameToNetworkInstance(ctx, aiStatus, ifname)
 			if niStatus != nil {
 				for _, ntpServer := range niStatus.NTPServers {
-					networkInfo.NtpServers = append(networkInfo.NtpServers,
-						ntpServer.String())
+					networkInfo.NtpServers = append(networkInfo.NtpServers, ntpServer)
 				}
 				networkInfo.DefaultRouters = []string{niStatus.Gateway.String()}
 				networkInfo.Dns = &info.ZInfoDNS{

pkg/pillar/cmd/zedagent/parseconfig.go

@@ -1375,7 +1375,7 @@ func parseOneSystemAdapterConfig(getconfigCtx *getconfigContext,
 			port.WirelessCfg = network.WirelessCfg
 			port.Gateway = network.Gateway
 			port.DomainName = network.DomainName
-			port.NTPServer = network.NTPServer
+			port.NTPServers = network.NTPServers
 			port.DNSServers = network.DNSServers
 			// Need to be careful since zedcloud can feed us bad Dhcp type
 			port.Dhcp = network.Dhcp
@@ -2255,11 +2255,10 @@ func parseIpspecNetworkXObject(ipspec *zconfig.Ipspec, config *types.NetworkXObj
 			return fmt.Errorf("invalid gateway IP (%s)", g)
 		}
 	}
-	if n := ipspec.GetNtp(); n != "" {
-		config.NTPServer = net.ParseIP(n)
-		if config.NTPServer == nil {
-			return fmt.Errorf("invalid NTP IP (%s)", n)
-		}
+
+	ntpServers := append([]string{ipspec.GetNtp()}, ipspec.GetMoreNtp()...)
+	if len(ntpServers) > 0 && ntpServers[0] != "" {
+		config.NTPServers = ntpServers
 	}
 	for _, dsStr := range ipspec.GetDns() {
 		ds := net.ParseIP(dsStr)
@@ -2288,9 +2287,12 @@ func parseIpspec(ipspec *zconfig.Ipspec,
 
 	config.DomainName = ipspec.GetDomain()
 	// Parse NTP Server
+	if config.NtpServers == nil {
+		config.NtpServers = make([]string, 0)
+	}
 	if n := ipspec.GetNtp(); n != "" {
-		config.NtpServer = net.ParseIP(n)
-		if config.NtpServer == nil {
+		config.NtpServers = append(config.NtpServers, n)
+		if config.NtpServers == nil {
 			return fmt.Errorf("invalid NTP IP (%s)", n)
 		}
 	}

pkg/pillar/cmd/zedagent/parseconfig_test.go

@@ -147,7 +147,7 @@ func TestParsePhysicalNetworkAdapters(t *testing.T) {
 	g.Expect(port.DhcpConfig.AddrSubnet).To(BeEmpty())
 	g.Expect(port.DhcpConfig.DNSServers).To(BeEmpty())
 	g.Expect(port.DhcpConfig.Gateway).To(BeNil())
-	g.Expect(port.DhcpConfig.NTPServer).To(BeNil())
+	g.Expect(port.DhcpConfig.NTPServers).To(BeNil())
 	g.Expect(port.ProxyConfig.Proxies).To(BeEmpty())
 	g.Expect(port.L2LinkConfig.L2Type).To(Equal(types.L2LinkTypeNone))
 	g.Expect(port.WirelessCfg.WType).To(Equal(types.WirelessTypeNone))

pkg/pillar/cmd/zedagent/reportinfo.go

@@ -889,7 +889,7 @@ func encodeNetInfo(port types.NetworkPortStatus) *info.ZInfoNetwork {
 
 	networkInfo.Proxy = encodeProxyStatus(&port.ProxyConfig)
 	networkInfo.NtpServers = []string{}
-	for _, server := range port.NtpServers {
+	for _, server := range port.DhcpNtpServers {
 		networkInfo.NtpServers = append(networkInfo.NtpServers, server.String())
 	}
 	return networkInfo
@@ -1042,12 +1042,25 @@ func encodeNetworkPortStatus(ctx *zedagentContext,
 	devicePort.Up = port.Up
 	devicePort.Mtu = uint32(port.MTU)
 	devicePort.Domainname = port.DomainName
-	// TODO: modify EVE APIs and allow to publish full list of NTP servers
-	if port.NtpServer != nil {
-		devicePort.NtpServer = port.NtpServer.String()
-	} else if len(port.NtpServers) > 0 {
-		devicePort.NtpServer = port.NtpServers[0].String()
+	ntpServers := make([]string, 0)
+	if port.ConfiguredNtpServers != nil {
+		ntpServers = append(ntpServers, port.ConfiguredNtpServers...)
+	}
+	if len(port.DhcpNtpServers) > 0 {
+		devicePort.NtpServer = port.DhcpNtpServers[0].String()
+		if len(port.DhcpNtpServers) > 1 {
+			for _, ntpServer := range port.DhcpNtpServers[1:] {
+				ntpServers = append(ntpServers, ntpServer.String())
+			}
+		}
+	}
+	if len(ntpServers) > 0 {
+		devicePort.NtpServer = ntpServers[0]
+	}
+	if len(ntpServers) > 1 {
+		devicePort.MoreNtpServers = ntpServers[1:]
 	}
+
 	devicePort.Proxy = encodeProxyStatus(&port.ProxyConfig)
 	devicePort.MacAddr = port.MacAddr.String()
 	for _, ipAddr := range port.AddrInfoList {
@@ -1150,7 +1163,13 @@ func encodeNetworkPortConfig(ctx *zedagentContext,
 	dp.DefaultRouters = make([]string, 0)
 	dp.DefaultRouters = append(dp.DefaultRouters, npc.Gateway.String())
 
-	dp.NtpServer = npc.NTPServer.String()
+	if npc.NTPServers != nil && len(npc.NTPServers) > 0 {
+		dp.NtpServer = npc.NTPServers[0]
+
+		if len(npc.NTPServers) > 1 {
+			dp.MoreNtpServers = append(dp.MoreNtpServers, npc.NTPServers[1:]...)
+		}
+	}
 
 	dp.Dns = new(info.ZInfoDNS)
 	dp.Dns.DNSdomain = npc.DomainName

pkg/pillar/cmd/zedrouter/networkinstance.go

@@ -124,12 +124,12 @@ func (z *zedrouter) updateNIPorts(niConfig types.NetworkInstanceConfig,
 	var (
 		newPorts         []*types.NetworkPortStatus
 		validatedPortLLs []string
-		newNTPServers    []net.IP
+		newNTPServers    []string
 		errorMsgs        []string
 	)
-	if niStatus.NtpServer != nil {
+	if niStatus.NtpServers != nil {
 		// The NTP server explicitly configured for the NI.
-		newNTPServers = append(newNTPServers, niStatus.NtpServer)
+		newNTPServers = append(newNTPServers, niStatus.NtpServers...)
 	}
 	if niStatus.PortLabel != "" {
 		newPorts = z.deviceNetworkStatus.LookupPortsByLabel(niStatus.PortLabel)
@@ -205,23 +205,24 @@ func (z *zedrouter) updateNIPorts(niConfig types.NetworkInstanceConfig,
 		}
 		// Port is valid for this network instance.
 		validatedPortLLs = append(validatedPortLLs, port.Logicallabel)
-		if port.NtpServer != nil {
+		if port.ConfiguredNtpServers != nil {
 			// The NTP server explicitly configured for the port.
-			newNTPServers = append(newNTPServers, port.NtpServer)
+			newNTPServers = append(newNTPServers, port.ConfiguredNtpServers...)
 		}
 		// NTP servers received via DHCP.
-		newNTPServers = append(newNTPServers, port.NtpServers...)
+		for _, dhcpNtpserver := range port.DhcpNtpServers {
+			newNTPServers = append(newNTPServers, dhcpNtpserver.String())
+		}
 	}
 	if niStatus.PortLabel != "" && len(newPorts) == 0 {
 		// This is potentially a transient state, wait for DNS update.
 		errorMsgs = append(errorMsgs,
 			fmt.Sprintf("no port is matching label '%s'", niStatus.PortLabel))
 	}
-	newNTPServers = generics.FilterDuplicatesFn(newNTPServers, netutils.EqualIPs)
+	newNTPServers = generics.FilterDuplicates(newNTPServers)
 	changed = changed || !generics.EqualSets(niStatus.Ports, validatedPortLLs)
 	niStatus.Ports = validatedPortLLs
-	changed = changed || !generics.EqualSetsFn(niStatus.NTPServers, newNTPServers,
-		netutils.EqualIPs)
+	changed = changed || !generics.EqualSets(niStatus.NTPServers, newNTPServers)
 	niStatus.NTPServers = newNTPServers
 	// Update BridgeMac for Switch NI bridge created by NIM.
 	if z.niBridgeIsCreatedByNIM(niConfig) {

pkg/pillar/dpcmanager/dns.go

@@ -66,7 +66,7 @@ func (m *DpcManager) updateDNS() {
 		// Start with any statically assigned values; update below
 		m.deviceNetStatus.Ports[ix].DomainName = port.DomainName
 		m.deviceNetStatus.Ports[ix].DNSServers = port.DNSServers
-		m.deviceNetStatus.Ports[ix].NtpServer = port.NTPServer
+		m.deviceNetStatus.Ports[ix].ConfiguredNtpServers = port.NTPServers
 		// Prefer errors recorded by DPC verification.
 		// New errors are recorded from this function only when there is none yet
 		// (HasError() == false).
@@ -269,7 +269,7 @@ func (m *DpcManager) getDHCPInfo(port *types.NetworkPortStatus) error {
 	if dhcpInfo.Subnet != nil {
 		port.Subnet = *dhcpInfo.Subnet
 	}
-	port.NtpServers = dhcpInfo.NtpServers
+	port.DhcpNtpServers = dhcpInfo.NtpServers
 	return nil
 }
 

pkg/pillar/dpcmanager/dpcmanager_test.go

@@ -1031,8 +1031,8 @@ func TestDNS(test *testing.T) {
 	t.Expect(eth0State.DomainName).To(Equal("eth-test-domain"))
 	t.Expect(eth0State.DNSServers).To(HaveLen(1))
 	t.Expect(eth0State.DNSServers[0].String()).To(Equal("8.8.8.8"))
-	t.Expect(eth0State.NtpServers).To(HaveLen(1))
-	t.Expect(eth0State.NtpServers[0].String()).To(Equal("132.163.96.5"))
+	t.Expect(eth0State.DhcpNtpServers).To(HaveLen(1))
+	t.Expect(eth0State.DhcpNtpServers[0].String()).To(Equal("132.163.96.5"))
 	t.Expect(eth0State.Subnet.String()).To(Equal("192.168.10.0/24"))
 	t.Expect(eth0State.MacAddr.String()).To(Equal("02:00:00:00:00:01"))
 	t.Expect(eth0State.Up).To(BeTrue())
@@ -1054,8 +1054,8 @@ func TestDNS(test *testing.T) {
 	t.Expect(eth1State.DomainName).To(Equal("eth-test-domain"))
 	t.Expect(eth1State.DNSServers).To(HaveLen(1))
 	t.Expect(eth1State.DNSServers[0].String()).To(Equal("1.1.1.1"))
-	t.Expect(eth1State.NtpServers).To(HaveLen(1))
-	t.Expect(eth1State.NtpServers[0].String()).To(Equal("132.163.96.6"))
+	t.Expect(eth1State.DhcpNtpServers).To(HaveLen(1))
+	t.Expect(eth1State.DhcpNtpServers[0].String()).To(Equal("132.163.96.6"))
 	t.Expect(eth1State.Subnet.String()).To(Equal("172.20.1.0/24"))
 	t.Expect(eth1State.MacAddr.String()).To(Equal("02:00:00:00:00:02"))
 	t.Expect(eth1State.Up).To(BeTrue())
@@ -1647,7 +1647,7 @@ func TestVlansAndBonds(test *testing.T) {
 	t.Expect(eth0State.IsL3Port).To(BeFalse())
 	t.Expect(eth0State.DomainName).To(BeEmpty())
 	t.Expect(eth0State.DNSServers).To(BeEmpty())
-	t.Expect(eth0State.NtpServers).To(BeEmpty())
+	t.Expect(eth0State.DhcpNtpServers).To(BeEmpty())
 	t.Expect(eth0State.Subnet.IP).To(BeNil())
 	t.Expect(eth0State.MacAddr.String()).To(Equal("02:00:00:00:00:01"))
 	t.Expect(eth0State.Up).To(BeTrue())
@@ -1664,7 +1664,7 @@ func TestVlansAndBonds(test *testing.T) {
 	t.Expect(eth1State.IsL3Port).To(BeFalse())
 	t.Expect(eth1State.DomainName).To(BeEmpty())
 	t.Expect(eth1State.DNSServers).To(BeEmpty())
-	t.Expect(eth1State.NtpServers).To(BeEmpty())
+	t.Expect(eth1State.DhcpNtpServers).To(BeEmpty())
 	t.Expect(eth1State.Subnet.IP).To(BeNil())
 	t.Expect(eth1State.MacAddr.String()).To(Equal("02:00:00:00:00:02"))
 	t.Expect(eth1State.Up).To(BeTrue())
@@ -1680,7 +1680,7 @@ func TestVlansAndBonds(test *testing.T) {
 	t.Expect(bond0State.IsL3Port).To(BeFalse())
 	t.Expect(bond0State.DomainName).To(BeEmpty())
 	t.Expect(bond0State.DNSServers).To(BeEmpty())
-	t.Expect(bond0State.NtpServers).To(BeEmpty())
+	t.Expect(bond0State.DhcpNtpServers).To(BeEmpty())
 	t.Expect(bond0State.Subnet.IP).To(BeNil())
 	t.Expect(bond0State.MacAddr.String()).To(Equal("02:00:00:00:00:03"))
 	t.Expect(bond0State.Up).To(BeTrue())
@@ -1698,8 +1698,8 @@ func TestVlansAndBonds(test *testing.T) {
 	t.Expect(vlan100State.DomainName).To(Equal("vlan100-test-domain"))
 	t.Expect(vlan100State.DNSServers).To(HaveLen(1))
 	t.Expect(vlan100State.DNSServers[0].String()).To(Equal("8.8.8.8"))
-	t.Expect(vlan100State.NtpServers).To(HaveLen(1))
-	t.Expect(vlan100State.NtpServers[0].String()).To(Equal("132.163.96.5"))
+	t.Expect(vlan100State.DhcpNtpServers).To(HaveLen(1))
+	t.Expect(vlan100State.DhcpNtpServers[0].String()).To(Equal("132.163.96.5"))
 	t.Expect(vlan100State.Subnet.String()).To(Equal("192.168.10.0/24"))
 	t.Expect(vlan100State.MacAddr.String()).To(Equal("02:00:00:00:00:04"))
 	t.Expect(vlan100State.Up).To(BeTrue())
@@ -1718,8 +1718,8 @@ func TestVlansAndBonds(test *testing.T) {
 	t.Expect(vlan200State.DomainName).To(Equal("vlan200-test-domain"))
 	t.Expect(vlan200State.DNSServers).To(HaveLen(1))
 	t.Expect(vlan200State.DNSServers[0].String()).To(Equal("1.1.1.1"))
-	t.Expect(vlan200State.NtpServers).To(HaveLen(1))
-	t.Expect(vlan200State.NtpServers[0].String()).To(Equal("132.163.96.6"))
+	t.Expect(vlan200State.DhcpNtpServers).To(HaveLen(1))
+	t.Expect(vlan200State.DhcpNtpServers[0].String()).To(Equal("132.163.96.6"))
 	t.Expect(vlan200State.Subnet.String()).To(Equal("172.20.1.0/24"))
 	t.Expect(vlan200State.MacAddr.String()).To(Equal("02:00:00:00:00:05"))
 	t.Expect(vlan200State.Up).To(BeTrue())

pkg/pillar/dpcreconciler/genericitems/dhcpcd.go

@@ -324,13 +324,14 @@ func (c *DhcpcdConfigurator) dhcpcdArgs(config types.DhcpConfig) (op string, arg
 				fmt.Sprintf("domain_name_servers=%s",
 					strings.Join(dnsServers, " ")))
 		}
-		if config.NTPServer != nil && !config.NTPServer.IsUnspecified() {
-			args = append(args, "--static",
-				fmt.Sprintf("ntp_servers=%s",
-					config.NTPServer.String()))
+		if config.NTPServers != nil {
+			for _, ntpServer := range config.NTPServers {
+				args = append(args, "--static", fmt.Sprintf("ntp_servers=%s", ntpServer))
+				args = append(args, extras...)
+			}
 		}
-		args = append(args, extras...)
 	}
+
 	return op, args
 }
 

pkg/pillar/dpcreconciler/genericitems/dhcpcd_test.go

@@ -25,7 +25,7 @@ func TestDhcpcdEqual(t *testing.T) {
 					AddrSubnet: "192.168.1.44/24",          // irrelevant
 					Gateway:    net.ParseIP("192.168.1.1"), // irrelevant
 					DomainName: "mydomain",                 // irrelevant
-					NTPServer:  net.ParseIP("192.168.1.1"), // irrelevant
+					NTPServers: []string{"192.168.1.1"},    // irrelevant
 					Type:       types.NetworkTypeIpv4Only,  // must match
 				},
 			},
@@ -93,7 +93,7 @@ func TestDhcpcdEqual(t *testing.T) {
 					Dhcp:       types.DhcpTypeStatic,
 					AddrSubnet: "192.168.1.44/24",
 					DomainName: "mydomain",
-					NTPServer:  net.ParseIP("192.168.1.1"),
+					NTPServers: []string{"192.168.1.1"},
 					DNSServers: []net.IP{net.ParseIP("8.8.8.8")},
 					Type:       types.NetworkTypeIpv4Only, // irrelevant
 				},
@@ -103,7 +103,7 @@ func TestDhcpcdEqual(t *testing.T) {
 					Dhcp:       types.DhcpTypeStatic,
 					AddrSubnet: "192.168.1.44/24",
 					DomainName: "mydomain",
-					NTPServer:  net.ParseIP("192.168.1.1"),
+					NTPServers: []string{"192.168.1.1"},
 					DNSServers: []net.IP{net.ParseIP("8.8.8.8")},
 					Type:       types.NetworkTypeIPv4, // irrelevant
 				},
@@ -117,7 +117,7 @@ func TestDhcpcdEqual(t *testing.T) {
 					Dhcp:       types.DhcpTypeStatic,
 					AddrSubnet: "192.168.1.44/24",
 					DomainName: "mydomain",
-					NTPServer:  net.ParseIP("192.168.1.1"),
+					NTPServers: []string{"192.168.1.1"},
 					DNSServers: []net.IP{net.ParseIP("8.8.8.8")}, // does not match
 				},
 			},
@@ -126,7 +126,7 @@ func TestDhcpcdEqual(t *testing.T) {
 					Dhcp:       types.DhcpTypeStatic,
 					AddrSubnet: "192.168.1.44/24",
 					DomainName: "mydomain",
-					NTPServer:  net.ParseIP("192.168.1.1"),
+					NTPServers: []string{"192.168.1.1"},
 					DNSServers: []net.IP{net.ParseIP("1.1.1.1")}, // does not match
 				},
 			},

pkg/pillar/nireconciler/linux_config.go

@@ -1095,6 +1095,30 @@ func (r *LinuxNIReconciler) getIntendedMetadataSrvCfg(niID uuid.UUID) (items []d
 	return items
 }
 
+func (r *LinuxNIReconciler) resolveNTPServers(ntpServers []string) []net.IP {
+	ret := make([]net.IP, 0)
+	for _, ntpServer := range ntpServers {
+		ip := net.ParseIP(ntpServer)
+		if ip != nil {
+			ret = append(ret, ip)
+			continue
+		}
+
+		// TODO: discuss with Milan how to use:
+		// devicenetwork.ResolveWithPortsLambda(ntpServer, XXX, devicenetwork.ResolveWithSrcIP)
+		// TODO: add timeout or resolve in background
+		ips, err := net.LookupIP(ntpServer)
+		if err != nil {
+			r.log.Warnf("could not lookup '%s': %v, skipping", ntpServer, err)
+			continue
+		}
+		ret = append(ret, ips...)
+	}
+
+	ret = generics.FilterDuplicatesFn(ret, netutils.EqualIPs)
+	return ret
+}
+
 func (r *LinuxNIReconciler) getIntendedDnsmasqCfg(niID uuid.UUID) (items []dg.Item) {
 	ni := r.nis[niID]
 	if ni.config.Type == types.NetworkInstanceTypeSwitch {
@@ -1130,18 +1154,19 @@ func (r *LinuxNIReconciler) getIntendedDnsmasqCfg(niID uuid.UUID) (items []dg.It
 	}
 	// Combine NTP servers assigned to the port(s) together with those statically
 	// configured for the network instance.
-	var ntpServers []net.IP
+	var ntpServers []string
 	for _, port := range ni.bridge.Ports {
 		ntpServers = append(ntpServers, port.NTPServers...)
 	}
-	if ni.config.NtpServer != nil {
-		ntpServers = append(ntpServers, ni.config.NtpServer)
+	if ni.config.NtpServers != nil {
+		ntpServers = append(ntpServers, ni.config.NtpServers...)
 	}
-	ntpServers = generics.FilterDuplicatesFn(ntpServers, netutils.EqualIPs)
+	ntpServers = generics.FilterDuplicates(ntpServers)
+	ntpServerIPs := r.resolveNTPServers(ntpServers)
 	var propagateRoutes []generic.IPRoute
 	// Use DHCP to propagate host routes towards user-configured NTP and DNS servers.
 	if bridgeIP != nil {
-		for _, ntpServer := range ntpServers {
+		for _, ntpServer := range ntpServerIPs {
 			propagateRoutes = append(propagateRoutes, generic.IPRoute{
 				DstNetwork: netutils.HostSubnet(ntpServer),
 				Gateway:    bridgeIP.IP,
@@ -1228,7 +1253,7 @@ func (r *LinuxNIReconciler) getIntendedDnsmasqCfg(niID uuid.UUID) (items []dg.It
 		WithDefaultRoute: withDefaultRoute,
 		DomainName:       ni.config.DomainName,
 		DNSServers:       ni.config.DnsServers,
-		NTPServers:       ntpServers,
+		NTPServers:       ntpServerIPs,
 		PropagateRoutes:  propagateRoutes,
 		MTU:              ni.bridge.MTU,
 	}