Skip to content

Commit

Permalink
stop querying DNS to resolve NTP peers by default
Browse files Browse the repository at this point in the history 
Summary: `ntpcheck` now gets the NTP peer names via the chrony network protocol and not by querying DNS (unless you really want it to do that). This means no more flurries of DNS requests when it runs.

Reviewed By: abulimov

Differential Revision: D52925522

fbshipit-source-id: 9084c26474438c39345095f6c8cfc74dced47cb9
  • Loading branch information
@t3lurid3 @facebook-github-bot
t3lurid3 authored and facebook-github-bot committed Jan 23, 2024
1 parent 80c83be commit aff76dc
Show file tree
Hide file tree
Showing 8 changed files with 100 additions and 15 deletions.
15 changes: 14 additions & 1 deletion cmd/ntpcheck/checker/chrony.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,20 @@ func (n *ChronyCheck) Run() (*NTPCheckResult, error) {
return nil, errors.Errorf("Got wrong 'ntpdata' response %+v", packet)
}
}
peer, err := NewPeerFromChrony(sourceData, ntpData)
// get peer sourcename using a unix socket
var ntpSourceName *chrony.ReplyNTPSourceName
if sourceData.Mode != chrony.SourceModeRef {
ntpSourceNameReq := chrony.NewNTPSourceNamePacket(sourceData.IPAddr)
packet, err = n.Client.Communicate(ntpSourceNameReq)
if err != nil {
return nil, errors.Wrapf(err, "failed to get 'sourcename' response for source #%d", i)
}
ntpSourceName, ok = packet.(*chrony.ReplyNTPSourceName)
if !ok {
return nil, errors.Errorf("Got wrong 'sourcename' response %+v", packet)
}
}
peer, err := NewPeerFromChrony(sourceData, ntpData, ntpSourceName)
if err != nil {
return nil, errors.Wrapf(err, "failed to create Peer structure from response packet for peer=%s", sourceData.IPAddr)
}
Expand Down
44 changes: 44 additions & 0 deletions cmd/ntpcheck/checker/chrony_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,44 @@ var (
LocalAddr: net.ParseIP("192.168.0.1"),
},
}
ntpPeer0 = [256]uint8{
0x6e, 0x74, 0x70, 0x5f, 0x70, 0x65, 0x65, 0x72, 0x30, 0x30,
0x31, 0x2e, 0x73, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x66,
0x61, 0x63, 0x65, 0x62, 0x6f, 0x6f, 0x6b, 0x2e, 0x63, 0x6f,
0x6d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
}
replyNTPSourceName0 = &chrony.ReplyNTPSourceName{
NTPSourceName: chrony.NTPSourceName{
Name: ntpPeer0,
},
}
replyNTPSourceName1 = &chrony.ReplyNTPSourceName{
NTPSourceName: chrony.NTPSourceName{
Name: [256]uint8{},
},
}
)

