fix/timesync: Ensure that auto-update waits for time sync

- Added check to not attempt auto update if time sync is needed and not yet successful (delays 30 second to recheck).
- Added resync of time when DHCP or link state changes if online
- Added conditional* fallback from configured* NTP servers to the IP-named NTP servers, and then to the DNS named ones if that fails
- Added conditional* fallback from the configured* HTTP servers to the default DNS named ones.
- Uses the configuration* option for how many queries to run in parallel
- Added known static IPs for time servers (in case DNS resolution isn't up yet)
- Added time.cloudflare.com to fall-back NTP servers

* Note: The UI for configuring many of these things doesn't exist yet, but the defaults are reasonable

Author: IDisposable

internal/network/netif.go

@@ -47,7 +47,7 @@ type NetworkInterfaceOptions struct {
 	DefaultHostname   string
 	OnStateChange     func(state *NetworkInterfaceState)
 	OnInitialCheck    func(state *NetworkInterfaceState)
-	OnDhcpLeaseChange func(lease *udhcpc.Lease)
+	OnDhcpLeaseChange func(lease *udhcpc.Lease, state *NetworkInterfaceState)
 	OnConfigChange    func(config *NetworkConfig)
 	NetworkConfig     *NetworkConfig
 }
@@ -92,7 +92,7 @@ func NewNetworkInterfaceState(opts *NetworkInterfaceOptions) (*NetworkInterfaceS
 
 			_ = s.setHostnameIfNotSame()
 
-			opts.OnDhcpLeaseChange(lease)
+			opts.OnDhcpLeaseChange(lease, s)
 		},
 	})
 

internal/timesync/http.go

@@ -19,9 +19,11 @@ var defaultHTTPUrls = []string{
 	// "http://www.msftconnecttest.com/connecttest.txt",
 }
 
