worker/api: introduce job tokens

Don't give out job ids to workers, but `tokens`, which serve as an
indirection. This way, restarting composer won't confuse it when a stray
worker returns a result for a job that was still running. Also,
artifacts are only moved to the final location once a job finishes.

This change breaks backwards compatibility, but we're not yet promising
a stable worker API to anyone.

This drops the transition tests in server_test.go. These don't make much
sense anymore, because there's only one allowed transition, from running
to finished. They heavily relied on job slot ids, which are not easily
accessible with the `TestRoute` API. Overall, adjusting this seemed like
too much work for their benefit.
This commit is contained in:
Lars Karlitski 2020-09-07 13:12:06 +02:00 committed by Tom Gundersen
parent 783a88d8cc
commit 26b36ba704
7 changed files with 319 additions and 322 deletions

View file

@ -1,11 +1,11 @@
package worker_test
import (
"context"
"fmt"
"net/http"
"testing"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
"github.com/osbuild/osbuild-composer/internal/distro"
@ -64,14 +64,11 @@ func TestCreate(t *testing.T) {
}
server := worker.NewServer(nil, testjobqueue.New(), "")
id, err := server.Enqueue(manifest, nil)
_, err = server.Enqueue(manifest, nil)
require.NoError(t, err)
test.TestRoute(t, server, false, "POST", "/jobs", `{}`, http.StatusCreated,
`{"id":"`+id.String()+`","manifest":{"sources":{},"pipeline":{}}}`, "created")
test.TestRoute(t, server, false, "GET", fmt.Sprintf("/jobs/%s", id), `{}`, http.StatusOK,
`{"id":"`+id.String()+`","canceled":false}`)
`{"manifest":{"sources":{},"pipeline":{}}}`, "token", "created")
}
func TestCancel(t *testing.T) {
@ -90,82 +87,15 @@ func TestCancel(t *testing.T) {
}
server := worker.NewServer(nil, testjobqueue.New(), "")
id, err := server.Enqueue(manifest, nil)
jobId, err := server.Enqueue(manifest, nil)
require.NoError(t, err)
test.TestRoute(t, server, false, "POST", "/jobs", `{}`, http.StatusCreated,
`{"id":"`+id.String()+`","manifest":{"sources":{},"pipeline":{}}}`, "created")
err = server.Cancel(id)
token, _, err := server.RequestJob(context.Background())
require.NoError(t, err)
test.TestRoute(t, server, false, "GET", fmt.Sprintf("/jobs/%s", id), `{}`, http.StatusOK,
`{"id":"`+id.String()+`","canceled":true}`)
}
func testUpdateTransition(t *testing.T, from, to string, expectedStatus int) {
distroStruct := fedoratest.New()
arch, err := distroStruct.GetArch("x86_64")
if err != nil {
t.Fatalf("error getting arch from distro")
}
imageType, err := arch.GetImageType("qcow2")
if err != nil {
t.Fatalf("error getting image type from arch")
}
server := worker.NewServer(nil, testjobqueue.New(), "")
id := uuid.Nil
if from != "VOID" {
manifest, err := imageType.Manifest(nil, distro.ImageOptions{Size: imageType.Size(0)}, nil, nil, nil)
if err != nil {
t.Fatalf("error creating osbuild manifest")
}
id, err = server.Enqueue(manifest, nil)
require.NoError(t, err)
if from != "WAITING" {
test.SendHTTP(server, false, "POST", "/jobs", `{}`)
if from != "RUNNING" {
test.SendHTTP(server, false, "PATCH", "/jobs/"+id.String(), `{"status":"`+from+`"}`)
}
}
}
test.TestRoute(t, server, false, "PATCH", "/jobs/"+id.String(), `{"status":"`+to+`"}`, expectedStatus, "{}", "message")
}
func TestUpdate(t *testing.T) {
var cases = []struct {
From string
To string
ExpectedStatus int
}{
{"VOID", "WAITING", http.StatusBadRequest},
{"VOID", "RUNNING", http.StatusBadRequest},
{"VOID", "FINISHED", http.StatusNotFound},
{"VOID", "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.StatusBadRequest},
{"RUNNING", "FINISHED", http.StatusOK},
{"RUNNING", "FAILED", http.StatusOK},
{"FINISHED", "WAITING", http.StatusBadRequest},
{"FINISHED", "RUNNING", http.StatusBadRequest},
{"FINISHED", "FINISHED", http.StatusBadRequest},
{"FINISHED", "FAILED", http.StatusBadRequest},
{"FAILED", "WAITING", http.StatusBadRequest},
{"FAILED", "RUNNING", http.StatusBadRequest},
{"FAILED", "FINISHED", http.StatusBadRequest},
{"FAILED", "FAILED", http.StatusBadRequest},
}
for _, c := range cases {
t.Log(c)
testUpdateTransition(t, c.From, c.To, c.ExpectedStatus)
}
err = server.Cancel(jobId)
require.NoError(t, err)
test.TestRoute(t, server, false, "GET", fmt.Sprintf("/jobs/%s", token), `{}`, http.StatusOK,
`{"canceled":true}`)
}