From c1eaabc3d1256e78e1d855007cacdeeb1b94aefb Mon Sep 17 00:00:00 2001 From: xu0o0 Date: Mon, 19 Jun 2023 07:56:31 +0800 Subject: [PATCH] Fix nil pointer dereference in supervisor example (#166) `Supervisor.healthCheckTicker` is prepared in `(*Supervisor).startAgent`. If there is no effective config file, we will skip the call of `startAgent` causing an nil pointer dereference in the loop. --- .../supervisor/supervisor/supervisor.go | 7 +- .../supervisor/supervisor/supervisor_test.go | 64 +++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 internal/examples/supervisor/supervisor/supervisor_test.go diff --git a/internal/examples/supervisor/supervisor/supervisor.go b/internal/examples/supervisor/supervisor/supervisor.go index 657fc4e4..83901d8f 100644 --- a/internal/examples/supervisor/supervisor/supervisor.go +++ b/internal/examples/supervisor/supervisor/supervisor.go @@ -475,6 +475,11 @@ func (s *Supervisor) runAgentProcess() { restartTimer.Stop() for { + var healthCheckTickerCh <-chan time.Time + if s.healthCheckTicker != nil { + healthCheckTickerCh = s.healthCheckTicker.C + } + select { case <-s.hasNewConfig: restartTimer.Stop() @@ -498,7 +503,7 @@ func (s *Supervisor) runAgentProcess() { case <-restartTimer.C: s.startAgent() - case <-s.healthCheckTicker.C: + case <-healthCheckTickerCh: s.healthCheck() } } diff --git a/internal/examples/supervisor/supervisor/supervisor_test.go b/internal/examples/supervisor/supervisor/supervisor_test.go new file mode 100644 index 00000000..45687201 --- /dev/null +++ b/internal/examples/supervisor/supervisor/supervisor_test.go @@ -0,0 +1,64 @@ +package supervisor + +import ( + "fmt" + "os" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/open-telemetry/opamp-go/internal" + "github.com/open-telemetry/opamp-go/internal/examples/server/data" + "github.com/open-telemetry/opamp-go/internal/examples/server/opampsrv" +) + +func changeCurrentDir(t *testing.T) string { + t.Helper() + + tmp := t.TempDir() + + oldCWD, err := os.Getwd() + if err != nil { + t.Fatalf("getting working directory: %v", err) + } + + if err := os.Chdir(tmp); err != nil { + t.Fatalf("changing working directory: %v", err) + } + t.Cleanup(func() { + if err := os.Chdir(oldCWD); err != nil { + t.Fatalf("restoring working directory: %v", err) + } + }) + + return tmp +} + +func startOpampServer(t *testing.T) { + t.Helper() + + opampSrv := opampsrv.NewServer(&data.AllAgents) + opampSrv.Start() + + t.Cleanup(func() { + opampSrv.Stop() + }) +} + +func TestNewSupervisor(t *testing.T) { + tmpDir := changeCurrentDir(t) + os.WriteFile("supervisor.yaml", []byte(fmt.Sprintf(` +server: + endpoint: ws://127.0.0.1:4320/v1/opamp +agent: + executable: %s/dummy_agent.sh`, tmpDir)), 0644) + + os.WriteFile("dummy_agent.sh", []byte("#!/bin/sh\nsleep 9999\n"), 0755) + + startOpampServer(t) + + supervisor, err := NewSupervisor(&internal.NopLogger{}) + assert.NoError(t, err) + + supervisor.Shutdown() +}