store: move queue out of the store
The store is responsible for two things: user state and the compose queue. This is problematic, because the rcm API has slightly different semantics from weldr and only used the queue part of the store. Also, the store is simply too complex. This commit splits the queue part out, using the new jobqueue package in both the weldr and the rcm package. The queue is saved to a new directory `queue/`. The weldr package now also has access to a worker server to enqueue and list jobs. Its store continues to track composes, but the `QueueStatus` for each compose (and image build) is deprecated. The field in `ImageBuild` is kept for backwards compatibility for composes which finished before this change, but a lot of code dealing with it in package compose is dropped. store.PushCompose() is degraded to storing a new compose. It should probably be renamed in the future. store.PopJob() is removed. Job ids are now independent of compose ids. Because of that, the local target gains ComposeId and ImageBuildId fields, because a worker cannot infer those from a job anymore. This also necessitates a change in the worker API: the job routes are changed to expect that instead of a (compose id, image build id) pair. The route that accepts built images keeps that pair, because it reports the image back to weldr. worker.Server() now interacts with a job queue instead of the store. It gains public functions that allow enqueuing an osbuild job and getting its status, because only it knows about the specific argument and result types in the job queue (OSBuildJob and OSBuildJobResult). One oddity remains: it needs to report an uploaded image to weldr. Do this with a function that's passed in for now, so that the dependency to the store can be dropped completely. The rcm API drops its dependencies to package blueprint and store, because it too interacts only with the worker server now. Fixes #342
This commit is contained in:
Lars Karlitski
Tom Gundersen
parent
64011e3cba
commit
b5769add2c
18 changed files with 415 additions and 443 deletions
|
|
@ -25,10 +25,9 @@ type Client struct {
|
|||
}
|
||||
|
||||
type Job struct {
|
||||
ComposeID uuid.UUID
|
||||
ImageBuildID int
|
||||
Manifest *osbuild.Manifest
|
||||
Targets []*target.Target
|
||||
Id uuid.UUID
|
||||
Manifest *osbuild.Manifest
|
||||
Targets []*target.Target
|
||||
}
|
||||
|
||||
func NewClient(address string, conf *tls.Config) *Client {
|
||||
|
|
@ -86,8 +85,7 @@ func (c *Client) AddJob() (*Job, error) {
|
|||
}
|
||||
|
||||
return &Job{
|
||||
jr.ComposeID,
|
||||
jr.ImageBuildID,
|
||||
jr.Id,
|
||||
jr.Manifest,
|
||||
jr.Targets,
|
||||
}, nil
|
||||
|
|
@ -99,7 +97,7 @@ func (c *Client) UpdateJob(job *Job, status common.ImageBuildState, result *comm
|
|||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
urlPath := fmt.Sprintf("/job-queue/v1/jobs/%s/builds/%d", job.ComposeID.String(), job.ImageBuildID)
|
||||
urlPath := fmt.Sprintf("/job-queue/v1/jobs/%s", job.Id)
|
||||
url := c.createURL(urlPath)
|
||||
req, err := http.NewRequest("PATCH", url, &b)
|
||||
if err != nil {
|
||||
|
|
@ -120,9 +118,9 @@ func (c *Client) UpdateJob(job *Job, status common.ImageBuildState, result *comm
|
|||
return nil
|
||||
}
|
||||
|
||||
func (c *Client) UploadImage(job *Job, reader io.Reader) error {
|
||||
func (c *Client) UploadImage(composeId uuid.UUID, imageBuildId int, reader io.Reader) error {
|
||||
// content type doesn't really matter
|
||||
url := c.createURL(fmt.Sprintf("/job-queue/v1/jobs/%s/builds/%d/image", job.ComposeID.String(), job.ImageBuildID))
|
||||
url := c.createURL(fmt.Sprintf("/job-queue/v1/jobs/%s/builds/%d/image", composeId, imageBuildId))
|
||||
_, err := c.client.Post(url, "application/octet-stream", reader)
|
||||
|
||||
return err
|
||||
|
|
|
|||
|
|
@ -8,6 +8,23 @@ import (
|
|||
"github.com/osbuild/osbuild-composer/internal/target"
|
||||
)
|
||||
|
||||
//
|
||||
// JSON-serializable types for the jobqueue
|
||||
//
|
||||
|
||||
type OSBuildJob struct {
|
||||
Manifest *osbuild.Manifest `json:"manifest"`
|
||||
Targets []*target.Target `json:"targets,omitempty"`
|
||||
}
|
||||
|
||||
type OSBuildJobResult struct {
|
||||
OSBuildOutput *common.ComposeResult `json:"osbuild_output,omitempty"`
|
||||
}
|
||||
|
||||
//
|
||||
// JSON-serializable types for the HTTP API
|
||||
//
|
||||
|
||||
type errorResponse struct {
|
||||
Message string `json:"message"`
|
||||
}
|
||||
|
|
@ -16,10 +33,9 @@ type addJobRequest struct {
|
|||
}
|
||||
|
||||
type addJobResponse struct {
|
||||
ComposeID uuid.UUID `json:"compose_id"`
|
||||
ImageBuildID int `json:"image_build_id"`
|
||||
Manifest *osbuild.Manifest `json:"manifest"`
|
||||
Targets []*target.Target `json:"targets"`
|
||||
Id uuid.UUID `json:"id"`
|
||||
Manifest *osbuild.Manifest `json:"manifest"`
|
||||
Targets []*target.Target `json:"targets,omitempty"`
|
||||
}
|
||||
|
||||
type updateJobRequest struct {
|
||||
|
|
|
|||
|
|
@ -3,27 +3,37 @@ package worker
|
|||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"io/ioutil"
|
||||
"log"
|
||||
"net"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/julienschmidt/httprouter"
|
||||
|
||||
"github.com/osbuild/osbuild-composer/internal/store"
|
||||
"github.com/osbuild/osbuild-composer/internal/common"
|
||||
"github.com/osbuild/osbuild-composer/internal/jobqueue"
|
||||
"github.com/osbuild/osbuild-composer/internal/osbuild"
|
||||
"github.com/osbuild/osbuild-composer/internal/target"
|
||||
)
|
||||
|
||||
type Server struct {
|
||||
logger *log.Logger
|
||||
store *store.Store
|
||||
router *httprouter.Router
|
||||
logger *log.Logger
|
||||
jobs jobqueue.JobQueue
|
||||
router *httprouter.Router
|
||||
imageWriter WriteImageFunc
|
||||
}
|
||||
|
||||
func NewServer(logger *log.Logger, store *store.Store) *Server {
|
||||
type WriteImageFunc func(composeID uuid.UUID, imageBuildID int, reader io.Reader) error
|
||||
|
||||
func NewServer(logger *log.Logger, jobs jobqueue.JobQueue, imageWriter WriteImageFunc) *Server {
|
||||
s := &Server{
|
||||
logger: logger,
|
||||
store: store,
|
||||
logger: logger,
|
||||
jobs: jobs,
|
||||
imageWriter: imageWriter,
|
||||
}
|
||||
|
||||
s.router = httprouter.New()
|
||||
|
|
@ -33,7 +43,7 @@ func NewServer(logger *log.Logger, store *store.Store) *Server {
|
|||
s.router.NotFound = http.HandlerFunc(notFoundHandler)
|
||||
|
||||
s.router.POST("/job-queue/v1/jobs", s.addJobHandler)
|
||||
s.router.PATCH("/job-queue/v1/jobs/:job_id/builds/:build_id", s.updateJobHandler)
|
||||
s.router.PATCH("/job-queue/v1/jobs/:job_id", s.updateJobHandler)
|
||||
s.router.POST("/job-queue/v1/jobs/:job_id/builds/:build_id/image", s.addJobImageHandler)
|
||||
|
||||
return s
|
||||
|
|
@ -59,6 +69,38 @@ func (s *Server) ServeHTTP(writer http.ResponseWriter, request *http.Request) {
|
|||
s.router.ServeHTTP(writer, request)
|
||||
}
|
||||
|
||||
func (s *Server) Enqueue(manifest *osbuild.Manifest, targets []*target.Target) (uuid.UUID, error) {
|
||||
job := OSBuildJob{
|
||||
Manifest: manifest,
|
||||
Targets: targets,
|
||||
}
|
||||
|
||||
return s.jobs.Enqueue("osbuild", job, nil)
|
||||
}
|
||||
|
||||
func (s *Server) JobStatus(id uuid.UUID) (state common.ComposeState, queued, started, finished time.Time, err error) {
|
||||
var result OSBuildJobResult
|
||||
var status jobqueue.JobStatus
|
||||
|
||||
status, queued, started, finished, err = s.jobs.JobStatus(id, &result)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
|
||||
state = composeStateFromJobStatus(status, result.OSBuildOutput)
|
||||
return
|
||||
}
|
||||
|
||||
func (s *Server) JobResult(id uuid.UUID) (common.ComposeState, *common.ComposeResult, error) {
|
||||
var result OSBuildJobResult
|
||||
status, _, _, _, err := s.jobs.JobStatus(id, &result)
|
||||
if err != nil {
|
||||
return common.CWaiting, nil, err
|
||||
}
|
||||
|
||||
return composeStateFromJobStatus(status, result.OSBuildOutput), result.OSBuildOutput, nil
|
||||
}
|
||||
|
||||
// jsonErrorf() is similar to http.Error(), but returns the message in a json
|
||||
// object with a "message" field.
|
||||
func jsonErrorf(writer http.ResponseWriter, code int, message string, args ...interface{}) {
|
||||
|
|
@ -92,15 +134,19 @@ func (s *Server) addJobHandler(writer http.ResponseWriter, request *http.Request
|
|||
return
|
||||
}
|
||||
|
||||
nextJob := s.store.PopJob()
|
||||
var job OSBuildJob
|
||||
id, err := s.jobs.Dequeue(request.Context(), []string{"osbuild"}, &job)
|
||||
if err != nil {
|
||||
jsonErrorf(writer, http.StatusInternalServerError, "%v", err)
|
||||
return
|
||||
}
|
||||
|
||||
writer.WriteHeader(http.StatusCreated)
|
||||
// FIXME: handle or comment this possible error
|
||||
_ = json.NewEncoder(writer).Encode(addJobResponse{
|
||||
ComposeID: nextJob.ComposeID,
|
||||
ImageBuildID: nextJob.ImageBuildID,
|
||||
Manifest: nextJob.Manifest,
|
||||
Targets: nextJob.Targets,
|
||||
Id: id,
|
||||
Manifest: job.Manifest,
|
||||
Targets: job.Targets,
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -117,13 +163,6 @@ func (s *Server) updateJobHandler(writer http.ResponseWriter, request *http.Requ
|
|||
return
|
||||
}
|
||||
|
||||
imageBuildId, err := strconv.Atoi(params.ByName("build_id"))
|
||||
|
||||
if err != nil {
|
||||
jsonErrorf(writer, http.StatusBadRequest, "cannot parse image build id: %v", err)
|
||||
return
|
||||
}
|
||||
|
||||
var body updateJobRequest
|
||||
err = json.NewDecoder(request.Body).Decode(&body)
|
||||
if err != nil {
|
||||
|
|
@ -131,13 +170,21 @@ func (s *Server) updateJobHandler(writer http.ResponseWriter, request *http.Requ
|
|||
return
|
||||
}
|
||||
|
||||
err = s.store.UpdateImageBuildInCompose(id, imageBuildId, body.Status, body.Result)
|
||||
// The jobqueue doesn't support setting the status before a job is
|
||||
// finished. This branch should never be hit, because the worker
|
||||
// doesn't attempt this. Change the API to remove this awkwardness.
|
||||
if body.Status != common.IBFinished && body.Status != common.IBFailed {
|
||||
jsonErrorf(writer, http.StatusBadRequest, "setting status of a job to waiting or running is not supported")
|
||||
return
|
||||
}
|
||||
|
||||
err = s.jobs.FinishJob(id, OSBuildJobResult{OSBuildOutput: body.Result})
|
||||
if err != nil {
|
||||
switch err.(type) {
|
||||
case *store.NotFoundError, *store.NotPendingError:
|
||||
jsonErrorf(writer, http.StatusNotFound, "%v", err)
|
||||
case *store.NotRunningError, *store.InvalidRequestError:
|
||||
jsonErrorf(writer, http.StatusBadRequest, "%v", err)
|
||||
switch err {
|
||||
case jobqueue.ErrNotExist:
|
||||
jsonErrorf(writer, http.StatusNotFound, "job does not exist: %s", id)
|
||||
case jobqueue.ErrNotRunning:
|
||||
jsonErrorf(writer, http.StatusBadRequest, "job is not running: %s", id)
|
||||
default:
|
||||
jsonErrorf(writer, http.StatusInternalServerError, "%v", err)
|
||||
}
|
||||
|
|
@ -155,23 +202,33 @@ func (s *Server) addJobImageHandler(writer http.ResponseWriter, request *http.Re
|
|||
}
|
||||
|
||||
imageBuildId, err := strconv.Atoi(params.ByName("build_id"))
|
||||
|
||||
if err != nil {
|
||||
jsonErrorf(writer, http.StatusBadRequest, "cannot parse image build id: %v", err)
|
||||
return
|
||||
}
|
||||
|
||||
err = s.store.AddImageToImageUpload(id, imageBuildId, request.Body)
|
||||
|
||||
if s.imageWriter == nil {
|
||||
_, err = io.Copy(ioutil.Discard, request.Body)
|
||||
} else {
|
||||
err = s.imageWriter(id, imageBuildId, request.Body)
|
||||
}
|
||||
if err != nil {
|
||||
switch err.(type) {
|
||||
case *store.NotFoundError:
|
||||
jsonErrorf(writer, http.StatusNotFound, "%v", err)
|
||||
case *store.NoLocalTargetError:
|
||||
jsonErrorf(writer, http.StatusBadRequest, "%v", err)
|
||||
default:
|
||||
jsonErrorf(writer, http.StatusInternalServerError, "%v", err)
|
||||
}
|
||||
return
|
||||
jsonErrorf(writer, http.StatusInternalServerError, "%v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func composeStateFromJobStatus(status jobqueue.JobStatus, output *common.ComposeResult) common.ComposeState {
|
||||
switch status {
|
||||
case jobqueue.JobPending:
|
||||
return common.CWaiting
|
||||
case jobqueue.JobRunning:
|
||||
return common.CRunning
|
||||
case jobqueue.JobFinished:
|
||||
if output.Success {
|
||||
return common.CFinished
|
||||
} else {
|
||||
return common.CFailed
|
||||
}
|
||||
}
|
||||
return common.CWaiting
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,10 +5,10 @@ import (
|
|||
"testing"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/osbuild/osbuild-composer/internal/blueprint"
|
||||
"github.com/osbuild/osbuild-composer/internal/distro/fedoratest"
|
||||
"github.com/osbuild/osbuild-composer/internal/store"
|
||||
"github.com/osbuild/osbuild-composer/internal/jobqueue/testjobqueue"
|
||||
"github.com/osbuild/osbuild-composer/internal/test"
|
||||
"github.com/osbuild/osbuild-composer/internal/worker"
|
||||
)
|
||||
|
|
@ -27,15 +27,15 @@ func TestErrors(t *testing.T) {
|
|||
// Wrong method
|
||||
{"GET", "/job-queue/v1/jobs", ``, http.StatusMethodNotAllowed},
|
||||
// Update job with invalid ID
|
||||
{"PATCH", "/job-queue/v1/jobs/foo/builds/0", `{"status":"RUNNING"}`, http.StatusBadRequest},
|
||||
{"PATCH", "/job-queue/v1/jobs/foo", `{"status":"FINISHED"}`, http.StatusBadRequest},
|
||||
// Update job that does not exist, with invalid body
|
||||
{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa/builds/0", ``, http.StatusBadRequest},
|
||||
{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", ``, http.StatusBadRequest},
|
||||
// Update job that does not exist
|
||||
{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa/builds/0", `{"status":"RUNNING"}`, http.StatusNotFound},
|
||||
{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", `{"status":"FINISHED"}`, http.StatusNotFound},
|
||||
}
|
||||
|
||||
for _, c := range cases {
|
||||
server := worker.NewServer(nil, store.New(nil))
|
||||
server := worker.NewServer(nil, testjobqueue.New(), nil)
|
||||
test.TestRoute(t, server, false, c.Method, c.Path, c.Body, c.ExpectedStatus, "{}", "message")
|
||||
}
|
||||
}
|
||||
|
|
@ -50,21 +50,18 @@ func TestCreate(t *testing.T) {
|
|||
if err != nil {
|
||||
t.Fatalf("error getting image type from arch")
|
||||
}
|
||||
store := store.New(nil)
|
||||
server := worker.NewServer(nil, store)
|
||||
server := worker.NewServer(nil, testjobqueue.New(), nil)
|
||||
|
||||
manifest, err := imageType.Manifest(nil, nil, nil, nil, imageType.Size(0))
|
||||
if err != nil {
|
||||
t.Fatalf("error creating osbuild manifest")
|
||||
}
|
||||
|
||||
id, err := store.PushCompose(manifest, imageType.Name(), &blueprint.Blueprint{}, 0, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("error pushing compose: %v", err)
|
||||
}
|
||||
id, err := server.Enqueue(manifest, nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
test.TestRoute(t, server, false, "POST", "/job-queue/v1/jobs", `{}`, http.StatusCreated,
|
||||
`{"compose_id":"`+id.String()+`","image_build_id":0,"manifest":{"sources":{},"pipeline":{}},"targets":[]}`, "created")
|
||||
`{"id":"`+id.String()+`","manifest":{"sources":{},"pipeline":{}}}`, "created")
|
||||
}
|
||||
|
||||
func testUpdateTransition(t *testing.T, from, to string, expectedStatus int) {
|
||||
|
|
@ -77,8 +74,7 @@ func testUpdateTransition(t *testing.T, from, to string, expectedStatus int) {
|
|||
if err != nil {
|
||||
t.Fatalf("error getting image type from arch")
|
||||
}
|
||||
store := store.New(nil)
|
||||
server := worker.NewServer(nil, store)
|
||||
server := worker.NewServer(nil, testjobqueue.New(), nil)
|
||||
|
||||
id := uuid.Nil
|
||||
if from != "VOID" {
|
||||
|
|
@ -87,20 +83,18 @@ func testUpdateTransition(t *testing.T, from, to string, expectedStatus int) {
|
|||
t.Fatalf("error creating osbuild manifest")
|
||||
}
|
||||
|
||||
id, err = store.PushCompose(manifest, imageType.Name(), &blueprint.Blueprint{}, 0, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("error pushing compose: %v", err)
|
||||
}
|
||||
id, err = server.Enqueue(manifest, nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
if from != "WAITING" {
|
||||
test.SendHTTP(server, false, "POST", "/job-queue/v1/jobs", `{}`)
|
||||
if from != "RUNNING" {
|
||||
test.SendHTTP(server, false, "PATCH", "/job-queue/v1/jobs/"+id.String()+"/builds/0", `{"status":"`+from+`"}`)
|
||||
test.SendHTTP(server, false, "PATCH", "/job-queue/v1/jobs/"+id.String(), `{"status":"`+from+`"}`)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
test.TestRoute(t, server, false, "PATCH", "/job-queue/v1/jobs/"+id.String()+"/builds/0", `{"status":"`+to+`"}`, expectedStatus, "{}", "message")
|
||||
test.TestRoute(t, server, false, "PATCH", "/job-queue/v1/jobs/"+id.String(), `{"status":"`+to+`"}`, expectedStatus, "{}", "message")
|
||||
}
|
||||
|
||||
func TestUpdate(t *testing.T) {
|
||||
|
|
@ -109,16 +103,16 @@ func TestUpdate(t *testing.T) {
|
|||
To string
|
||||
ExpectedStatus int
|
||||
}{
|
||||
{"VOID", "WAITING", http.StatusNotFound},
|
||||
{"VOID", "RUNNING", http.StatusNotFound},
|
||||
{"VOID", "WAITING", http.StatusBadRequest},
|
||||
{"VOID", "RUNNING", http.StatusBadRequest},
|
||||
{"VOID", "FINISHED", http.StatusNotFound},
|
||||
{"VOID", "FAILED", http.StatusNotFound},
|
||||
{"WAITING", "WAITING", http.StatusNotFound},
|
||||
{"WAITING", "RUNNING", http.StatusNotFound},
|
||||
{"WAITING", "FINISHED", http.StatusNotFound},
|
||||
{"WAITING", "FAILED", http.StatusNotFound},
|
||||
{"WAITING", "WAITING", http.StatusBadRequest},
|
||||
{"WAITING", "RUNNING", http.StatusBadRequest},
|
||||
{"WAITING", "FINISHED", http.StatusBadRequest},
|
||||
{"WAITING", "FAILED", http.StatusBadRequest},
|
||||
{"RUNNING", "WAITING", http.StatusBadRequest},
|
||||
{"RUNNING", "RUNNING", http.StatusOK},
|
||||
{"RUNNING", "RUNNING", http.StatusBadRequest},
|
||||
{"RUNNING", "FINISHED", http.StatusOK},
|
||||
{"RUNNING", "FAILED", http.StatusOK},
|
||||
{"FINISHED", "WAITING", http.StatusBadRequest},
|
||||
|
|
@ -132,6 +126,7 @@ func TestUpdate(t *testing.T) {
|
|||
}
|
||||
|
||||
for _, c := range cases {
|
||||
t.Log(c)
|
||||
testUpdateTransition(t, c.From, c.To, c.ExpectedStatus)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue