[ntp-check] detect and use cloud provider's ntp servers

cmd/agent/dist/conf.d/ntp.d/conf.yaml.default

@@ -12,9 +12,9 @@ instances:
     #
     # offset_threshold: 60
 
-    ## @param host - string - optional - default: <X>.datadog.pool.ntp.org
-    ## NTP host to connect to, default is `<X>.datadog.pool.ntp.org` where
-    ## <X> is a number between 0 and 3.
+    ## @param host - string - optional - default: ntp host of cloud provider if detected, else <X>.datadog.pool.ntp.org
+    ## NTP host to connect to, default is the private NTP server of the Cloud Provider if one is detected, otherwise
+    ## `<X>.datadog.pool.ntp.org`, where <X> is a number between 0 and 3.
     #
     # host: <X>.datadog.pool.ntp.org
 

pkg/collector/corechecks/net/ntp.go

@@ -13,19 +13,22 @@ import (
 	"time"
 
 	"github.com/beevik/ntp"
-	yaml "gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v2"
 
 	"github.com/DataDog/datadog-agent/pkg/aggregator"
 	"github.com/DataDog/datadog-agent/pkg/autodiscovery/integration"
 	"github.com/DataDog/datadog-agent/pkg/collector/check"
 	core "github.com/DataDog/datadog-agent/pkg/collector/corechecks"
 	"github.com/DataDog/datadog-agent/pkg/metrics"
 	"github.com/DataDog/datadog-agent/pkg/telemetry"
+	"github.com/DataDog/datadog-agent/pkg/util"
 	"github.com/DataDog/datadog-agent/pkg/util/log"
 )
 
-const ntpCheckName = "ntp"
-const defaultMinCollectionInterval = 900 // 15 minutes, to follow pool.ntp.org's guidelines on the query rate
+const (
+	ntpCheckName                 = "ntp"
+	defaultMinCollectionInterval = 900 // 15 minutes, to follow pool.ntp.org's guidelines on the query rate
+)
 
 var (
 	ntpExpVar = expvar.NewFloat("ntpOffset")
@@ -73,7 +76,13 @@ func (c *ntpConfig) parse(data []byte, initData []byte, getLocalServers func() (
 	defaultPort := 123
 	defaultOffsetThreshold := 60
 
-	defaultHosts := []string{"0.datadog.pool.ntp.org", "1.datadog.pool.ntp.org", "2.datadog.pool.ntp.org", "3.datadog.pool.ntp.org"}
+	defaultHosts := util.GetCloudProviderNTPHosts()
+
+	// Default to our domains on pool.ntp.org if no cloud provider detected
+	if defaultHosts == nil {
+		log.Debug("No cloud provider detected, using default ntp pool.")
+		defaultHosts = []string{"0.datadog.pool.ntp.org", "1.datadog.pool.ntp.org", "2.datadog.pool.ntp.org", "3.datadog.pool.ntp.org"}
+	}
 
 	if err := yaml.Unmarshal(data, &instance); err != nil {
 		return err

pkg/collector/corechecks/net/ntp_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/stretchr/testify/mock"
 
 	"github.com/DataDog/datadog-agent/pkg/aggregator/mocksender"
+	"github.com/DataDog/datadog-agent/pkg/config"
 	"github.com/DataDog/datadog-agent/pkg/metrics"
 )
 
@@ -337,6 +338,7 @@ hosts:
 func TestDefaultHostConfig(t *testing.T) {
 	expectedHosts := []string{"0.datadog.pool.ntp.org", "1.datadog.pool.ntp.org", "2.datadog.pool.ntp.org", "3.datadog.pool.ntp.org"}
 	testedConfig := []byte(``)
+	config.Datadog.Set("cloud_provider_metadata", []string{})
 
 	ntpCheck := new(NTPCheck)
 	ntpCheck.Configure(testedConfig, []byte(""), "test")

pkg/util/alibaba/alibaba.go

@@ -44,6 +44,21 @@ func GetHostAlias() (string, error) {
 	return res, err
 }
 
+// GetNTPHosts returns the NTP hosts for Alibaba if it is detected as the cloud provider, otherwise an empty array.
+// These are their public NTP servers, as Alibaba uses two different types of private/internal networks for their cloud
+// machines and we can't be sure those servers are always accessible for every customer on every network type.
+// Docs: https://www.alibabacloud.com/help/doc-detail/92704.htm
+func GetNTPHosts() []string {
+	if IsRunningOn() {
+		return []string{
+			"ntp.aliyun.com", "ntp1.aliyun.com", "ntp2.aliyun.com", "ntp3.aliyun.com",
+			"ntp4.aliyun.com", "ntp5.aliyun.com", "ntp6.aliyun.com", "ntp7.aliyun.com",
+		}
+	}
+
+	return nil
+}
+
 func getResponseWithMaxLength(endpoint string, maxLength int) (string, error) {
 	result, err := getResponse(endpoint)
 	if err != nil {

pkg/util/alibaba/alibaba_test.go

@@ -12,6 +12,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	"github.com/DataDog/datadog-agent/pkg/config"
 )
 
 func TestGetHostname(t *testing.T) {
@@ -30,3 +32,22 @@ func TestGetHostname(t *testing.T) {
 	assert.Equal(t, expected, val)
 	assert.Equal(t, lastRequest.URL.Path, "/latest/meta-data/instance-id")
 }
+
+func TestGetNTPHosts(t *testing.T) {
+	expectedHosts := []string{
+		"ntp.aliyun.com", "ntp1.aliyun.com", "ntp2.aliyun.com", "ntp3.aliyun.com",
+		"ntp4.aliyun.com", "ntp5.aliyun.com", "ntp6.aliyun.com", "ntp7.aliyun.com",
+	}
+
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/plain")
+		io.WriteString(w, "test")
+	}))
+	defer ts.Close()
+
+	metadataURL = ts.URL
+	config.Datadog.Set("cloud_provider_metadata", []string{"alibaba"})
+	actualHosts := GetNTPHosts()
+
+	assert.Equal(t, expectedHosts, actualHosts)
+}

pkg/util/azure/azure.go

@@ -64,6 +64,16 @@ func GetClusterName() (string, error) {
 	return splitAll[len(splitAll)-2], nil
 }
 
+// GetNTPHosts returns the NTP hosts for Azure if it is detected as the cloud provider, otherwise an empty array.
+// Demo: https://docs.microsoft.com/en-us/azure/virtual-machines/linux/time-sync
+func GetNTPHosts() []string {
+	if IsRunningOn() {
+		return []string{"time.windows.com"}
+	}
+
+	return nil
+}
+
 func getResponseWithMaxLength(endpoint string, maxLength int) (string, error) {
 	result, err := getResponse(endpoint)
 	if err != nil {

pkg/util/azure/azure_test.go

@@ -12,6 +12,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	"github.com/DataDog/datadog-agent/pkg/config"
 )
 
 func TestGetHostname(t *testing.T) {
@@ -76,3 +78,19 @@ func TestGetClusterName(t *testing.T) {
 		})
 	}
 }
+
+func TestGetNTPHosts(t *testing.T) {
+	expectedHosts := []string{"time.windows.com"}
+
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/plain")
+		io.WriteString(w, "test")
+	}))
+	defer ts.Close()
+
+	metadataURL = ts.URL
+	config.Datadog.Set("cloud_provider_metadata", []string{"azure"})
+	actualHosts := GetNTPHosts()
+
+	assert.Equal(t, expectedHosts, actualHosts)
+}

