From ba51956ec3e44c81d4efd45394c897484a2f6d57 Mon Sep 17 00:00:00 2001 From: Jan-Eric Hellenberg Date: Wed, 5 May 2021 09:07:23 +0200 Subject: [PATCH] Add destroy runner route --- api/api.go | 12 +++--- api/api_test.go | 4 +- api/runners.go | 21 +++++++++- api/runners_test.go | 92 +++++++++++++++++++++++++++++++++++++++++++- main.go | 6 +-- mocks/ExecutorApi.go | 15 ++++++-- nomad/nomad.go | 10 +++++ 7 files changed, 144 insertions(+), 16 deletions(-) diff --git a/api/api.go b/api/api.go index a9019a3..d50f4b6 100644 --- a/api/api.go +++ b/api/api.go @@ -5,6 +5,7 @@ import ( "gitlab.hpi.de/codeocean/codemoon/poseidon/api/auth" "gitlab.hpi.de/codeocean/codemoon/poseidon/environment" "gitlab.hpi.de/codeocean/codemoon/poseidon/logging" + "gitlab.hpi.de/codeocean/codemoon/poseidon/nomad" "net/http" ) @@ -21,18 +22,17 @@ const ( // always returns a router for the newest version of our API. We // use gorilla/mux because it is more convenient than net/http, e.g. // when extracting path parameters. -func NewRouter(runnerPool environment.RunnerPool) *mux.Router { +func NewRouter(apiClient nomad.ExecutorApi, runnerPool environment.RunnerPool) *mux.Router { router := mux.NewRouter() // this can later be restricted to a specific host with // `router.Host(...)` and to HTTPS with `router.Schemes("https")` - router = newRouterV1(router, runnerPool) + router = newRouterV1(router, apiClient, runnerPool) router.Use(logging.HTTPLoggingMiddleware) return router } -// newRouterV1 returns a sub-router containing the routes of version -// 1 of our API. -func newRouterV1(router *mux.Router, runnerPool environment.RunnerPool) *mux.Router { +// newRouterV1 returns a sub-router containing the routes of version 1 of our API. +func newRouterV1(router *mux.Router, apiClient nomad.ExecutorApi, runnerPool environment.RunnerPool) *mux.Router { v1 := router.PathPrefix(RouteBase).Subrouter() v1.HandleFunc(RouteHealth, Health).Methods(http.MethodGet) @@ -42,7 +42,7 @@ func newRouterV1(router *mux.Router, runnerPool environment.RunnerPool) *mux.Rou v1 = v1.PathPrefix("").Subrouter() v1.Use(auth.HTTPAuthenticationMiddleware) } - registerRunnerRoutes(v1.PathPrefix(RouteRunners).Subrouter(), runnerPool) + registerRunnerRoutes(v1.PathPrefix(RouteRunners).Subrouter(), apiClient, runnerPool) return v1 } diff --git a/api/api_test.go b/api/api_test.go index 83a580c..4fc48de 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -17,7 +17,7 @@ func mockHTTPHandler(writer http.ResponseWriter, _ *http.Request) { func TestNewRouterV1WithAuthenticationDisabled(t *testing.T) { config.Config.Server.Token = "" router := mux.NewRouter() - v1 := newRouterV1(router, environment.NewLocalRunnerPool()) + v1 := newRouterV1(router, nil, environment.NewLocalRunnerPool()) t.Run("health route is accessible", func(t *testing.T) { request, err := http.NewRequest(http.MethodGet, "/api/v1/health", nil) @@ -44,7 +44,7 @@ func TestNewRouterV1WithAuthenticationDisabled(t *testing.T) { func TestNewRouterV1WithAuthenticationEnabled(t *testing.T) { config.Config.Server.Token = "TestToken" router := mux.NewRouter() - v1 := newRouterV1(router, environment.NewLocalRunnerPool()) + v1 := newRouterV1(router, nil, environment.NewLocalRunnerPool()) t.Run("health route is accessible", func(t *testing.T) { request, err := http.NewRequest(http.MethodGet, "/api/v1/health", nil) diff --git a/api/runners.go b/api/runners.go index c3b9f4c..00dddcb 100644 --- a/api/runners.go +++ b/api/runners.go @@ -7,6 +7,7 @@ import ( "gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto" "gitlab.hpi.de/codeocean/codemoon/poseidon/config" "gitlab.hpi.de/codeocean/codemoon/poseidon/environment" + "gitlab.hpi.de/codeocean/codemoon/poseidon/nomad" "gitlab.hpi.de/codeocean/codemoon/poseidon/runner" "net/http" "net/url" @@ -15,6 +16,7 @@ import ( const ( ExecutePath = "/execute" WebsocketPath = "/websocket" + DeleteRoute = "deleteRunner" RunnerIdKey = "runnerId" ExecutionIdKey = "executionId" ) @@ -111,10 +113,27 @@ func findRunnerMiddleware(runnerPool environment.RunnerPool) func(handler http.H } } -func registerRunnerRoutes(router *mux.Router, runnerPool environment.RunnerPool) { +func deleteRunner(apiClient nomad.ExecutorApi, runnerPool environment.RunnerPool) func(writer http.ResponseWriter, request *http.Request) { + return func(writer http.ResponseWriter, request *http.Request) { + targetRunner, _ := runner.FromContext(request.Context()) + + err := apiClient.DeleteRunner(targetRunner.Id()) + if err != nil { + writeInternalServerError(writer, err, dto.ErrorNomadInternalServerError) + return + } + + runnerPool.Delete(targetRunner.Id()) + + writer.WriteHeader(http.StatusNoContent) + } +} + +func registerRunnerRoutes(router *mux.Router, apiClient nomad.ExecutorApi, runnerPool environment.RunnerPool) { router.HandleFunc("", provideRunner).Methods(http.MethodPost) runnerRouter := router.PathPrefix(fmt.Sprintf("/{%s}", RunnerIdKey)).Subrouter() runnerRouter.Use(findRunnerMiddleware(runnerPool)) runnerRouter.HandleFunc(ExecutePath, executeCommand(runnerRouter)).Methods(http.MethodPost).Name(ExecutePath) runnerRouter.HandleFunc(WebsocketPath, connectToRunner).Methods(http.MethodGet).Name(WebsocketPath) + runnerRouter.HandleFunc("", deleteRunner(apiClient, runnerPool)).Methods(http.MethodDelete).Name(DeleteRoute) } diff --git a/api/runners_test.go b/api/runners_test.go index ea023ba..0a2eb09 100644 --- a/api/runners_test.go +++ b/api/runners_test.go @@ -3,11 +3,15 @@ package api import ( "bytes" "encoding/json" + "errors" "fmt" "github.com/gorilla/mux" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" "gitlab.hpi.de/codeocean/codemoon/poseidon/api/dto" "gitlab.hpi.de/codeocean/codemoon/poseidon/environment" + "gitlab.hpi.de/codeocean/codemoon/poseidon/mocks" "gitlab.hpi.de/codeocean/codemoon/poseidon/runner" "net/http" "net/http/httptest" @@ -67,7 +71,7 @@ func TestFindRunnerMiddleware(t *testing.T) { func TestExecuteRoute(t *testing.T) { runnerPool := environment.NewLocalRunnerPool() - router := NewRouter(runnerPool) + router := NewRouter(nil, runnerPool) testRunner := runner.NewExerciseRunner("testRunner") runnerPool.Add(testRunner) @@ -127,3 +131,89 @@ func TestExecuteRoute(t *testing.T) { assert.Equal(t, http.StatusBadRequest, recorder.Code) }) } + +func TestDeleteRunnerRouteTestSuite(t *testing.T) { + suite.Run(t, new(DeleteRunnerRouteTestSuite)) +} + +type DeleteRunnerRouteTestSuite struct { + suite.Suite + runnerPool environment.RunnerPool + apiClient *mocks.ExecutorApi + router *mux.Router + testRunner runner.Runner + path string +} + +func (suite *DeleteRunnerRouteTestSuite) SetupTest() { + suite.runnerPool = environment.NewLocalRunnerPool() + suite.apiClient = &mocks.ExecutorApi{} + suite.router = NewRouter(suite.apiClient, suite.runnerPool) + + suite.testRunner = runner.NewExerciseRunner("testRunner") + suite.runnerPool.Add(suite.testRunner) + + var err error + runnerUrl, err := suite.router.Get(DeleteRoute).URL(RunnerIdKey, suite.testRunner.Id()) + if err != nil { + suite.T().Fatal(err) + } + suite.path = runnerUrl.String() +} + +func (suite *DeleteRunnerRouteTestSuite) TestValidRequestReturnsNoContent() { + suite.apiClient.On("DeleteRunner", mock.AnythingOfType("string")).Return(nil) + + recorder := httptest.NewRecorder() + request, err := http.NewRequest(http.MethodDelete, suite.path, nil) + if err != nil { + suite.T().Fatal(err) + } + + suite.router.ServeHTTP(recorder, request) + + suite.Equal(http.StatusNoContent, recorder.Code) + + suite.Run("runner is deleted on nomad", func() { + suite.apiClient.AssertCalled(suite.T(), "DeleteRunner", suite.testRunner.Id()) + }) + + suite.Run("runner is deleted from runnerPool", func() { + returnedRunner, ok := suite.runnerPool.Get(suite.testRunner.Id()) + suite.Nil(returnedRunner) + suite.False(ok) + }) +} + +func (suite *DeleteRunnerRouteTestSuite) TestReturnInternalServerErrorWhenApiCallToNomadFailed() { + suite.apiClient.On("DeleteRunner", mock.AnythingOfType("string")).Return(errors.New("API call failed")) + + recorder := httptest.NewRecorder() + request, err := http.NewRequest(http.MethodDelete, suite.path, nil) + if err != nil { + suite.T().Fatal(err) + } + + suite.router.ServeHTTP(recorder, request) + + suite.Equal(http.StatusInternalServerError, recorder.Code) +} + +func (suite *DeleteRunnerRouteTestSuite) TestDeleteInvalidRunnerIdReturnsNotFound() { + var err error + runnersUrl, err := suite.router.Get(DeleteRoute).URL(RunnerIdKey, "1nv4l1dID") + if err != nil { + suite.T().Fatal(err) + } + suite.path = runnersUrl.String() + + recorder := httptest.NewRecorder() + request, err := http.NewRequest(http.MethodDelete, suite.path, nil) + if err != nil { + suite.T().Fatal(err) + } + + suite.router.ServeHTTP(recorder, request) + + suite.Equal(http.StatusNotFound, recorder.Code) +} diff --git a/main.go b/main.go index 7af381f..87c7340 100644 --- a/main.go +++ b/main.go @@ -38,13 +38,13 @@ func runServer(server *http.Server) { } } -func initServer(runnerPool environment.RunnerPool) *http.Server { +func initServer(apiClient nomad.ExecutorApi, runnerPool environment.RunnerPool) *http.Server { return &http.Server{ Addr: config.Config.PoseidonAPIURL().Host, WriteTimeout: time.Second * 15, ReadTimeout: time.Second * 15, IdleTimeout: time.Second * 60, - Handler: api.NewRouter(runnerPool), + Handler: api.NewRouter(apiClient, runnerPool), } } @@ -78,7 +78,7 @@ func main() { runnerPool := environment.NewLocalRunnerPool() environment.DebugInit(runnerPool, nomadAPIClient) - server := initServer(runnerPool) + server := initServer(nomadAPIClient, runnerPool) go runServer(server) shutdownOnOSSignal(server) } diff --git a/mocks/ExecutorApi.go b/mocks/ExecutorApi.go index 1e2283a..f30dceb 100644 --- a/mocks/ExecutorApi.go +++ b/mocks/ExecutorApi.go @@ -12,9 +12,18 @@ type ExecutorApi struct { mock.Mock } -// CreateJob provides a mock function with given fields: -func (_m *ExecutorApi) CreateJob() { - _m.Called() +// DeleteRunner provides a mock function with given fields: runnerId +func (_m *ExecutorApi) DeleteRunner(runnerId string) error { + ret := _m.Called(runnerId) + + var r0 error + if rf, ok := ret.Get(0).(func(string) error); ok { + r0 = rf(runnerId) + } else { + r0 = ret.Error(0) + } + + return r0 } // GetJobScale provides a mock function with given fields: jobId diff --git a/nomad/nomad.go b/nomad/nomad.go index 9297e91..27d6aec 100644 --- a/nomad/nomad.go +++ b/nomad/nomad.go @@ -11,6 +11,7 @@ type ExecutorApi interface { GetJobScale(jobId string) (jobScale int, err error) SetJobScaling(jobId string, count int, reason string) (err error) LoadRunners(jobId string) (runnerIds []string, err error) + DeleteRunner(runnerId string) (err error) } // ApiClient provides access to the Nomad functionality @@ -69,3 +70,12 @@ func (apiClient *ApiClient) LoadRunners(jobId string) (runnerIds []string, err e } return } + +func (apiClient *ApiClient) DeleteRunner(runnerId string) (err error) { + allocation, _, err := apiClient.client.Allocations().Info(runnerId, nil) + if err != nil { + return + } + _, err = apiClient.client.Allocations().Stop(allocation, nil) + return err +}