func TestChronyCheckRun(t *testing.T) {
Expand All @@ -126,8 +164,10 @@ func TestChronyCheckRun(t *testing.T) {
replySources,
// first source
replySD0,
replyNTPSourceName0,
// second source
replySD1,
replyNTPSourceName1,
}

want := &NTPCheckResult{
Expand All @@ -148,6 +188,7 @@ func TestChronyCheckRun(t *testing.T) {
Selection: 6,
Condition: "sync",
SRCAdr: "192.168.0.2",
Hostname: "ntp_peer001.sample.facebook.com",
Stratum: 2,
Reach: 255,
PPoll: 10,
Expand Down Expand Up @@ -189,9 +230,11 @@ func TestChronyCheckRunUnix(t *testing.T) {
// first source
replySD0,
replyNTPData0,
replyNTPSourceName0,
// second source
replySD1,
replyNTPData1,
replyNTPSourceName1,
}

want := &NTPCheckResult{
Expand All @@ -215,6 +258,7 @@ func TestChronyCheckRunUnix(t *testing.T) {
Condition: "sync",
SRCAdr: "192.168.0.2",
DSTAdr: "192.168.0.1",
Hostname: "ntp_peer001.sample.facebook.com",
Stratum: 2,
Reach: 255,
PPoll: 10,
Expand Down
11 changes: 9 additions & 2 deletions cmd/ntpcheck/checker/peer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package checker

import (
"bytes"
"fmt"
"strconv"

Expand All @@ -38,6 +39,7 @@ type Peer struct {
// from variables
SRCAdr string
SRCPort int
Hostname string
DSTAdr string
DSTPort int
Leap int
Expand Down Expand Up @@ -171,8 +173,8 @@ var chronyToPeerSelection = map[chrony.SourceStateType]uint8{
chrony.SourceStateOutlier: control.SelOutlier,
}

// NewPeerFromChrony constructs Peer from two chrony packets
func NewPeerFromChrony(s *chrony.ReplySourceData, p *chrony.ReplyNTPData) (*Peer, error) {
// NewPeerFromChrony constructs Peer from three chrony packets
func NewPeerFromChrony(s *chrony.ReplySourceData, p *chrony.ReplyNTPData, n *chrony.ReplyNTPSourceName) (*Peer, error) {
if s == nil {
return nil, fmt.Errorf("no ReplySourceData to create Peer")
}
Expand Down Expand Up @@ -225,6 +227,11 @@ func NewPeerFromChrony(s *chrony.ReplySourceData, p *chrony.ReplyNTPData) (*Peer
peer.Delay = secToMS(p.PeerDelay)
peer.RootDisp = secToMS(p.RootDispersion)
}
if n != nil {
// this field is zero padded in chrony, so we need to trim it
peer.Hostname = string(bytes.TrimRight(n.Name[:], "\x00"))
}

// no need for sanity check as we are not parsing k=v pairs in case of chrony proto
return &peer, nil
}
7 changes: 6 additions & 1 deletion cmd/ntpcheck/checker/peer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,17 +110,20 @@ func TestNewPeerFromChrony(t *testing.T) {
ntpData.Poll = 10
ntpData.RefID = 123456
ntpData.RefTime = time.Unix(1587738257, 0)
ntpSourceName := &chrony.ReplyNTPSourceName{}
tests := []struct {
name string
s *chrony.ReplySourceData
p *chrony.ReplyNTPData
n *chrony.ReplyNTPSourceName
want *Peer
wantErr bool
}{
{
name: "no data",
s: nil,
p: nil,
n: nil,
want: nil,
wantErr: true,
},
Expand All @@ -147,6 +150,7 @@ func TestNewPeerFromChrony(t *testing.T) {
name: "full data",
s: sourceData,
p: ntpData,
n: ntpSourceName,
want: &Peer{
Stratum: 3,
Offset: -0,
Expand All @@ -162,13 +166,14 @@ func TestNewPeerFromChrony(t *testing.T) {
DSTAdr: "<nil>",
RefID: "0001E240",
RefTime: ntpData.RefTime.String(),
Hostname: "",
},
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := NewPeerFromChrony(tt.s, tt.p)
got, err := NewPeerFromChrony(tt.s, tt.p, tt.n)
if (err != nil) != tt.wantErr {
t.Errorf("NewPeerFromChrony() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down
21 changes: 16 additions & 5 deletions cmd/ntpcheck/checker/peerstats.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
)

// NewNTPPeerStats constructs NTPStats from NTPCheckResult
func NewNTPPeerStats(r *NTPCheckResult) (map[string]any, error) {
func NewNTPPeerStats(r *NTPCheckResult, noDNS bool) (map[string]any, error) {
if r.SysVars == nil {
return nil, errors.New("no system variables to output stats")
}
Expand All @@ -40,10 +40,7 @@ func NewNTPPeerStats(r *NTPCheckResult) (map[string]any, error) {
if ip == nil || ip.IsUnspecified() {
continue
}
hostnames, err := net.LookupAddr(peer.SRCAdr)
if err != nil || len(hostnames) == 0 {
hostnames = []string{peer.SRCAdr}
}
hostnames := peerName(peer, noDNS)
// Replace "." and ":" for "_"
hostname := strings.ReplaceAll(strings.ReplaceAll(strings.TrimSuffix(hostnames[0], "."), ".", "_"), ":", "_")

Expand All @@ -56,3 +53,17 @@ func NewNTPPeerStats(r *NTPCheckResult) (map[string]any, error) {

return result, nil
}

func peerName(p *Peer, noDNS bool) []string {
if noDNS {
if len(p.Hostname) != 0 {
return []string{p.Hostname}
}
return []string{p.SRCAdr}
}
hostnames, err := net.LookupAddr(p.SRCAdr)
if err != nil || len(hostnames) == 0 {
hostnames = []string{p.SRCAdr}
}
return hostnames
}
9 changes: 6 additions & 3 deletions cmd/ntpcheck/checker/peerstats_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ func TestNTPPeerStatsNoSysVars(t *testing.T) {
},
},
}
_, err := NewNTPPeerStats(r)
noDNS := false
_, err := NewNTPPeerStats(r, noDNS)
require.EqualError(t, err, "no system variables to output stats")
}

Expand All @@ -47,7 +48,8 @@ func TestNTPPeerStatsNoPeers(t *testing.T) {
SysVars: &s,
Peers: map[uint16]*Peer{},
}
peerStats, err := NewNTPPeerStats(r)
noDNS := true
peerStats, err := NewNTPPeerStats(r, noDNS)
require.NoError(t, err)
want := map[string]any{}
require.Equal(t, want, peerStats)
Expand Down Expand Up @@ -84,7 +86,8 @@ func TestNTPPeerStatsWithSysPeer(t *testing.T) {
},
},
}
peerStats, err := NewNTPPeerStats(r)
noDNS := true
peerStats, err := NewNTPPeerStats(r, noDNS)
require.NoError(t, err)
want := map[string]any{
"ntp.peers.192_168_0_2.delay": 2.01,
Expand Down
7 changes: 4 additions & 3 deletions cmd/ntpcheck/cmd/peerstats.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import (
"github.com/spf13/cobra"
)

func printPeerStats(r *checker.NTPCheckResult) error {
output, err := checker.NewNTPPeerStats(r)
func printPeerStats(r *checker.NTPCheckResult, noDNS bool) error {
output, err := checker.NewNTPPeerStats(r, noDNS)
if err != nil {
return err
}
Expand All @@ -41,6 +41,7 @@ func printPeerStats(r *checker.NTPCheckResult) error {
func init() {
RootCmd.AddCommand(peerstatsCmd)
peerstatsCmd.Flags().StringVarP(&server, "server", "S", "", "server to connect to")
peerstatsCmd.Flags().BoolVarP(&noDNS, "no-resolving", "n", true, "disable resolving of NTP peer addresses to hostnames")
}

var peerstatsCmd = &cobra.Command{
Expand All @@ -53,7 +54,7 @@ var peerstatsCmd = &cobra.Command{
if err != nil {
log.Fatal(err)
}
err = printPeerStats(result)
err = printPeerStats(result, noDNS)
if err != nil {
log.Fatal(err)
}
Expand Down
1 change: 1 addition & 0 deletions cmd/ntpcheck/cmd/root.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ var RootCmd = &cobra.Command{

var verbose bool
var server string
var noDNS bool

func init() {
RootCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "verbose output")
Expand Down

0 comments on commit aff76dc

Please sign in to comment.