pkg/util/cloudprovider.go

@@ -22,6 +22,11 @@ type cloudProviderDetector struct {
 	callback func() bool
 }
 
+type cloudProviderNTPDetector struct {
+	name     string
+	callback func() []string
+}
+
 // DetectCloudProvider detects the cloud provider where the agent is running in order:
 // * AWS ECS/Fargate
 // * AWS EC2
@@ -48,3 +53,24 @@ func DetectCloudProvider() {
 	}
 	log.Info("No cloud provider detected")
 }
+
+// GetCloudProviderNTPHosts detects the cloud provider where the agent is running in order and returns its NTP host name.
+func GetCloudProviderNTPHosts() []string {
+	detectors := []cloudProviderNTPDetector{
+		{name: ecscommon.CloudProviderName, callback: ecs.GetNTPHosts},
+		{name: ec2.CloudProviderName, callback: ec2.GetNTPHosts},
+		{name: gce.CloudProviderName, callback: gce.GetNTPHosts},
+		{name: azure.CloudProviderName, callback: azure.GetNTPHosts},
+		{name: alibaba.CloudProviderName, callback: alibaba.GetNTPHosts},
+		{name: tencent.CloudProviderName, callback: tencent.GetNTPHosts},
+	}
+
+	for _, cloudNTPDetector := range detectors {
+		if cloudNTPServer := cloudNTPDetector.callback(); cloudNTPServer != nil {
+			log.Debug("Using NTP servers from %s cloud provider", cloudNTPDetector.name)
+			return cloudNTPServer
+		}
+	}
+
+	return nil
+}

pkg/util/ec2/ec2.go

@@ -140,6 +140,16 @@ func GetNetworkID() (string, error) {
 	}
 }
 
