worker/server: move it to the style of koji server

The previous code was smelling a bit (e.g. Server.server field) so I decided
to rewrite it in the style of the much nicer koji server.

Not a functional change.

Signed-off-by: Ondřej Budai <ondrej@budai.cz>
This commit is contained in:
Ondřej Budai 2020-11-19 10:09:39 +01:00 committed by Tom Gundersen
parent 2dff7d0529
commit 978e309153
4 changed files with 39 additions and 37 deletions

View file

@ -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)
}

View file

@ -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)
}

View file

@ -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) {

View file

@ -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, `?`)
}