-func (t *TimeSync) queryAllHttpTime() (now *time.Time) {
+func (t *TimeSync) queryAllHttpTime(httpUrls []string) (now *time.Time) {
 	chunkSize := 4
-	httpUrls := t.httpUrls
+	if t.networkConfig.TimeSyncParallel.Valid {
+		chunkSize = int(t.networkConfig.TimeSyncParallel.Int64)
+	}
 
 	// shuffle the http urls to avoid always querying the same servers
 	rand.Shuffle(len(httpUrls), func(i, j int) { httpUrls[i], httpUrls[j] = httpUrls[j], httpUrls[i] })

internal/timesync/ntp.go

@@ -8,22 +8,39 @@ import (
 	"github.com/beevik/ntp"
 )
 
-var defaultNTPServers = []string{
+var defaultNTPServerIPs = []string{
+	// These servers are known by static IP and as such don't need DNS lookups
+	// These are from Google and Cloudflare since if they're down, the internet
+	// is broken anyway
+	"162.159.200.1",      // time.cloudflare.com IPv4
+	"162.159.200.123",    // time.cloudflare.com IPv4
+	"2606:4700:f1::1",    // time.cloudflare.com IPv6
+	"2606:4700:f1::123",  // time.cloudflare.com IPv6
+	"216.239.35.0",       // time.google.com IPv4
+	"216.239.35.4",       // time.google.com IPv4
+	"216.239.35.8",       // time.google.com IPv4
+	"216.239.35.12",      // time.google.com IPv4
+	"2001:4860:4806::",   // time.google.com IPv6
+	"2001:4860:4806:4::", // time.google.com IPv6
+	"2001:4860:4806:8::", // time.google.com IPv6
+	"2001:4860:4806:c::", // time.google.com IPv6
+}
+
+var defaultNTPServerHostnames = []string{
+	// should use something from https://github.com/jauderho/public-ntp-servers
 	"time.apple.com",
 	"time.aws.com",
 	"time.windows.com",
 	"time.google.com",
-	"162.159.200.123",   // time.cloudflare.com IPv4
-	"2606:4700:f1::123", // time.cloudflare.com IPv6
-	"0.pool.ntp.org",
-	"1.pool.ntp.org",
-	"2.pool.ntp.org",
-	"3.pool.ntp.org",
+	"time.cloudflare.com",
+	"pool.ntp.org",
 }
 
-func (t *TimeSync) queryNetworkTime() (now *time.Time, offset *time.Duration) {
+func (t *TimeSync) queryNetworkTime(ntpServers []string) (now *time.Time, offset *time.Duration) {
 	chunkSize := 4
-	ntpServers := t.ntpServers
+	if t.networkConfig.TimeSyncParallel.Valid {
+		chunkSize = int(t.networkConfig.TimeSyncParallel.Int64)
+	}
 
 	// shuffle the ntp servers to avoid always querying the same servers
 	rand.Shuffle(len(ntpServers), func(i, j int) { ntpServers[i], ntpServers[j] = ntpServers[j], ntpServers[i] })

internal/timesync/timesync.go

@@ -69,7 +69,7 @@ func NewTimeSync(opts *TimeSyncOptions) *TimeSync {
 		rtcDevicePath: rtcDevice,
 		rtcLock:       &sync.Mutex{},
 		preCheckFunc:  opts.PreCheckFunc,
-		ntpServers:    defaultNTPServers,
+		ntpServers:    defaultNTPServerIPs,
 		httpUrls:      defaultHTTPUrls,
 		networkConfig: opts.NetworkConfig,
 	}
@@ -159,11 +159,28 @@ func (t *TimeSync) Sync() error {
 	metricTimeSyncCount.Inc()
 
 	if syncMode.Ntp {
-		now, offset = t.queryNetworkTime()
+		// try the configured servers first
+		now, offset = t.queryNetworkTime(t.ntpServers)
+
+		if syncMode.NtpUseFallback && now == nil {
+			// now try the default hard-coded IP servers in case DNS is down
+			now, offset = t.queryNetworkTime(defaultNTPServerIPs)
+
+			if now == nil {
+				// unlikely to arrive here, but fall-back to DNS names default servers
+				now, offset = t.queryNetworkTime(defaultNTPServerHostnames)
+			}
+		}
 	}
 
 	if syncMode.Http && now == nil {
-		now = t.queryAllHttpTime()
+		// try the configured servers first
+		now = t.queryAllHttpTime(t.httpUrls)
+
+		if syncMode.HttpUseFallback && now == nil {
+			// unlikely to arrive here, but fall-back to DNS names default servers
+			now = t.queryAllHttpTime(defaultHTTPUrls)
+		}
 	}
 
 	if now == nil {

main.go

@@ -96,16 +96,25 @@ func Main() {
 			if !config.AutoUpdateEnabled {
 				return
 			}
+
+			if isTimeSyncNeeded() && !timeSync.IsSyncSuccess() {
+				logger.Debug().Msg("system time is not synced, will retry in 30 seconds")
+				time.Sleep(30 * time.Second)
+				continue
+			}
+
 			if currentSession != nil {
 				logger.Debug().Msg("skipping update since a session is active")
 				time.Sleep(1 * time.Minute)
 				continue
 			}
+
 			includePreRelease := config.IncludePreRelease
 			err = TryUpdate(context.Background(), GetDeviceID(), includePreRelease)
 			if err != nil {
 				logger.Warn().Err(err).Msg("failed to auto update")
 			}
+
 			time.Sleep(1 * time.Hour)
 		}
 	}()

network.go

@@ -15,7 +15,7 @@ var (
 	networkState *network.NetworkInterfaceState
 )
 
-func networkStateChanged() {
+func networkStateChanged(isOnline bool) {
 	// do not block the main thread
 	go waitCtrlAndRequestDisplayUpdate(true)
 
@@ -26,6 +26,13 @@ func networkStateChanged() {
 			networkState.GetFQDN(),
 		}, true)
 	}
+
+	// if the network is now online, trigger an NTP sync if still needed
+	if isOnline && timeSync != nil && (isTimeSyncNeeded() || !timeSync.IsSyncSuccess()) {
+		if err := timeSync.Sync(); err != nil {
+			logger.Warn().Str("error", err.Error()).Msg("unable to sync time on network state change")
+		}
+	}
 }
 
 func initNetwork() error {
@@ -37,13 +44,13 @@ func initNetwork() error {
 		NetworkConfig:   config.NetworkConfig,
 		Logger:          networkLogger,
 		OnStateChange: func(state *network.NetworkInterfaceState) {
-			networkStateChanged()
+			networkStateChanged(state.IsOnline())
 		},
 		OnInitialCheck: func(state *network.NetworkInterfaceState) {
-			networkStateChanged()
+			networkStateChanged(state.IsOnline())
 		},
-		OnDhcpLeaseChange: func(lease *udhcpc.Lease) {
-			networkStateChanged()
+		OnDhcpLeaseChange: func(lease *udhcpc.Lease, state *network.NetworkInterfaceState) {
+			networkStateChanged(state.IsOnline())
 
 			if currentSession == nil {
 				return
@@ -53,7 +60,7 @@ func initNetwork() error {
 		},
 		OnConfigChange: func(networkConfig *network.NetworkConfig) {
 			config.NetworkConfig = networkConfig
-			networkStateChanged()
+			networkStateChanged(false)
 
 			if mDNS != nil {
 				_ = mDNS.SetListenOptions(networkConfig.GetMDNSMode())