From 78179f5e0e2f7717c7763ca7ac6b70ba8e235005 Mon Sep 17 00:00:00 2001 From: Curtis Robert Date: Fri, 5 Apr 2024 09:50:22 -0700 Subject: [PATCH] [chore][receiver/sshcheck] Enable goleak check (#32181) This enables `goleak` checks for the sshcheck receiver to help ensure no goroutines are being leaked. This is a test only change. The test package's SSH Server needed to be modified to properly shutdown. **Link to tracking Issue:** #30438 --- receiver/sshcheckreceiver/package_test.go | 14 ++++++ receiver/sshcheckreceiver/scraper_test.go | 56 ++++++++++++++++++----- 2 files changed, 59 insertions(+), 11 deletions(-) create mode 100644 receiver/sshcheckreceiver/package_test.go diff --git a/receiver/sshcheckreceiver/package_test.go b/receiver/sshcheckreceiver/package_test.go new file mode 100644 index 000000000000..69b409efc3e4 --- /dev/null +++ b/receiver/sshcheckreceiver/package_test.go @@ -0,0 +1,14 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package sshcheckreceiver + +import ( + "testing" + + "go.uber.org/goleak" +) + +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} diff --git a/receiver/sshcheckreceiver/scraper_test.go b/receiver/sshcheckreceiver/scraper_test.go index c0064c9684e6..a9e736abea69 100644 --- a/receiver/sshcheckreceiver/scraper_test.go +++ b/receiver/sshcheckreceiver/scraper_test.go @@ -24,7 +24,24 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest/pmetrictest" ) -func setupSSHServer(t *testing.T) string { +type sshServer struct { + listener net.Listener + done chan struct{} +} + +func newSSHServer(network, endpoint string) (*sshServer, error) { + listener, err := net.Listen(network, endpoint) + if err != nil { + return nil, err + } + + return &sshServer{ + listener: listener, + done: make(chan struct{}), + }, nil +} + +func (s *sshServer) runSSHServer(t *testing.T) string { config := &ssh.ServerConfig{ NoClientAuth: true, PasswordCallback: func(c ssh.ConnMetadata, pass []byte) (*ssh.Permissions, error) { @@ -43,14 +60,16 @@ func setupSSHServer(t *testing.T) string { config.AddHostKey(private) - listener, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - go func() { for { - conn, err := listener.Accept() + conn, err := s.listener.Accept() if err != nil { - break + select { + case <-s.done: + return + default: + require.NoError(t, err) + } } _, chans, reqs, err := ssh.NewServerConn(conn, config) if err != nil { @@ -62,7 +81,12 @@ func setupSSHServer(t *testing.T) string { } }() - return listener.Addr().String() + return s.listener.Addr().String() +} + +func (s *sshServer) shutdown() { + close(s.done) + s.listener.Close() } func handleChannels(chans <-chan ssh.NewChannel) { @@ -116,8 +140,12 @@ func TestScraper(t *testing.T) { if !supportedOS() { t.Skip("Skip tests if not running on one of: [linux, darwin, freebsd, openbsd]") } - endpoint := setupSSHServer(t) + + s, err := newSSHServer("tcp", "127.0.0.1:0") + require.NoError(t, err) + endpoint := s.runSSHServer(t) require.NotEmpty(t, endpoint) + defer s.shutdown() testCases := []struct { name string @@ -187,8 +215,11 @@ func TestScraperPropagatesResourceAttributes(t *testing.T) { if !supportedOS() { t.Skip("Skip tests if not running on one of: [linux, darwin, freebsd, openbsd]") } - endpoint := setupSSHServer(t) + s, err := newSSHServer("tcp", "127.0.0.1:0") + require.NoError(t, err) + endpoint := s.runSSHServer(t) require.NotEmpty(t, endpoint) + defer s.shutdown() f := NewFactory() cfg := f.CreateDefaultConfig().(*Config) @@ -223,8 +254,11 @@ func TestScraperDoesNotErrForSSHErr(t *testing.T) { if !supportedOS() { t.Skip("Skip tests if not running on one of: [linux, darwin, freebsd, openbsd]") } - endpoint := setupSSHServer(t) + s, err := newSSHServer("tcp", "127.0.0.1:0") + require.NoError(t, err) + endpoint := s.runSSHServer(t) require.NotEmpty(t, endpoint) + defer s.shutdown() f := NewFactory() cfg := f.CreateDefaultConfig().(*Config) @@ -239,7 +273,7 @@ func TestScraperDoesNotErrForSSHErr(t *testing.T) { scraper := newScraper(cfg, settings) require.NoError(t, scraper.start(context.Background(), componenttest.NewNopHost()), "should not err to start") - _, err := scraper.scrape(context.Background()) + _, err = scraper.scrape(context.Background()) require.NoError(t, err, "should not err") }