Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Jakub Warczarek <[email protected]>
  • Loading branch information
czeslavo and programmer04 committed Feb 7, 2025
1 parent f37d74a commit 4c29f45
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 15 deletions.
2 changes: 1 addition & 1 deletion pkg/manager/multiinstance/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ type DiagnosticsServer struct {
listenerPort int
handlers map[manager.ID]http.Handler

mux *http.ServeMux
muxLock sync.Mutex
mux *http.ServeMux
}

func NewDiagnosticsServer(listenerPort int) *DiagnosticsServer {
Expand Down
13 changes: 5 additions & 8 deletions pkg/manager/multiinstance/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,9 @@ func TestManager_WithDiagnosticsExposer(t *testing.T) {
diagnosticsExposer := newMockDiagnosticsExposer()
multiManager := multiinstance.NewManager(testr.New(t), multiinstance.WithDiagnosticsExposer(diagnosticsExposer))

managerRunning := make(chan struct{})
go func() {
close(managerRunning)
require.NoError(t, multiManager.Run(ctx))
}()
<-managerRunning // Wait for the manager to start.

instanceID1 := manager.NewRandomID()
instanceID2 := manager.NewRandomID()
Expand All @@ -112,8 +109,8 @@ func TestManager_WithDiagnosticsExposer(t *testing.T) {

t.Log("Expecting the diagnostics exposer to have the first instance registered")
require.EventuallyWithT(t, func(t *assert.CollectT) {
require.Contains(t, diagnosticsExposer.RegisteredInstances(), instanceID1)
require.NotContains(t, diagnosticsExposer.RegisteredInstances(), instanceID2)
assert.Contains(t, diagnosticsExposer.RegisteredInstances(), instanceID1)
assert.NotContains(t, diagnosticsExposer.RegisteredInstances(), instanceID2)
}, waitTime, tickTime)

t.Log("Scheduling second instance")
Expand All @@ -122,7 +119,7 @@ func TestManager_WithDiagnosticsExposer(t *testing.T) {

t.Log("Expecting the diagnostics exposer to have both instances registered")
require.EventuallyWithT(t, func(t *assert.CollectT) {
require.ElementsMatch(t, diagnosticsExposer.RegisteredInstances(), []manager.ID{instanceID1, instanceID2})
assert.ElementsMatch(t, diagnosticsExposer.RegisteredInstances(), []manager.ID{instanceID1, instanceID2})
}, waitTime, tickTime)

t.Log("Stopping first instance")
Expand All @@ -131,8 +128,8 @@ func TestManager_WithDiagnosticsExposer(t *testing.T) {

t.Log("Expecting the diagnostics exposer to have only the second instance registered")
require.EventuallyWithT(t, func(t *assert.CollectT) {
require.Contains(t, diagnosticsExposer.RegisteredInstances(), instanceID2)
require.NotContains(t, diagnosticsExposer.RegisteredInstances(), instanceID1)
assert.Contains(t, diagnosticsExposer.RegisteredInstances(), instanceID2)
assert.NotContains(t, diagnosticsExposer.RegisteredInstances(), instanceID1)
}, waitTime, tickTime)

t.Log("Stopping second instance")
Expand Down
14 changes: 8 additions & 6 deletions test/envtest/multiinstance_manager_diagnostics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,16 @@ func TestMultiInstanceManagerDiagnostics(t *testing.T) {
t.Log("Waiting for the diagnostics server to expose instances' diagnostics endpoints")
require.EventuallyWithT(t, func(t *assert.CollectT) {
resp, err := http.Get(fmt.Sprintf("http://localhost:%d/%s/debug/config/successful", diagPort, mgrInstance1.ID()))
require.NoError(t, err)
resp.Body.Close()
require.Equal(t, http.StatusOK, resp.StatusCode)
if assert.NoError(t, err) {
resp.Body.Close()
assert.Equal(t, http.StatusOK, resp.StatusCode)
}

resp, err = http.Get(fmt.Sprintf("http://localhost:%d/%s/debug/config/successful", diagPort, mgrInstance2.ID()))
require.NoError(t, err)
resp.Body.Close()
require.Equal(t, http.StatusOK, resp.StatusCode)
if assert.NoError(t, err) {
resp.Body.Close()
assert.Equal(t, http.StatusOK, resp.StatusCode)
}
}, waitTime, tickTime, "diagnostics should be exposed under /{instanceID}/debug/config prefix for both instances")

t.Log("Stopping the first instance and waiting for its diagnostics endpoints to be removed from the server")
Expand Down

0 comments on commit 4c29f45

Please sign in to comment.