+// GetNTPHosts returns the NTP hosts for EC2 if it is detected as the cloud provider, otherwise an empty array.
+// Docs: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/set-time.html#configure_ntp
+func GetNTPHosts() []string {
+	if IsRunningOn() {
+		return []string{"169.254.169.123"}
+	}
+
+	return nil
+}
+
 func getMetadataItemWithMaxLength(endpoint string, maxLength int) (string, error) {
 	result, err := getMetadataItem(endpoint)
 	if err != nil {

pkg/util/ec2/ec2_test.go

@@ -444,3 +444,19 @@ func TestMetedataRequestWithoutToken(t *testing.T) {
 	assert.Equal(t, "/local-ipv4", requestWithoutToken.RequestURI)
 	assert.Equal(t, http.MethodGet, requestWithoutToken.Method)
 }
+
+func TestGetNTPHosts(t *testing.T) {
+	expectedHosts := []string{"169.254.169.123"}
+
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/plain")
+		io.WriteString(w, "test")
+	}))
+	defer ts.Close()
+
+	metadataURL = ts.URL
+	config.Datadog.Set("cloud_provider_metadata", []string{"aws"})
+	actualHosts := GetNTPHosts()
+
+	assert.Equal(t, expectedHosts, actualHosts)
+}

pkg/util/ecs/detection.go

@@ -104,6 +104,16 @@ func HasFargateResourceTags() bool {
 	})
 }
 
+// GetNTPHosts returns the NTP hosts for ECS/Fargate if it is detected as the cloud provider, otherwise an empty array.
+// Docs: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/set-time.html#configure_ntp
+func GetNTPHosts() []string {
+	if IsRunningOn() {
+		return []string{"169.254.169.123"}
+	}
+
+	return nil
+}
+
 func queryCacheBool(cacheKey string, cacheMissEvalFunc func() (bool, time.Duration)) bool {
 	if !config.IsCloudProviderEnabled(common.CloudProviderName) {
 		return false

pkg/util/ecs/detection_nodocker.go

@@ -22,3 +22,8 @@ func IsFargateInstance() bool {
 func IsRunningOn() bool {
 	return false
 }
+
+// GetNTPHosts returns the NTP hosts for ECS/Fargate if it is detected as the cloud provider, otherwise an empty array.
+func GetNTPHosts() []string {
+	return nil
+}

pkg/util/gce/gce.go

@@ -120,6 +120,16 @@ func GetNetworkID() (string, error) {
 
 }
 
+// GetNTPHosts returns the NTP hosts for GCE if it is detected as the cloud provider, otherwise an empty array.
+// Docs: https://cloud.google.com/compute/docs/instances/managing-instances
+func GetNTPHosts() []string {
+	if IsRunningOn() {
+		return []string{"metadata.google.internal"}
+	}
+
+	return nil
+}
+
 func getResponseWithMaxLength(endpoint string, maxLength int) (string, error) {
 	result, err := getResponse(endpoint)
 	if err != nil {

pkg/util/gce/gce_test.go

@@ -13,6 +13,8 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/DataDog/datadog-agent/pkg/config"
 )
 
 func TestGetHostname(t *testing.T) {
@@ -172,3 +174,19 @@ func TestGetNetworkMultipleVPC(t *testing.T) {
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "more than one network interface")
 }
+
+func TestGetNTPHosts(t *testing.T) {
+	expectedHosts := []string{"metadata.google.internal"}
+
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/plain")
+		io.WriteString(w, "test")
+	}))
+	defer ts.Close()
+
+	metadataURL = ts.URL
+	config.Datadog.Set("cloud_provider_metadata", []string{"gcp"})
+	actualHosts := GetNTPHosts()
+
+	assert.Equal(t, expectedHosts, actualHosts)
+}

pkg/util/tencent/tencent.go

@@ -58,6 +58,16 @@ func HostnameProvider() (string, error) {
 	return GetInstanceID()
 }
 
+// GetNTPHosts returns the NTP hosts for Tencent if it is detected as the cloud provider, otherwise an empty array.
+// Demo: https://intl.cloud.tencent.com/document/product/213/32379
+func GetNTPHosts() []string {
+	if IsRunningOn() {
+		return []string{"ntpupdate.tencentyun.com"}
+	}
+
+	return nil
+}
+
 func getMetadataItemWithMaxLength(endpoint string, maxLength int) (string, error) {
 	result, err := getMetadataItem(endpoint)
 	if err != nil {

pkg/util/tencent/tencent_test.go

@@ -35,3 +35,19 @@ func TestGetInstanceID(t *testing.T) {
 	assert.Equal(t, expected, val)
 	assert.Equal(t, lastRequest.URL.Path, "/meta-data/instance-id")
 }
+
+func TestGetNTPHosts(t *testing.T) {
+	expectedHosts := []string{"ntpupdate.tencentyun.com"}
+
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/plain")
+		io.WriteString(w, "test")
+	}))
+	defer ts.Close()
+
+	metadataURL = ts.URL
+	config.Datadog.Set("cloud_provider_metadata", []string{"tencent"})
+	actualHosts := GetNTPHosts()
+
+	assert.Equal(t, expectedHosts, actualHosts)
+}

releasenotes/notes/use-cloud-provider-ntp-2995657d00d3007d.yaml

@@ -0,0 +1,4 @@
+enhancements:
+  - |
+    The NTP check now uses the cloud provider's recommended NTP servers by default, if the Agent
+    detects that it's running on said cloud provider.