From 76750019a1987fdfcd651c9163ee0e735aeabfff Mon Sep 17 00:00:00 2001 From: a-palchikov Date: Thu, 10 Jun 2021 10:44:37 +0200 Subject: [PATCH] Lint warnings #14 (#2521) * Address linter warnings for lib/checks * Update linter configuration * Remove redundant lint warning on non-linux platforms --- Makefile | 3 +- lib/checks/checks.go | 167 ++++++++++++++-------------- lib/checks/checks_test.go | 55 ++++----- lib/checks/disks.go | 8 +- lib/checks/pingpong.go | 12 +- lib/checks/remote.go | 42 +++---- lib/expand/phases/checks.go | 6 +- lib/system/caps.go | 1 - lib/update/cluster/checks/checks.go | 2 +- 9 files changed, 147 insertions(+), 149 deletions(-) diff --git a/Makefile b/Makefile index 42209cae8d..d83ba7f136 100644 --- a/Makefile +++ b/Makefile @@ -45,7 +45,8 @@ GOLINT_PACKAGES ?= \ ./lib/testutils/... \ ./lib/network/... \ ./lib/pack/... \ - ./lib/utils/... + ./lib/utils/... \ + ./lib/checks/... GOPATH ?= $(shell go env GOPATH) diff --git a/lib/checks/checks.go b/lib/checks/checks.go index 084aba6883..7614447783 100644 --- a/lib/checks/checks.go +++ b/lib/checks/checks.go @@ -49,16 +49,17 @@ import ( "github.com/sirupsen/logrus" ) -var log = logrus.WithField(trace.Component, "checks") - // New creates a new checker for the specified list of servers using given // set of server information payloads and the specified interface for // running remote commands. -func New(config Config) (*checker, error) { +func New(config Config) (*Checker, error) { if err := config.check(); err != nil { return nil, trace.Wrap(err) } - return &checker{Config: config}, nil + return &Checker{ + config: config, + log: logrus.WithField(trace.Component, "checks"), + }, nil } // Validate verifies the specified manifest against the host environment. @@ -171,7 +172,7 @@ func (r *LocalChecksResult) GetFailed() []*agentpb.Probe { // ValidateLocal runs checks on the local node and returns their outcome func ValidateLocal(ctx context.Context, req LocalChecksRequest) (*LocalChecksResult, error) { if ifTestsDisabled() { - log.Infof("Skipping local checks due to %v set.", constants.PreflightChecksOffEnvVar) + logrus.Infof("Skipping local checks due to %v set.", constants.PreflightChecksOffEnvVar) return &LocalChecksResult{}, nil } @@ -285,8 +286,8 @@ func DockerConfigFromSchemaValue(dockerSchema schema.Docker) (config storage.Doc } } -// Checker defines a preflight checker interface. -type Checker interface { +// Interface defines a preflight checker interface. +type Interface interface { // Run runs a full set of checks on the nodes configured in the checker. Run(ctx context.Context) error // CheckNode executes single-node checks (such as CPU/RAM requirements, @@ -299,9 +300,11 @@ type Checker interface { Check(ctx context.Context) []*agentpb.Probe } -type checker struct { - // Config is the checker configuration. - Config +// Checker is the runner for tests +type Checker struct { + // config is the checker configuration. + config Config + log logrus.FieldLogger } // Config represents the checker configuration. @@ -356,7 +359,7 @@ type Server struct { } // Run runs a full set of checks on the servers specified in r.servers -func (r *checker) Run(ctx context.Context) error { +func (r *Checker) Run(ctx context.Context) error { failed := r.Check(ctx) if len(failed) != 0 { return trace.BadParameter("The following checks failed:\n%v", @@ -366,15 +369,15 @@ func (r *checker) Run(ctx context.Context) error { } // Check executes checks on r.servers and returns a list of failed probes. -func (r *checker) Check(ctx context.Context) (failed []*agentpb.Probe) { +func (r *Checker) Check(ctx context.Context) (failed []*agentpb.Probe) { if ifTestsDisabled() { - log.Infof("Skipping checks due to %q set.", + r.log.Infof("Skipping checks due to %q set.", constants.PreflightChecksOffEnvVar) return nil } - serverC := make(chan Server, len(r.Servers)) - for _, server := range r.Servers { + serverC := make(chan Server, len(r.config.Servers)) + for _, server := range r.config.Servers { serverC <- server } @@ -408,52 +411,52 @@ func (r *checker) Check(ctx context.Context) (failed []*agentpb.Probe) { wg.Wait() // run checks that take all servers into account - failed = append(failed, r.CheckNodes(ctx, r.Servers)...) + failed = append(failed, r.CheckNodes(ctx, r.config.Servers)...) return failed } // CheckNode executes checks for the provided individual server. -func (r *checker) CheckNode(ctx context.Context, server Server) (failed []*agentpb.Probe) { +func (r *Checker) CheckNode(ctx context.Context, server Server) (failed []*agentpb.Probe) { if ifTestsDisabled() { - log.Infof("Skipping single-node checks due to %q set.", + r.log.Infof("Skipping single-node checks due to %q set.", constants.PreflightChecksOffEnvVar) return nil } - requirements := r.Requirements[server.Server.Role] + requirements := r.config.Requirements[server.Server.Role] validateCtx, cancel := context.WithTimeout(ctx, defaults.AgentValidationTimeout) defer cancel() - failed, err := r.Remote.Validate(validateCtx, server.AdvertiseIP, ValidateConfig{ - Manifest: r.Manifest, + failed, err := r.config.Remote.Validate(validateCtx, server.AdvertiseIP, ValidateConfig{ + Manifest: r.config.Manifest, Profile: server.Server.Role, Docker: requirements.Docker, }) if err != nil { - log.WithError(err).Warn("Failed to validate remote node.") + r.log.WithError(err).Warn("Failed to validate remote node.") failed = append(failed, newFailedProbe( fmt.Sprintf("Failed to validate node %v", server), err.Error())) } - err = checkServerProfile(server, requirements) + err = r.checkServerProfile(server, requirements) if err != nil { - log.WithError(err).Warn("Failed to validate profile requirements.") + r.log.WithError(err).Warn("Failed to validate profile requirements.") failed = append(failed, newFailedProbe( "Failed to validate profile requirements", err.Error())) } err = r.checkTempDir(ctx, server) if err != nil { - log.WithError(err).Warn("Failed to validate temporary directory.") + r.log.WithError(err).Warn("Failed to validate temporary directory.") failed = append(failed, newFailedProbe( "Failed to validate temporary directory", err.Error())) } - if server.IsMaster() && r.TestEtcdDisk { + if server.IsMaster() && r.config.TestEtcdDisk { probes, err := r.checkEtcdDisk(ctx, server) if err != nil { - log.WithError(err).Warn("Failed to validate etcd disk requirements.") + r.log.WithError(err).Warn("Failed to validate etcd disk requirements.") } // The checker will only return probes if etcd disk test succeeded and // some iops/latency requirements are not met. @@ -462,7 +465,7 @@ func (r *checker) CheckNode(ctx context.Context, server Server) (failed []*agent err = r.checkDisks(ctx, server) if err != nil { - log.WithError(err).Warn("Failed to validate disk requirements.") + r.log.WithError(err).Warn("Failed to validate disk requirements.") failed = append(failed, newFailedProbe( "Failed to validate disk requirements", err.Error())) } @@ -471,33 +474,33 @@ func (r *checker) CheckNode(ctx context.Context, server Server) (failed []*agent } // CheckNodes executes checks that take all provided servers into account. -func (r *checker) CheckNodes(ctx context.Context, servers []Server) (failed []*agentpb.Probe) { +func (r *Checker) CheckNodes(ctx context.Context, servers []Server) (failed []*agentpb.Probe) { if ifTestsDisabled() { - log.Infof("Skipping multi-node checks due to %q set.", + r.log.Infof("Skipping multi-node checks due to %q set.", constants.PreflightChecksOffEnvVar) return nil } - err := checkTime(time.Now().UTC(), servers) + err := r.checkTime(time.Now().UTC(), servers) if err != nil { - log.WithError(err).Warn("Failed to validate time drift requirements.") + r.log.WithError(err).Warn("Failed to validate time drift requirements.") failed = append(failed, newFailedProbe( "Failed to validate time drift requirement", err.Error())) } - if r.TestPorts { + if r.config.TestPorts { err = r.checkPorts(ctx, servers) if err != nil { - log.WithError(err).Warn("Failed to validate port requirements.") + r.log.WithError(err).Warn("Failed to validate port requirements.") failed = append(failed, newFailedProbe( "Failed to validate port requirements", err.Error())) } } - if r.TestBandwidth { + if r.config.TestBandwidth { err = r.checkBandwidth(ctx, servers) if err != nil { - log.WithError(err).Warn("Failed to validate bandwidth requirements.") + r.log.WithError(err).Warn("Failed to validate bandwidth requirements.") failed = append(failed, newFailedProbe( "Failed to validate network bandwidth requirements", err.Error())) } @@ -507,8 +510,8 @@ func (r *checker) CheckNodes(ctx context.Context, servers []Server) (failed []*a } // checkDisks verifies that disk performance satisfies the profile requirements. -func (r *checker) checkDisks(ctx context.Context, server Server) error { - requirements := r.Requirements[server.Server.Role] +func (r *Checker) checkDisks(ctx context.Context, server Server) error { + requirements := r.config.Requirements[server.Server.Role] targets, err := r.collectTargets(ctx, server, requirements) if err != nil { return trace.Wrap(err) @@ -534,7 +537,7 @@ func (r *checker) checkDisks(ctx context.Context, server Server) error { target.rate.String()) } - log.Infof("Server %q passed disk I/O check on %v: %v/s.", + r.log.Infof("Server %q passed disk I/O check on %v: %v/s.", server.ServerInfo.GetHostname(), target, humanize.Bytes(maxBps)) } @@ -542,25 +545,25 @@ func (r *checker) checkDisks(ctx context.Context, server Server) error { } // checkServerDisk runs a simple disk performance test and returns the write speed in bytes per second -func (r *checker) checkServerDisk(ctx context.Context, server storage.Server, target string) (uint64, error) { +func (r *Checker) checkServerDisk(ctx context.Context, server storage.Server, target string) (uint64, error) { var out bytes.Buffer // remove the testfile after the test defer func() { // testfile was created only on real filesystem if !strings.HasPrefix(target, "/dev") { - err := r.Remote.Exec(ctx, server.AdvertiseIP, []string{"rm", target}, nil, &out) + err := r.config.Remote.Exec(ctx, server.AdvertiseIP, []string{"rm", target}, nil, &out) if err != nil { - log.WithField("output", out.String()).Warn("Failed to remove test file.") + r.log.WithField("output", out.String()).Warn("Failed to remove test file.") } } }() - err := r.Remote.Exec(ctx, server.AdvertiseIP, []string{ + err := r.config.Remote.Exec(ctx, server.AdvertiseIP, []string{ "dd", "if=/dev/zero", fmt.Sprintf("of=%v", target), "bs=100K", "count=1024", "conv=fdatasync"}, &out, &out) if err != nil { - log.WithFields(logrus.Fields{ + r.log.WithFields(logrus.Fields{ "server-ip": server.AdvertiseIP, "target": target, "output": out.String(), @@ -577,13 +580,13 @@ func (r *checker) checkServerDisk(ctx context.Context, server storage.Server, ta } // checkTempDir makes sure agents can create temporary files on servers -func (r *checker) checkTempDir(ctx context.Context, server Server) error { +func (r *Checker) checkTempDir(ctx context.Context, server Server) error { filename := filepath.Join(server.TempDir, fmt.Sprintf("tmpcheck.%v", uuid.New())) var out bytes.Buffer - err := r.Remote.Exec(ctx, server.AdvertiseIP, []string{"touch", filename}, nil, &out) + err := r.config.Remote.Exec(ctx, server.AdvertiseIP, []string{"touch", filename}, nil, &out) if err != nil { - log.WithFields(logrus.Fields{ + r.log.WithFields(logrus.Fields{ "filename": filename, "server-ip": server.AdvertiseIP, "hostname": server.ServerInfo.GetHostname(), @@ -592,40 +595,36 @@ func (r *checker) checkTempDir(ctx context.Context, server Server) error { filepath.Join(server.TempDir, filename), server.ServerInfo.GetHostname(), out.String()) } - err = r.Remote.Exec(ctx, server.AdvertiseIP, []string{"rm", filename}, nil, &out) + err = r.config.Remote.Exec(ctx, server.AdvertiseIP, []string{"rm", filename}, nil, &out) if err != nil { - log.WithFields(logrus.Fields{ + r.log.WithFields(logrus.Fields{ "path": filename, "server-ip": server.AdvertiseIP, "output": out.String(), }).Warn("Failed to delete.") } - log.Infof("Server %q passed temp directory check: %v.", + r.log.Infof("Server %q passed temp directory check: %v.", server.ServerInfo.GetHostname(), server.TempDir) return nil } // checkPorts makes sure ports specified in profile are unoccupied and reachable -func (r *checker) checkPorts(ctx context.Context, servers []Server) error { - req, err := constructPingPongRequest(servers, r.Requirements) - if err != nil { - return trace.Wrap(err) - } - - log.Infof("Ping pong request: %v.", req) +func (r *Checker) checkPorts(ctx context.Context, servers []Server) error { + req := constructPingPongRequest(servers, r.config.Requirements) + r.log.Infof("Ping pong request: %v.", req) if len(req) == 0 { - log.Info("Empty ping pong request.") + r.log.Info("Empty ping pong request.") return nil } - resp, err := r.Remote.CheckPorts(ctx, req) + resp, err := r.config.Remote.CheckPorts(ctx, req) if err != nil { return trace.Wrap(err) } - log.Infof("Ping pong response: %v.", resp) + r.log.Infof("Ping pong response: %v.", resp) if len(resp.Failures()) != 0 { return trace.BadParameter(strings.Join(resp.Failures(), ", ")) @@ -636,24 +635,20 @@ func (r *checker) checkPorts(ctx context.Context, servers []Server) error { // checkBandwidth measures network bandwidth between servers and makes sure it satisfies // the profile -func (r *checker) checkBandwidth(ctx context.Context, servers []Server) error { +func (r *Checker) checkBandwidth(ctx context.Context, servers []Server) error { if len(servers) < 2 { return nil } - req, err := constructBandwidthRequest(servers) - if err != nil { - return trace.Wrap(err) - } - - log.Infof("Bandwidth test request: %v.", req) + req := constructBandwidthRequest(servers) + r.log.Infof("Bandwidth test request: %v.", req) - resp, err := r.Remote.CheckBandwidth(ctx, req) + resp, err := r.config.Remote.CheckBandwidth(ctx, req) if err != nil { return trace.Wrap(err) } - log.Infof("Bandwidth test response: %v.", resp) + r.log.Infof("Bandwidth test response: %v.", resp) if len(resp.Failures()) != 0 { return trace.BadParameter("%v", strings.Join(resp.Failures(), ", ")) @@ -666,7 +661,7 @@ func (r *checker) checkBandwidth(ctx context.Context, servers []Server) error { return trace.Wrap(err) } - requirements := r.Requirements[server.Server.Role] + requirements := r.config.Requirements[server.Server.Role] transferRate := requirements.Network.MinTransferRate if result.BandwidthResult < transferRate.BytesPerSecond() { return trace.BadParameter( @@ -676,7 +671,7 @@ func (r *checker) checkBandwidth(ctx context.Context, servers []Server) error { transferRate.String()) } - log.Infof("Server %q network bandwidth: %v/s.", + r.log.Infof("Server %q network bandwidth: %v/s.", server.ServerInfo.GetHostname(), humanize.Bytes(result.BandwidthResult)) } @@ -685,7 +680,7 @@ func (r *checker) checkBandwidth(ctx context.Context, servers []Server) error { // collectTargets returns a list of targets (devices or existing filesystems) // for the disk performance test -func (r *checker) collectTargets(ctx context.Context, server Server, requirements Requirements) ([]diskCheckTarget, error) { +func (r *Checker) collectTargets(ctx context.Context, server Server, requirements Requirements) ([]diskCheckTarget, error) { var targets []diskCheckTarget // Explicit system state directory disk performance target @@ -694,7 +689,7 @@ func (r *checker) collectTargets(ctx context.Context, server Server, requirement rate: defaultTransferRate, }) - remote := &serverRemote{server, r.Remote} + remote := &serverRemote{server, r.config.Remote} // check if there's a system device specified if path := getDevicePath(server.SystemState.Device.Name, storage.DeviceName(server.SystemDevice)); path != "" { @@ -762,16 +757,16 @@ func (r diskCheckTarget) String() string { // checkServerProfile checks information for a single server collected by agent // against its profile -func checkServerProfile(server Server, requirements Requirements) error { +func (r *Checker) checkServerProfile(server Server, requirements Requirements) error { if requirements.CPU != nil { - err := checkCPU(server.ServerInfo, *requirements.CPU) + err := r.checkCPU(server.ServerInfo, *requirements.CPU) if err != nil { return trace.Wrap(err) } } if requirements.RAM != nil { - err := checkRAM(server.ServerInfo, *requirements.RAM) + err := r.checkRAM(server.ServerInfo, *requirements.RAM) if err != nil { return trace.Wrap(err) } @@ -781,7 +776,7 @@ func checkServerProfile(server Server, requirements Requirements) error { } // checkCPU makes sure server's CPU count satisfies the profile -func checkCPU(info ServerInfo, cpu schema.CPU) error { +func (r *Checker) checkCPU(info ServerInfo, cpu schema.CPU) error { if info.GetNumCPU() < uint(cpu.Min) { return trace.BadParameter("server %q has %v CPUs which is less than required minimum of %v", info.GetHostname(), info.GetNumCPU(), cpu.Min) @@ -792,12 +787,12 @@ func checkCPU(info ServerInfo, cpu schema.CPU) error { info.GetHostname(), info.GetNumCPU(), cpu.Max) } - log.Infof("Server %q passed CPU check: %v.", info.GetHostname(), info.GetNumCPU()) + r.log.Infof("Server %q passed CPU check: %v.", info.GetHostname(), info.GetNumCPU()) return nil } // checkRAM makes sure server's RAM amount satisfies the profile -func checkRAM(info ServerInfo, ram schema.RAM) error { +func (r *Checker) checkRAM(info ServerInfo, ram schema.RAM) error { if info.GetMemory().Total < ram.Min.Bytes() { return trace.BadParameter("server %q has %v RAM which is less than required minimum of %v", info.GetHostname(), humanize.Bytes(info.GetMemory().Total), ram.Min.String()) @@ -808,13 +803,13 @@ func checkRAM(info ServerInfo, ram schema.RAM) error { info.GetHostname(), humanize.Bytes(info.GetMemory().Total), ram.Max.String()) } - log.Infof("Server %q passed RAM check: %v.", info.GetHostname(), + r.log.Infof("Server %q passed RAM check: %v.", info.GetHostname(), humanize.Bytes(info.GetMemory().Total)) return nil } // checkTime checks if time it out of sync between servers -func checkTime(currentTime time.Time, servers []Server) error { +func (r *Checker) checkTime(currentTime time.Time, servers []Server) error { // server can not be out of sync with itself if len(servers) < 2 { return nil @@ -839,7 +834,7 @@ func checkTime(currentTime time.Time, servers []Server) error { } } - log.Infof("Servers %v passed time drift check.", servers) + r.log.Infof("Servers %v passed time drift check.", servers) return nil } @@ -916,7 +911,7 @@ func portRange(rs []schema.PortRange) (result []monitoring.PortRange) { } // constructPingPongRequest constructs a regular ping-pong game request -func constructPingPongRequest(servers []Server, requirements map[string]Requirements) (PingPongGame, error) { +func constructPingPongRequest(servers []Server, requirements map[string]Requirements) PingPongGame { game := make(PingPongGame, len(servers)) var listenServers []validationpb.Addr for _, server := range servers { @@ -954,11 +949,11 @@ func constructPingPongRequest(servers []Server, requirements map[string]Requirem game[ip] = req } - return game, nil + return game } // constructBandwidthRequest constructs a ping-pong game request for a bandwidth test -func constructBandwidthRequest(servers []Server) (PingPongGame, error) { +func constructBandwidthRequest(servers []Server) PingPongGame { // use up to defaults.BandwidthTestMaxServers servers for the test servers = servers[:utils.Min(len(servers), defaults.BandwidthTestMaxServers)] @@ -983,7 +978,7 @@ func constructBandwidthRequest(servers []Server) (PingPongGame, error) { } } - return game, nil + return game } func findServer(servers []Server, addr string) (*Server, error) { diff --git a/lib/checks/checks_test.go b/lib/checks/checks_test.go index 027a3df891..ebbf442b71 100644 --- a/lib/checks/checks_test.go +++ b/lib/checks/checks_test.go @@ -31,48 +31,51 @@ import ( func TestChecks(t *testing.T) { check.TestingT(t) } type ChecksSuite struct { - info ServerInfo + info ServerInfo + checker *Checker } var _ = check.Suite(&ChecksSuite{}) func (s *ChecksSuite) SetUpSuite(c *check.C) { - sysinfo := storage.NewSystemInfo(storage.SystemSpecV2{ - Hostname: "foo", - Filesystems: []storage.Filesystem{ - { - DirName: "/foo/bar", - Type: "tmpfs", + s.info = ServerInfo{ + System: storage.NewSystemInfo(storage.SystemSpecV2{ + Hostname: "foo", + Filesystems: []storage.Filesystem{ + { + DirName: "/foo/bar", + Type: "tmpfs", + }, }, - }, - FilesystemStats: map[string]storage.FilesystemUsage{ - "/foo/bar": { - TotalKB: 2, - FreeKB: 1, + FilesystemStats: map[string]storage.FilesystemUsage{ + "/foo/bar": { + TotalKB: 2, + FreeKB: 1, + }, }, - }, - Memory: storage.Memory{Total: 1000, Free: 500, ActualFree: 640}, - NumCPU: 4, - }) - s.info = ServerInfo{ - System: sysinfo, + Memory: storage.Memory{Total: 1000, Free: 500, ActualFree: 640}, + NumCPU: 4, + }), } + var err error + s.checker, err = New(Config{}) + c.Assert(err, check.IsNil) } func (s *ChecksSuite) TestCheckCPU(c *check.C) { enoughCPU := schema.CPU{Min: 4} - c.Assert(checkCPU(s.info, enoughCPU), check.IsNil) + c.Assert(s.checker.checkCPU(s.info, enoughCPU), check.IsNil) notEnoughCPU := schema.CPU{Min: 5} - c.Assert(checkCPU(s.info, notEnoughCPU), check.NotNil) + c.Assert(s.checker.checkCPU(s.info, notEnoughCPU), check.NotNil) } func (s *ChecksSuite) TestCheckRAM(c *check.C) { enoughRAM := schema.RAM{Min: 800} - c.Assert(checkRAM(s.info, enoughRAM), check.IsNil) + c.Assert(s.checker.checkRAM(s.info, enoughRAM), check.IsNil) notEnoughRAM := schema.RAM{Min: 1100} - c.Assert(checkRAM(s.info, notEnoughRAM), check.NotNil) + c.Assert(s.checker.checkRAM(s.info, notEnoughRAM), check.NotNil) } func (s *ChecksSuite) TestTime(c *check.C) { @@ -87,12 +90,12 @@ func (s *ChecksSuite) TestTime(c *check.C) { }) now := time.Date(2016, 12, 1, 2, 3, 40, 5000, time.UTC) - c.Assert(checkTime(now, nil), check.IsNil) + c.Assert(s.checker.checkTime(now, nil), check.IsNil) // anchor time is the time on the first server anchorTime := time.Date(2016, 12, 1, 2, 3, 4, 5000, time.UTC) // we have received the first info 10 seconds ago localTime := now.Add(-10 * time.Second) - err := checkTime(now, []Server{ + err := s.checker.checkTime(now, []Server{ {ServerInfo: ServerInfo{ServerTime: anchorTime, LocalTime: localTime}}}, ) c.Assert(err, check.IsNil) @@ -100,7 +103,7 @@ func (s *ChecksSuite) TestTime(c *check.C) { // we have received the second info 9 seconds ago server2LocalTime := now.Add(-9 * time.Second) server2Time := anchorTime.Add(defaults.MaxOutOfSyncTimeDelta / 2).Add(time.Second) - err = checkTime(now, []Server{ + err = s.checker.checkTime(now, []Server{ {ServerInfo: ServerInfo{ServerTime: anchorTime, LocalTime: localTime}}, {ServerInfo: ServerInfo{ServerTime: server2Time, LocalTime: server2LocalTime}}, }) @@ -109,7 +112,7 @@ func (s *ChecksSuite) TestTime(c *check.C) { // we have received the third info 8 seconds ago server3LocalTime := now.Add(-8 * time.Second) server3Time := anchorTime.Add(defaults.MaxOutOfSyncTimeDelta + time.Nanosecond).Add(2 * time.Second) - err = checkTime(now, []Server{ + err = s.checker.checkTime(now, []Server{ {ServerInfo: ServerInfo{System: server, ServerTime: anchorTime, LocalTime: localTime}}, {ServerInfo: ServerInfo{System: server2, ServerTime: server2Time, LocalTime: server2LocalTime}}, {ServerInfo: ServerInfo{System: server3, ServerTime: server3Time, LocalTime: server3LocalTime}}, diff --git a/lib/checks/disks.go b/lib/checks/disks.go index 09b59fd5b2..e52e8e6449 100644 --- a/lib/checks/disks.go +++ b/lib/checks/disks.go @@ -32,14 +32,14 @@ import ( // checkEtcdDisk makes sure that the disk used for etcd wal satisfies // performance requirements. -func (r *checker) checkEtcdDisk(ctx context.Context, server Server) ([]*agentpb.Probe, error) { +func (r *Checker) checkEtcdDisk(ctx context.Context, server Server) ([]*agentpb.Probe, error) { // Test file should reside where etcd data will be. testPath := state.InEtcdDir(server.ServerInfo.StateDir, testFile) - res, err := r.Remote.CheckDisks(ctx, server.AdvertiseIP, fioEtcdJob(testPath)) + res, err := r.config.Remote.CheckDisks(ctx, server.AdvertiseIP, fioEtcdJob(testPath)) if err != nil { return nil, trace.Wrap(err) } - log.Debugf("Server %v disk test results: %s.", server.Hostname, res.String()) + r.log.Debugf("Server %v disk test results: %s.", server.Hostname, res.String()) if len(res.Jobs) != 1 { return nil, trace.BadParameter("expected 1 job result: %v", res) } @@ -49,7 +49,7 @@ func (r *checker) checkEtcdDisk(ctx context.Context, server Server) ([]*agentpb. if len(probes) > 0 { return probes, nil } - log.Infof("Server %v passed etcd disk check, has %v sequential write iops and %vms fsync latency.", + r.log.Infof("Server %v passed etcd disk check, has %v sequential write iops and %vms fsync latency.", server.Hostname, iops, latency) return nil, nil } diff --git a/lib/checks/pingpong.go b/lib/checks/pingpong.go index 0eae2939c0..014549dfd2 100644 --- a/lib/checks/pingpong.go +++ b/lib/checks/pingpong.go @@ -46,7 +46,7 @@ const ( ModeBandwidth = "bandwidth" ) -// Checks makes sure the request is correct +// Check makes sure the request is correct func (r PingPongRequest) Check() error { if !utils.StringInSlice([]string{ModePingPong, ModeBandwidth}, r.Mode) { return trace.BadParameter("unsupported mode %q", r.Mode) @@ -89,8 +89,8 @@ func (r PingPongRequest) PortsProto() *pb.CheckPortsRequest { func (r PingPongRequest) BandwidthProto() *pb.CheckBandwidthRequest { listen := r.Listen[0] var pings []*pb.Addr - for _, ping := range r.Ping { - pings = append(pings, &ping) + for i := range r.Ping { + pings = append(pings, &r.Ping[i]) } return &pb.CheckBandwidthRequest{ Listen: &listen, @@ -127,7 +127,7 @@ func ResultFromBandwidthProto(resp *pb.CheckBandwidthResponse, err error) *PingP // PingPongResult is a result of a ping-pong game type PingPongResult struct { - // Code means that the whole operation has succeded + // Code means that the whole operation has succeeded // 0 does not mean that all results are success, it just // means that experiment went uninterrupted, and there // still can be some failures in results @@ -147,12 +147,12 @@ func (r PingPongResult) FailureCount() int { var count int for _, l := range r.ListenResults { if l.Code != 0 { - count += 1 + count++ } } for _, p := range r.PingResults { if p.Code != 0 { - count += 1 + count++ } } return count diff --git a/lib/checks/remote.go b/lib/checks/remote.go index fe19a27f04..9717fa2605 100644 --- a/lib/checks/remote.go +++ b/lib/checks/remote.go @@ -58,33 +58,33 @@ type ValidateConfig struct { } // NewRemote creates a remote node validator from the provided agents repository. -func NewRemote(agents rpc.AgentRepository) *remote { - return &remote{ - AgentRepository: agents, - FieldLogger: logrus.WithField(trace.Component, +func NewRemote(agents rpc.AgentRepository) *Remoter { + return &Remoter{ + agents: agents, + log: logrus.WithField(trace.Component, "checks:remote"), } } -// remote allows to execute remote commands and validate remote nodes. +// Remoter allows to execute remote commands and validate remote nodes. // // Implements Remote. -type remote struct { - // AgentRepository provides access to running RPC agents. - rpc.AgentRepository - // FieldLogger is used for logging. - logrus.FieldLogger +type Remoter struct { + // agents provides access to running RPC agents. + agents rpc.AgentRepository + // log is used for logging. + log logrus.FieldLogger } // Exec executes the command remotely on the specified node. // // The command's output is written to the provided writer. -func (r *remote) Exec(ctx context.Context, addr string, command []string, stdout, stderr io.Writer) error { - clt, err := r.GetClient(ctx, addr) +func (r *Remoter) Exec(ctx context.Context, addr string, command []string, stdout, stderr io.Writer) error { + clt, err := r.agents.GetClient(ctx, addr) if err != nil { return trace.Wrap(err) } - err = clt.Command(ctx, r.FieldLogger, stdout, stderr, command...) + err = clt.Command(ctx, r.log, stdout, stderr, command...) if err != nil { return trace.Wrap(err) } @@ -92,8 +92,8 @@ func (r *remote) Exec(ctx context.Context, addr string, command []string, stdout } // CheckPorts executes network test to test port availability. -func (r *remote) CheckPorts(ctx context.Context, req PingPongGame) (PingPongGameResults, error) { - resp, err := pingPong(ctx, r, req, ports) +func (r *Remoter) CheckPorts(ctx context.Context, req PingPongGame) (PingPongGameResults, error) { + resp, err := pingPong(ctx, r.agents, req, ports) if err != nil { return nil, trace.Wrap(err) } @@ -101,8 +101,8 @@ func (r *remote) CheckPorts(ctx context.Context, req PingPongGame) (PingPongGame } // CheckBandwidth executes network bandwidth test. -func (r *remote) CheckBandwidth(ctx context.Context, req PingPongGame) (PingPongGameResults, error) { - resp, err := pingPong(ctx, r, req, bandwidth) +func (r *Remoter) CheckBandwidth(ctx context.Context, req PingPongGame) (PingPongGameResults, error) { + resp, err := pingPong(ctx, r.agents, req, bandwidth) if err != nil { return nil, trace.Wrap(err) } @@ -110,8 +110,8 @@ func (r *remote) CheckBandwidth(ctx context.Context, req PingPongGame) (PingPong } // CheckDisks executes disk performance test. -func (r *remote) CheckDisks(ctx context.Context, addr string, req *proto.CheckDisksRequest) (*proto.CheckDisksResponse, error) { - clt, err := r.GetClient(ctx, addr) +func (r *Remoter) CheckDisks(ctx context.Context, addr string, req *proto.CheckDisksRequest) (*proto.CheckDisksResponse, error) { + clt, err := r.agents.GetClient(ctx, addr) if err != nil { return nil, trace.Wrap(err) } @@ -125,8 +125,8 @@ func (r *remote) CheckDisks(ctx context.Context, addr string, req *proto.CheckDi // Validate performs local checks on the specified node. // // Returns a list of failed test results. -func (r *remote) Validate(ctx context.Context, addr string, config ValidateConfig) ([]*agentpb.Probe, error) { - clt, err := r.GetClient(ctx, addr) +func (r *Remoter) Validate(ctx context.Context, addr string, config ValidateConfig) ([]*agentpb.Probe, error) { + clt, err := r.agents.GetClient(ctx, addr) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/expand/phases/checks.go b/lib/expand/phases/checks.go index 2d3dab1156..b50647ac5b 100644 --- a/lib/expand/phases/checks.go +++ b/lib/expand/phases/checks.go @@ -73,7 +73,7 @@ type checksExecutor struct { // Execute executes preflight checks on the joining node. func (p *checksExecutor) Execute(ctx context.Context) (err error) { - var checker checks.Checker + var checker checks.Interface if p.Phase.Data.Server.IsMaster() { checker, err = p.getMasterChecker(ctx) } else { @@ -109,7 +109,7 @@ func (p *checksExecutor) Execute(ctx context.Context) (err error) { // In addition to the local node checks, it also makes sure that there's no // time drift between this and other master which is important since it's // going to run a full etcd member. -func (p *checksExecutor) getMasterChecker(ctx context.Context) (checks.Checker, error) { +func (p *checksExecutor) getMasterChecker(ctx context.Context) (checks.Interface, error) { master, err := checks.GetServer(ctx, p.Runner, *p.Phase.Data.Master) if err != nil { return nil, trace.Wrap(err) @@ -130,7 +130,7 @@ func (p *checksExecutor) getMasterChecker(ctx context.Context) (checks.Checker, } // getNodeChecker returns a checker that performs checks when adding a regular node. -func (p *checksExecutor) getNodeChecker(ctx context.Context) (checks.Checker, error) { +func (p *checksExecutor) getNodeChecker(ctx context.Context) (checks.Interface, error) { node, err := checks.GetServer(ctx, p.Runner, *p.Phase.Data.Server) if err != nil { return nil, trace.Wrap(err) diff --git a/lib/system/caps.go b/lib/system/caps.go index cb82005d66..3d7e81c6ef 100644 --- a/lib/system/caps.go +++ b/lib/system/caps.go @@ -22,7 +22,6 @@ import "github.com/gravitational/trace" // DropCapabilitiesForJournalExport drops capabilities except those required // to export a systemd journal -//nolint:staticcheck func DropCapabilitiesForJournalExport() error { return trace.NotImplemented("API is not supported") } diff --git a/lib/update/cluster/checks/checks.go b/lib/update/cluster/checks/checks.go index 07fd13468c..cf7d8cbd91 100644 --- a/lib/update/cluster/checks/checks.go +++ b/lib/update/cluster/checks/checks.go @@ -45,7 +45,7 @@ type CheckerConfig struct { // NewChecker creates a checker for validating requirements of the // upgrade operation. -func NewChecker(ctx context.Context, config CheckerConfig) (checks.Checker, error) { +func NewChecker(ctx context.Context, config CheckerConfig) (checks.Interface, error) { cluster, err := config.ClusterOperator.GetLocalSite(ctx) if err != nil { return nil, trace.Wrap(err)