From 700bf0a25d0e52e63b4b7c9ee0f217c03b824ff7 Mon Sep 17 00:00:00 2001 From: Anton Kalpakchiev Date: Thu, 21 Nov 2024 16:19:29 +0100 Subject: [PATCH 1/4] Enable caching of agent readiness --- agent/agentserver/server.go | 17 +++++- agent/agentserver/server_test.go | 94 +++++++++++++++++++++++++++----- 2 files changed, 97 insertions(+), 14 deletions(-) diff --git a/agent/agentserver/server.go b/agent/agentserver/server.go index 30eed7f9..93376021 100644 --- a/agent/agentserver/server.go +++ b/agent/agentserver/server.go @@ -23,6 +23,7 @@ import ( "os" "strings" "sync" + "time" "github.com/uber/kraken/build-index/tagclient" "github.com/uber/kraken/core" @@ -39,7 +40,10 @@ import ( ) // Config defines Server configuration. -type Config struct{} +type Config struct { + // How long a successful readiness check is valid for. If 0, disable caching successful readiness. + readinessCacheTTL time.Duration `yaml:"readiness_cache_ttl"` +} // Server defines the agent HTTP server. type Server struct { @@ -50,6 +54,7 @@ type Server struct { tags tagclient.Client ac announceclient.Client containerRuntime containerruntime.Factory + lastReady time.Time } // New creates a new Server. @@ -208,6 +213,14 @@ func (s *Server) healthHandler(w http.ResponseWriter, r *http.Request) error { } func (s *Server) readinessCheckHandler(w http.ResponseWriter, r *http.Request) error { + if s.config.readinessCacheTTL != 0 { + rCacheValid := s.lastReady.Add(s.config.readinessCacheTTL).After(time.Now()) + if rCacheValid { + io.WriteString(w, "OK") + return nil + } + } + var schedErr, buildIndexErr, trackerErr error var wg sync.WaitGroup @@ -236,6 +249,8 @@ func (s *Server) readinessCheckHandler(w http.ResponseWriter, r *http.Request) e if len(errMsgs) != 0 { return handler.Errorf("agent not ready: %v", strings.Join(errMsgs, "\n")).Status(http.StatusServiceUnavailable) } + + s.lastReady = time.Now() io.WriteString(w, "OK") return nil } diff --git a/agent/agentserver/server_test.go b/agent/agentserver/server_test.go index 00020787..d2c4f896 100644 --- a/agent/agentserver/server_test.go +++ b/agent/agentserver/server_test.go @@ -78,8 +78,8 @@ func newServerMocks(t *testing.T) (*serverMocks, func()) { containerruntime, &cleanup}, cleanup.Run } -func (m *serverMocks) startServer() string { - s := New(Config{}, tally.NoopScope, m.cads, m.sched, m.tags, m.ac, m.containerRuntime) +func (m *serverMocks) startServer(c Config) string { + s := New(c, tally.NoopScope, m.cads, m.sched, m.tags, m.ac, m.containerRuntime) addr, stop := testutil.StartServer(s.Handler()) m.cleanup.Add(stop) return addr @@ -96,7 +96,7 @@ func TestGetTag(t *testing.T) { mocks.tags.EXPECT().Get(tag).Return(d, nil) - c := agentclient.New(mocks.startServer()) + c := agentclient.New(mocks.startServer(Config{})) result, err := c.GetTag(tag) require.NoError(err) @@ -113,7 +113,7 @@ func TestGetTagNotFound(t *testing.T) { mocks.tags.EXPECT().Get(tag).Return(core.Digest{}, tagclient.ErrTagNotFound) - c := agentclient.New(mocks.startServer()) + c := agentclient.New(mocks.startServer(Config{})) _, err := c.GetTag(tag) require.Error(err) @@ -134,7 +134,7 @@ func TestDownload(t *testing.T) { return store.RunDownload(mocks.cads, d, blob.Content) }) - addr := mocks.startServer() + addr := mocks.startServer(Config{}) c := agentclient.New(addr) r, err := c.Download(namespace, blob.Digest) @@ -155,7 +155,7 @@ func TestDownloadNotFound(t *testing.T) { mocks.sched.EXPECT().Download(namespace, blob.Digest).Return(scheduler.ErrTorrentNotFound) - addr := mocks.startServer() + addr := mocks.startServer(Config{}) c := agentclient.New(addr) _, err := c.Download(namespace, blob.Digest) @@ -174,7 +174,7 @@ func TestDownloadUnknownError(t *testing.T) { mocks.sched.EXPECT().Download(namespace, blob.Digest).Return(fmt.Errorf("test error")) - addr := mocks.startServer() + addr := mocks.startServer(Config{}) c := agentclient.New(addr) _, err := c.Download(namespace, blob.Digest) @@ -199,7 +199,7 @@ func TestHealthHandler(t *testing.T) { mocks.sched.EXPECT().Probe().Return(test.probeErr) - addr := mocks.startServer() + addr := mocks.startServer(Config{}) _, err := httputil.Get(fmt.Sprintf("http://%s/health", addr)) if test.probeErr != nil { @@ -265,7 +265,7 @@ func TestReadinessCheckHandler(t *testing.T) { mocks.tags.EXPECT().CheckReadiness().Return(tc.buildIndexErr) mocks.ac.EXPECT().CheckReadiness().Return(tc.trackerErr) - addr := mocks.startServer() + addr := mocks.startServer(Config{}) _, err := httputil.Get(fmt.Sprintf("http://%s/readiness", addr)) if tc.wantErr == "" { require.Nil(err) @@ -276,13 +276,81 @@ func TestReadinessCheckHandler(t *testing.T) { } } +func TestReadinessCheckHandlerCache(t *testing.T) { + for _, tc := range []struct { + desc string + firstCallErr error + readinessCacheTTL time.Duration + waitInvalidation bool + }{ + { + desc: "call 1 succeeds, so second call automatically succeeds", + firstCallErr: nil, + readinessCacheTTL: 10 * time.Minute, + waitInvalidation: false, + }, + { + desc: "call 1 fails, so second call performs checks", + firstCallErr: errors.New("test-error"), + readinessCacheTTL: 10 * time.Minute, + waitInvalidation: false, + }, + { + desc: "call 1 succeeds, but cache becomes invalid, so second call performs checks", + firstCallErr: nil, + readinessCacheTTL: 50 * time.Millisecond, + waitInvalidation: true, + }, + { + desc: "call 1 succeeds, but caching is disabled, so second call performs checks", + firstCallErr: nil, + readinessCacheTTL: 0, + waitInvalidation: false, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + require := require.New(t) + mocks, cleanup := newServerMocks(t) + defer cleanup() + + mocks.sched.EXPECT().Probe().Return(tc.firstCallErr).Times(1) + mocks.tags.EXPECT().CheckReadiness().Return(tc.firstCallErr).Times(1) + mocks.ac.EXPECT().CheckReadiness().Return(tc.firstCallErr).Times(1) + if tc.firstCallErr != nil || tc.waitInvalidation || tc.readinessCacheTTL == 0 { + mocks.sched.EXPECT().Probe().Return(nil).Times(1) + mocks.tags.EXPECT().CheckReadiness().Return(nil).Times(1) + mocks.ac.EXPECT().CheckReadiness().Return(nil).Times(1) + } + + addr := mocks.startServer(Config{readinessCacheTTL: tc.readinessCacheTTL}) + r, err := httputil.Get(fmt.Sprintf("http://%s/readiness", addr)) + if tc.firstCallErr == nil { + require.Nil(err) + respBody, _ := ioutil.ReadAll(r.Body) + require.Equal("OK", string(respBody)) + } else { + require.Error(err) + } + + if tc.waitInvalidation { + time.Sleep(tc.readinessCacheTTL) + } + + r, err = httputil.Get(fmt.Sprintf("http://%s/readiness", addr)) + require.Nil(err) + respBody, _ := ioutil.ReadAll(r.Body) + require.Equal("OK", string(respBody)) + }) + } +} + func TestPatchSchedulerConfigHandler(t *testing.T) { require := require.New(t) mocks, cleanup := newServerMocks(t) defer cleanup() - addr := mocks.startServer() + addr := mocks.startServer(Config{}) config := scheduler.Config{ ConnTTI: time.Minute, @@ -311,7 +379,7 @@ func TestGetBlacklistHandler(t *testing.T) { }} mocks.sched.EXPECT().BlacklistSnapshot().Return(blacklist, nil) - addr := mocks.startServer() + addr := mocks.startServer(Config{}) resp, err := httputil.Get(fmt.Sprintf("http://%s/x/blacklist", addr)) require.NoError(err) @@ -329,7 +397,7 @@ func TestDeleteBlobHandler(t *testing.T) { d := core.DigestFixture() - addr := mocks.startServer() + addr := mocks.startServer(Config{}) mocks.sched.EXPECT().RemoveTorrent(d).Return(nil) @@ -381,7 +449,7 @@ func TestPreloadHandler(t *testing.T) { defer cleanup() tt.setup(mocks) - addr := mocks.startServer() + addr := mocks.startServer(Config{}) _, err := httputil.Get(fmt.Sprintf("http://%s%s", addr, tt.url)) if tt.expectedError != "" { From 61783c50f604debc4f36c22544e39bfe3a2d5fb9 Mon Sep 17 00:00:00 2001 From: Anton Kalpakchiev Date: Fri, 22 Nov 2024 15:56:56 +0100 Subject: [PATCH 2/4] Remove time.Sleep from tests --- agent/agentserver/server_test.go | 36 +++++++++++++++++--------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/agent/agentserver/server_test.go b/agent/agentserver/server_test.go index d2c4f896..1f749cc5 100644 --- a/agent/agentserver/server_test.go +++ b/agent/agentserver/server_test.go @@ -78,11 +78,11 @@ func newServerMocks(t *testing.T) (*serverMocks, func()) { containerruntime, &cleanup}, cleanup.Run } -func (m *serverMocks) startServer(c Config) string { +func (m *serverMocks) startServer(c Config) (*Server, string) { s := New(c, tally.NoopScope, m.cads, m.sched, m.tags, m.ac, m.containerRuntime) addr, stop := testutil.StartServer(s.Handler()) m.cleanup.Add(stop) - return addr + return s, addr } func TestGetTag(t *testing.T) { @@ -95,8 +95,8 @@ func TestGetTag(t *testing.T) { d := core.DigestFixture() mocks.tags.EXPECT().Get(tag).Return(d, nil) - - c := agentclient.New(mocks.startServer(Config{})) + _, addr := mocks.startServer(Config{}) + c := agentclient.New(addr) result, err := c.GetTag(tag) require.NoError(err) @@ -113,7 +113,8 @@ func TestGetTagNotFound(t *testing.T) { mocks.tags.EXPECT().Get(tag).Return(core.Digest{}, tagclient.ErrTagNotFound) - c := agentclient.New(mocks.startServer(Config{})) + _, addr := mocks.startServer(Config{}) + c := agentclient.New(addr) _, err := c.GetTag(tag) require.Error(err) @@ -134,7 +135,7 @@ func TestDownload(t *testing.T) { return store.RunDownload(mocks.cads, d, blob.Content) }) - addr := mocks.startServer(Config{}) + _, addr := mocks.startServer(Config{}) c := agentclient.New(addr) r, err := c.Download(namespace, blob.Digest) @@ -155,7 +156,7 @@ func TestDownloadNotFound(t *testing.T) { mocks.sched.EXPECT().Download(namespace, blob.Digest).Return(scheduler.ErrTorrentNotFound) - addr := mocks.startServer(Config{}) + _, addr := mocks.startServer(Config{}) c := agentclient.New(addr) _, err := c.Download(namespace, blob.Digest) @@ -174,7 +175,7 @@ func TestDownloadUnknownError(t *testing.T) { mocks.sched.EXPECT().Download(namespace, blob.Digest).Return(fmt.Errorf("test error")) - addr := mocks.startServer(Config{}) + _, addr := mocks.startServer(Config{}) c := agentclient.New(addr) _, err := c.Download(namespace, blob.Digest) @@ -199,7 +200,7 @@ func TestHealthHandler(t *testing.T) { mocks.sched.EXPECT().Probe().Return(test.probeErr) - addr := mocks.startServer(Config{}) + _, addr := mocks.startServer(Config{}) _, err := httputil.Get(fmt.Sprintf("http://%s/health", addr)) if test.probeErr != nil { @@ -265,7 +266,7 @@ func TestReadinessCheckHandler(t *testing.T) { mocks.tags.EXPECT().CheckReadiness().Return(tc.buildIndexErr) mocks.ac.EXPECT().CheckReadiness().Return(tc.trackerErr) - addr := mocks.startServer(Config{}) + _, addr := mocks.startServer(Config{}) _, err := httputil.Get(fmt.Sprintf("http://%s/readiness", addr)) if tc.wantErr == "" { require.Nil(err) @@ -298,7 +299,7 @@ func TestReadinessCheckHandlerCache(t *testing.T) { { desc: "call 1 succeeds, but cache becomes invalid, so second call performs checks", firstCallErr: nil, - readinessCacheTTL: 50 * time.Millisecond, + readinessCacheTTL: 10 * time.Minute, waitInvalidation: true, }, { @@ -322,7 +323,7 @@ func TestReadinessCheckHandlerCache(t *testing.T) { mocks.ac.EXPECT().CheckReadiness().Return(nil).Times(1) } - addr := mocks.startServer(Config{readinessCacheTTL: tc.readinessCacheTTL}) + s, addr := mocks.startServer(Config{readinessCacheTTL: tc.readinessCacheTTL}) r, err := httputil.Get(fmt.Sprintf("http://%s/readiness", addr)) if tc.firstCallErr == nil { require.Nil(err) @@ -333,7 +334,8 @@ func TestReadinessCheckHandlerCache(t *testing.T) { } if tc.waitInvalidation { - time.Sleep(tc.readinessCacheTTL) + // To avoid using time.Sleep, we can rollback the server's lastReady variable to simulate cache invalidation. + s.lastReady = s.lastReady.Add(-1 * tc.readinessCacheTTL) } r, err = httputil.Get(fmt.Sprintf("http://%s/readiness", addr)) @@ -350,7 +352,7 @@ func TestPatchSchedulerConfigHandler(t *testing.T) { mocks, cleanup := newServerMocks(t) defer cleanup() - addr := mocks.startServer(Config{}) + _, addr := mocks.startServer(Config{}) config := scheduler.Config{ ConnTTI: time.Minute, @@ -379,7 +381,7 @@ func TestGetBlacklistHandler(t *testing.T) { }} mocks.sched.EXPECT().BlacklistSnapshot().Return(blacklist, nil) - addr := mocks.startServer(Config{}) + _, addr := mocks.startServer(Config{}) resp, err := httputil.Get(fmt.Sprintf("http://%s/x/blacklist", addr)) require.NoError(err) @@ -397,7 +399,7 @@ func TestDeleteBlobHandler(t *testing.T) { d := core.DigestFixture() - addr := mocks.startServer(Config{}) + _, addr := mocks.startServer(Config{}) mocks.sched.EXPECT().RemoveTorrent(d).Return(nil) @@ -449,7 +451,7 @@ func TestPreloadHandler(t *testing.T) { defer cleanup() tt.setup(mocks) - addr := mocks.startServer(Config{}) + _, addr := mocks.startServer(Config{}) _, err := httputil.Get(fmt.Sprintf("http://%s%s", addr, tt.url)) if tt.expectedError != "" { From f9406e5e3242122838e68ab1b1fe79231b4a8c87 Mon Sep 17 00:00:00 2001 From: Anton Kalpakchiev Date: Mon, 25 Nov 2024 17:40:36 +0100 Subject: [PATCH 3/4] Refactor tests to improve readability --- agent/agentserver/server_test.go | 77 ++++++++++++++++++++++---------- 1 file changed, 54 insertions(+), 23 deletions(-) diff --git a/agent/agentserver/server_test.go b/agent/agentserver/server_test.go index 1f749cc5..ceb98212 100644 --- a/agent/agentserver/server_test.go +++ b/agent/agentserver/server_test.go @@ -278,35 +278,73 @@ func TestReadinessCheckHandler(t *testing.T) { } func TestReadinessCheckHandlerCache(t *testing.T) { + testErr := errors.New("test-err") + type call struct { + success bool + } + for _, tc := range []struct { desc string - firstCallErr error readinessCacheTTL time.Duration waitInvalidation bool + expCalls []call + setupMocks func(m *serverMocks) }{ { - desc: "call 1 succeeds, so second call automatically succeeds", - firstCallErr: nil, + desc: "call 1 succeeds, so second call succeeds without checks", readinessCacheTTL: 10 * time.Minute, waitInvalidation: false, + expCalls: []call{{success: true}, {success: true}}, + setupMocks: func(m *serverMocks) { + m.sched.EXPECT().Probe().Return(nil).Times(1) + m.tags.EXPECT().CheckReadiness().Return(nil).Times(1) + m.ac.EXPECT().CheckReadiness().Return(nil).Times(1) + }, }, { desc: "call 1 fails, so second call performs checks", - firstCallErr: errors.New("test-error"), readinessCacheTTL: 10 * time.Minute, waitInvalidation: false, + expCalls: []call{{success: false}, {success: true}}, + setupMocks: func(m *serverMocks) { + m.sched.EXPECT().Probe().Return(testErr).Times(1) + m.tags.EXPECT().CheckReadiness().Return(testErr).Times(1) + m.ac.EXPECT().CheckReadiness().Return(testErr).Times(1) + + m.sched.EXPECT().Probe().Return(nil).Times(1) + m.tags.EXPECT().CheckReadiness().Return(nil).Times(1) + m.ac.EXPECT().CheckReadiness().Return(nil).Times(1) + }, }, { desc: "call 1 succeeds, but cache becomes invalid, so second call performs checks", - firstCallErr: nil, readinessCacheTTL: 10 * time.Minute, waitInvalidation: true, + expCalls: []call{{success: true}, {success: false}}, + setupMocks: func(m *serverMocks) { + m.sched.EXPECT().Probe().Return(nil).Times(1) + m.tags.EXPECT().CheckReadiness().Return(nil).Times(1) + m.ac.EXPECT().CheckReadiness().Return(nil).Times(1) + + m.sched.EXPECT().Probe().Return(testErr).Times(1) + m.tags.EXPECT().CheckReadiness().Return(testErr).Times(1) + m.ac.EXPECT().CheckReadiness().Return(testErr).Times(1) + }, }, { desc: "call 1 succeeds, but caching is disabled, so second call performs checks", - firstCallErr: nil, readinessCacheTTL: 0, waitInvalidation: false, + expCalls: []call{{success: true}, {success: false}}, + setupMocks: func(m *serverMocks) { + m.sched.EXPECT().Probe().Return(nil).Times(1) + m.tags.EXPECT().CheckReadiness().Return(nil).Times(1) + m.ac.EXPECT().CheckReadiness().Return(nil).Times(1) + + m.sched.EXPECT().Probe().Return(testErr).Times(1) + m.tags.EXPECT().CheckReadiness().Return(testErr).Times(1) + m.ac.EXPECT().CheckReadiness().Return(testErr).Times(1) + }, }, } { t.Run(tc.desc, func(t *testing.T) { @@ -314,21 +352,12 @@ func TestReadinessCheckHandlerCache(t *testing.T) { mocks, cleanup := newServerMocks(t) defer cleanup() - mocks.sched.EXPECT().Probe().Return(tc.firstCallErr).Times(1) - mocks.tags.EXPECT().CheckReadiness().Return(tc.firstCallErr).Times(1) - mocks.ac.EXPECT().CheckReadiness().Return(tc.firstCallErr).Times(1) - if tc.firstCallErr != nil || tc.waitInvalidation || tc.readinessCacheTTL == 0 { - mocks.sched.EXPECT().Probe().Return(nil).Times(1) - mocks.tags.EXPECT().CheckReadiness().Return(nil).Times(1) - mocks.ac.EXPECT().CheckReadiness().Return(nil).Times(1) - } + tc.setupMocks(mocks) s, addr := mocks.startServer(Config{readinessCacheTTL: tc.readinessCacheTTL}) - r, err := httputil.Get(fmt.Sprintf("http://%s/readiness", addr)) - if tc.firstCallErr == nil { - require.Nil(err) - respBody, _ := ioutil.ReadAll(r.Body) - require.Equal("OK", string(respBody)) + _, err := httputil.Get(fmt.Sprintf("http://%s/readiness", addr)) + if tc.expCalls[0].success { + require.NoError(err) } else { require.Error(err) } @@ -338,10 +367,12 @@ func TestReadinessCheckHandlerCache(t *testing.T) { s.lastReady = s.lastReady.Add(-1 * tc.readinessCacheTTL) } - r, err = httputil.Get(fmt.Sprintf("http://%s/readiness", addr)) - require.Nil(err) - respBody, _ := ioutil.ReadAll(r.Body) - require.Equal("OK", string(respBody)) + _, err = httputil.Get(fmt.Sprintf("http://%s/readiness", addr)) + if tc.expCalls[1].success { + require.NoError(err) + } else { + require.Error(err) + } }) } } From 41c2b65a556edb709a3a613e8abe90e90e9f6b0d Mon Sep 17 00:00:00 2001 From: Anton Kalpakchiev Date: Mon, 25 Nov 2024 21:04:42 +0100 Subject: [PATCH 4/4] Polish tests --- agent/agentserver/server_test.go | 96 ++++++++++++++++---------------- 1 file changed, 49 insertions(+), 47 deletions(-) diff --git a/agent/agentserver/server_test.go b/agent/agentserver/server_test.go index ceb98212..7bea45ee 100644 --- a/agent/agentserver/server_test.go +++ b/agent/agentserver/server_test.go @@ -279,71 +279,73 @@ func TestReadinessCheckHandler(t *testing.T) { func TestReadinessCheckHandlerCache(t *testing.T) { testErr := errors.New("test-err") - type call struct { - success bool - } for _, tc := range []struct { - desc string - readinessCacheTTL time.Duration - waitInvalidation bool - expCalls []call - setupMocks func(m *serverMocks) + desc string + readinessCacheTTL time.Duration + waitInvalidation bool + wantFirstCallSuccess bool + wantSecondCallSuccess bool + setupMocks func(m *serverMocks) }{ { - desc: "call 1 succeeds, so second call succeeds without checks", - readinessCacheTTL: 10 * time.Minute, - waitInvalidation: false, - expCalls: []call{{success: true}, {success: true}}, + desc: "call 1 succeeds, so second call succeeds without checks", + readinessCacheTTL: 10 * time.Minute, + waitInvalidation: false, + wantFirstCallSuccess: true, + wantSecondCallSuccess: true, setupMocks: func(m *serverMocks) { - m.sched.EXPECT().Probe().Return(nil).Times(1) - m.tags.EXPECT().CheckReadiness().Return(nil).Times(1) - m.ac.EXPECT().CheckReadiness().Return(nil).Times(1) + m.sched.EXPECT().Probe().Return(nil) + m.tags.EXPECT().CheckReadiness().Return(nil) + m.ac.EXPECT().CheckReadiness().Return(nil) }, }, { - desc: "call 1 fails, so second call performs checks", - readinessCacheTTL: 10 * time.Minute, - waitInvalidation: false, - expCalls: []call{{success: false}, {success: true}}, + desc: "call 1 fails, so second call performs checks", + readinessCacheTTL: 10 * time.Minute, + waitInvalidation: false, + wantFirstCallSuccess: false, + wantSecondCallSuccess: true, setupMocks: func(m *serverMocks) { - m.sched.EXPECT().Probe().Return(testErr).Times(1) - m.tags.EXPECT().CheckReadiness().Return(testErr).Times(1) - m.ac.EXPECT().CheckReadiness().Return(testErr).Times(1) + m.sched.EXPECT().Probe().Return(testErr) + m.tags.EXPECT().CheckReadiness().Return(testErr) + m.ac.EXPECT().CheckReadiness().Return(testErr) - m.sched.EXPECT().Probe().Return(nil).Times(1) - m.tags.EXPECT().CheckReadiness().Return(nil).Times(1) - m.ac.EXPECT().CheckReadiness().Return(nil).Times(1) + m.sched.EXPECT().Probe().Return(nil) + m.tags.EXPECT().CheckReadiness().Return(nil) + m.ac.EXPECT().CheckReadiness().Return(nil) }, }, { - desc: "call 1 succeeds, but cache becomes invalid, so second call performs checks", - readinessCacheTTL: 10 * time.Minute, - waitInvalidation: true, - expCalls: []call{{success: true}, {success: false}}, + desc: "call 1 succeeds, but cache becomes invalid, so second call performs checks", + readinessCacheTTL: 10 * time.Minute, + waitInvalidation: true, + wantFirstCallSuccess: true, + wantSecondCallSuccess: false, setupMocks: func(m *serverMocks) { - m.sched.EXPECT().Probe().Return(nil).Times(1) - m.tags.EXPECT().CheckReadiness().Return(nil).Times(1) - m.ac.EXPECT().CheckReadiness().Return(nil).Times(1) + m.sched.EXPECT().Probe().Return(nil) + m.tags.EXPECT().CheckReadiness().Return(nil) + m.ac.EXPECT().CheckReadiness().Return(nil) - m.sched.EXPECT().Probe().Return(testErr).Times(1) - m.tags.EXPECT().CheckReadiness().Return(testErr).Times(1) - m.ac.EXPECT().CheckReadiness().Return(testErr).Times(1) + m.sched.EXPECT().Probe().Return(testErr) + m.tags.EXPECT().CheckReadiness().Return(testErr) + m.ac.EXPECT().CheckReadiness().Return(testErr) }, }, { - desc: "call 1 succeeds, but caching is disabled, so second call performs checks", - readinessCacheTTL: 0, - waitInvalidation: false, - expCalls: []call{{success: true}, {success: false}}, + desc: "call 1 succeeds, but caching is disabled, so second call performs checks", + readinessCacheTTL: 0, + waitInvalidation: false, + wantFirstCallSuccess: true, + wantSecondCallSuccess: false, setupMocks: func(m *serverMocks) { - m.sched.EXPECT().Probe().Return(nil).Times(1) - m.tags.EXPECT().CheckReadiness().Return(nil).Times(1) - m.ac.EXPECT().CheckReadiness().Return(nil).Times(1) + m.sched.EXPECT().Probe().Return(nil) + m.tags.EXPECT().CheckReadiness().Return(nil) + m.ac.EXPECT().CheckReadiness().Return(nil) - m.sched.EXPECT().Probe().Return(testErr).Times(1) - m.tags.EXPECT().CheckReadiness().Return(testErr).Times(1) - m.ac.EXPECT().CheckReadiness().Return(testErr).Times(1) + m.sched.EXPECT().Probe().Return(testErr) + m.tags.EXPECT().CheckReadiness().Return(testErr) + m.ac.EXPECT().CheckReadiness().Return(testErr) }, }, } { @@ -356,7 +358,7 @@ func TestReadinessCheckHandlerCache(t *testing.T) { s, addr := mocks.startServer(Config{readinessCacheTTL: tc.readinessCacheTTL}) _, err := httputil.Get(fmt.Sprintf("http://%s/readiness", addr)) - if tc.expCalls[0].success { + if tc.wantFirstCallSuccess { require.NoError(err) } else { require.Error(err) @@ -368,7 +370,7 @@ func TestReadinessCheckHandlerCache(t *testing.T) { } _, err = httputil.Get(fmt.Sprintf("http://%s/readiness", addr)) - if tc.expCalls[1].success { + if tc.wantSecondCallSuccess { require.NoError(err) } else { require.Error(err)