diff --git a/cmd/osbuild-composer/composer.go b/cmd/osbuild-composer/composer.go index c91fe0b6a..c8690d1cd 100644 --- a/cmd/osbuild-composer/composer.go +++ b/cmd/osbuild-composer/composer.go @@ -171,7 +171,11 @@ func (c *Composer) Start() error { if c.localWorkerListener != nil { go func() { - err := c.workers.Serve(c.localWorkerListener) + s := &http.Server{ + ErrorLog: c.logger, + Handler: c.workers.Handler(), + } + err := s.Serve(c.localWorkerListener) if err != nil { panic(err) } @@ -180,7 +184,11 @@ func (c *Composer) Start() error { if c.workerListener != nil { go func() { - err := c.workers.Serve(c.workerListener) + s := &http.Server{ + ErrorLog: c.logger, + Handler: c.workers.Handler(), + } + err := s.Serve(c.workerListener) if err != nil { panic(err) } diff --git a/internal/kojiapi/server_test.go b/internal/kojiapi/server_test.go index c57b44c01..76c1bf99d 100644 --- a/internal/kojiapi/server_test.go +++ b/internal/kojiapi/server_test.go @@ -69,6 +69,7 @@ func TestCompose(t *testing.T) { kojiServer, workerServer := newTestKojiServer(t, dir) handler := kojiServer.Handler("/api/composer-koji/v1") + workerHandler := workerServer.Handler() type kojiCase struct { initResult worker.KojiInitJobResult @@ -252,7 +253,7 @@ func TestCompose(t *testing.T) { initJobResult, err := json.Marshal(&jobResult{Result: result}) require.NoError(t, err) - test.TestRoute(t, workerServer, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), string(initJobResult), http.StatusOK, `{}`) + test.TestRoute(t, workerHandler, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), string(initJobResult), http.StatusOK, `{}`) wg.Done() }(t, c.initResult) @@ -302,7 +303,7 @@ func TestCompose(t *testing.T) { buildJobResult, err := json.Marshal(&jobResult{Result: c.buildResult}) require.NoError(t, err) - test.TestRoute(t, workerServer, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), string(buildJobResult), http.StatusOK, `{}`) + test.TestRoute(t, workerHandler, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), string(buildJobResult), http.StatusOK, `{}`) token, _, jobType, rawJob, _, err = workerServer.RequestJob(context.Background(), "x86_64", []string{"osbuild-koji"}) require.NoError(t, err) @@ -314,7 +315,7 @@ func TestCompose(t *testing.T) { require.Equal(t, "test.img", osbuildJob.ImageName) require.NotEmpty(t, osbuildJob.KojiDirectory) - test.TestRoute(t, workerServer, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), `{ + test.TestRoute(t, workerHandler, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), `{ "result": { "arch": "x86_64", "host_os": "fedora-30", @@ -341,7 +342,7 @@ func TestCompose(t *testing.T) { finalizeResult, err := json.Marshal(&jobResult{Result: c.finalizeResult}) require.NoError(t, err) - test.TestRoute(t, workerServer, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), string(finalizeResult), http.StatusOK, `{}`) + test.TestRoute(t, workerHandler, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%v", token), string(finalizeResult), http.StatusOK, `{}`) test.TestRoute(t, handler, false, "GET", fmt.Sprintf("/api/composer-koji/v1/compose/%v", finalizeID), ``, http.StatusOK, c.composeStatus) } diff --git a/internal/worker/server.go b/internal/worker/server.go index baa2b7293..c81ac18bc 100644 --- a/internal/worker/server.go +++ b/internal/worker/server.go @@ -8,7 +8,6 @@ import ( "io" "io/ioutil" "log" - "net" "net/http" "os" "path" @@ -24,7 +23,7 @@ import ( type Server struct { jobs jobqueue.JobQueue - server *http.Server + logger *log.Logger artifactsDir string // Currently running jobs. Workers are not handed job ids, but @@ -50,37 +49,25 @@ type JobStatus struct { var ErrTokenNotExist = errors.New("worker token does not exist") func NewServer(logger *log.Logger, jobs jobqueue.JobQueue, artifactsDir string) *Server { - s := &Server{ + return &Server{ jobs: jobs, + logger: logger, artifactsDir: artifactsDir, running: make(map[uuid.UUID]uuid.UUID), } +} +func (s *Server) Handler() http.Handler { e := echo.New() e.Binder = binder{} - e.StdLogger = logger + e.StdLogger = s.logger - api.RegisterHandlers(e.Group(api.BasePath), &apiHandlers{s}) - - s.server = &http.Server{ - ErrorLog: logger, - Handler: e, + handler := apiHandlers{ + server: s, } + api.RegisterHandlers(e.Group(api.BasePath), &handler) - return s -} - -func (s *Server) Serve(listener net.Listener) error { - err := s.server.Serve(listener) - if err != nil && err != http.ErrServerClosed { - return err - } - - return nil -} - -func (s *Server) ServeHTTP(writer http.ResponseWriter, request *http.Request) { - s.server.Handler.ServeHTTP(writer, request) + return e } func (s *Server) EnqueueOSBuild(arch string, job *OSBuildJob) (uuid.UUID, error) { diff --git a/internal/worker/server_test.go b/internal/worker/server_test.go index 9f73d6457..864d3239b 100644 --- a/internal/worker/server_test.go +++ b/internal/worker/server_test.go @@ -18,7 +18,8 @@ import ( // Ensure that the status request returns OK. func TestStatus(t *testing.T) { server := worker.NewServer(nil, testjobqueue.New(), "") - test.TestRoute(t, server, false, "GET", "/api/worker/v1/status", ``, http.StatusOK, `{"status":"OK"}`, "message") + handler := server.Handler() + test.TestRoute(t, handler, false, "GET", "/api/worker/v1/status", ``, http.StatusOK, `{"status":"OK"}`, "message") } func TestErrors(t *testing.T) { @@ -44,7 +45,8 @@ func TestErrors(t *testing.T) { for _, c := range cases { server := worker.NewServer(nil, testjobqueue.New(), "") - test.TestRoute(t, server, false, c.Method, c.Path, c.Body, c.ExpectedStatus, "{}", "message") + handler := server.Handler() + test.TestRoute(t, handler, false, c.Method, c.Path, c.Body, c.ExpectedStatus, "{}", "message") } } @@ -63,11 +65,12 @@ func TestCreate(t *testing.T) { t.Fatalf("error creating osbuild manifest") } server := worker.NewServer(nil, testjobqueue.New(), "") + handler := server.Handler() _, err = server.EnqueueOSBuild(arch.Name(), &worker.OSBuildJob{Manifest: manifest}) require.NoError(t, err) - test.TestRoute(t, server, false, "POST", "/api/worker/v1/jobs", `{"types":["osbuild"],"arch":"x86_64"}`, http.StatusCreated, + test.TestRoute(t, handler, false, "POST", "/api/worker/v1/jobs", `{"types":["osbuild"],"arch":"x86_64"}`, http.StatusCreated, `{"type":"osbuild","args":{"manifest":{"pipeline":{},"sources":{}}}}`, "id", "location", "artifact_location") } @@ -86,6 +89,7 @@ func TestCancel(t *testing.T) { t.Fatalf("error creating osbuild manifest") } server := worker.NewServer(nil, testjobqueue.New(), "") + handler := server.Handler() jobId, err := server.EnqueueOSBuild(arch.Name(), &worker.OSBuildJob{Manifest: manifest}) require.NoError(t, err) @@ -97,13 +101,13 @@ func TestCancel(t *testing.T) { require.NotNil(t, args) require.Nil(t, dynamicArgs) - test.TestRoute(t, server, false, "GET", fmt.Sprintf("/api/worker/v1/jobs/%s", token), `{}`, http.StatusOK, + test.TestRoute(t, handler, false, "GET", fmt.Sprintf("/api/worker/v1/jobs/%s", token), `{}`, http.StatusOK, `{"canceled":false}`) err = server.Cancel(jobId) require.NoError(t, err) - test.TestRoute(t, server, false, "GET", fmt.Sprintf("/api/worker/v1/jobs/%s", token), `{}`, http.StatusOK, + test.TestRoute(t, handler, false, "GET", fmt.Sprintf("/api/worker/v1/jobs/%s", token), `{}`, http.StatusOK, `{"canceled":true}`) } @@ -122,6 +126,7 @@ func TestUpdate(t *testing.T) { t.Fatalf("error creating osbuild manifest") } server := worker.NewServer(nil, testjobqueue.New(), "") + handler := server.Handler() jobId, err := server.EnqueueOSBuild(arch.Name(), &worker.OSBuildJob{Manifest: manifest}) require.NoError(t, err) @@ -133,8 +138,8 @@ func TestUpdate(t *testing.T) { require.NotNil(t, args) require.Nil(t, dynamicArgs) - test.TestRoute(t, server, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%s", token), `{}`, http.StatusOK, `{}`) - test.TestRoute(t, server, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%s", token), `{}`, http.StatusNotFound, `*`) + test.TestRoute(t, handler, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%s", token), `{}`, http.StatusOK, `{}`) + test.TestRoute(t, handler, false, "PATCH", fmt.Sprintf("/api/worker/v1/jobs/%s", token), `{}`, http.StatusNotFound, `*`) } func TestUpload(t *testing.T) { @@ -152,6 +157,7 @@ func TestUpload(t *testing.T) { t.Fatalf("error creating osbuild manifest") } server := worker.NewServer(nil, testjobqueue.New(), "") + handler := server.Handler() jobID, err := server.EnqueueOSBuild(arch.Name(), &worker.OSBuildJob{Manifest: manifest}) require.NoError(t, err) @@ -163,5 +169,5 @@ func TestUpload(t *testing.T) { require.NotNil(t, args) require.Nil(t, dynamicArgs) - test.TestRoute(t, server, false, "PUT", fmt.Sprintf("/api/worker/v1/jobs/%s/artifacts/foobar", token), `this is my artifact`, http.StatusOK, `?`) + test.TestRoute(t, handler, false, "PUT", fmt.Sprintf("/api/worker/v1/jobs/%s/artifacts/foobar", token), `this is my artifact`, http.StatusOK, `?`) }