diff --git a/docs/jobs.md b/docs/jobs.md index 6c954b5..c777744 100644 --- a/docs/jobs.md +++ b/docs/jobs.md @@ -114,6 +114,15 @@ the cluster for a plan and shows it. If the cluster cannot plan the file, for example because of a syntax error, urga opens the editor again with your edit and the reason at the top. See [Editor](configuration.md#editor). +An edit keeps the count a group runs with. If the autoscaler or `s` scaled a +group to 5 and you leave its `count` in the file alone, the job is submitted +with 5, not with the number in the file. If you change the `count` in the +editor, the number you wrote is used. A group the job does not run yet gets +the count of the file. The plan lists the counts it kept above what will +happen. When the cluster cannot read the counts of the file as it was +opened, for example a file that calls `file()`, the editor still opens, and +the plan says that every count is the one in the file. + ## Creating a job `n` opens a template of a job in your editor: a service with one group `web` diff --git a/internal/nomad/edit.go b/internal/nomad/edit.go index e64b0b8..19fe93d 100644 --- a/internal/nomad/edit.go +++ b/internal/nomad/edit.go @@ -21,9 +21,10 @@ import ( // makes, and so are the values of its variables: the next edit opens the same // file, and the job runs with the values it ran with before. It is submitted // at the index of its plan: a job that changed since is refused rather than -// overwritten. -func (c *Client) SubmitJob(ctx context.Context, namespace, source string, vars JobVariables, index uint64) error { - job, kept, err := c.jobOf(ctx, namespace, source, vars) +// overwritten. The counts it opened with decide which counts the job keeps, +// see keepCounts. +func (c *Client) SubmitJob(ctx context.Context, namespace, source string, vars JobVariables, index uint64, opened map[string]int) error { + job, kept, _, err := c.jobOf(ctx, namespace, source, vars, opened) if err != nil { return err } @@ -38,18 +39,95 @@ func (c *Client) SubmitJob(ctx context.Context, namespace, source string, vars J return err } -// jobOf reads a job file and puts the job in the namespace it is sent to. -func (c *Client) jobOf(ctx context.Context, namespace, source string, vars JobVariables) (*api.Job, *api.JobSubmission, error) { - job, kept, err := c.parseJob(ctx, namespace, source, vars, true) +// jobOf reads a job file and puts the job in the namespace it is sent to, +// with the counts it keeps from the job that runs. +func (c *Client) jobOf(ctx context.Context, namespace, source string, vars JobVariables, opened map[string]int) (*api.Job, *api.JobSubmission, []KeptCount, error) { + job, submission, err := c.parseJob(ctx, namespace, source, vars, true) if err != nil { - return nil, nil, err + return nil, nil, nil, err } if namespace != "" && namespace != AllNamespaces { job.Namespace = &namespace } - return job, kept, nil + kept, err := c.keepCounts(ctx, namespace, job, opened) + + return job, submission, kept, err +} + +// KeptCount is a group that keeps the count its job runs with, though the +// file says another. +type KeptCount struct { + Group string + Kept int + File int +} + +// keepCounts sets the count of each group of an edited job. A group whose +// count is as in the file when it was opened keeps the count the job runs it +// with, which the autoscaler or a person may have changed since. A group +// whose count was edited, a new group, and a job that is not running take +// the count of the file. Nil opened counts keep nothing. +func (c *Client) keepCounts(ctx context.Context, namespace string, job *api.Job, opened map[string]int) ([]KeptCount, error) { + if opened == nil { + return nil, nil + } + + running, _, err := c.api.Jobs().Info(valueOf(job.ID), c.query(ctx, namespace)) + if answered(err, http.StatusNotFound) { + return nil, nil + } + + if err != nil { + return nil, err + } + + var kept []KeptCount + + for _, group := range job.TaskGroups { + name := valueOf(group.Name) + file := countOf(group) + + was, wasOpened := opened[name] + now := slices.IndexFunc(running.TaskGroups, func(g *api.TaskGroup) bool { return valueOf(g.Name) == name }) + + if !wasOpened || was != file || now < 0 { + continue + } + + if count := countOf(running.TaskGroups[now]); count != file { + group.Count = &count + kept = append(kept, KeptCount{Group: name, Kept: count, File: file}) + } + } + + return kept, nil +} + +// countOf is the count of a group, which is 1 when the file leaves it out. +func countOf(group *api.TaskGroup) int { + if group.Count == nil { + return 1 + } + + return *group.Count +} + +// JobCounts reads the count of each group of a job file, the way a plan of +// it would. +func (c *Client) JobCounts(ctx context.Context, namespace, source string, vars JobVariables) (map[string]int, error) { + job, _, _, err := c.jobOf(ctx, namespace, source, vars, nil) + if err != nil { + return nil, err + } + + counts := make(map[string]int, len(job.TaskGroups)) + for _, group := range job.TaskGroups { + counts[valueOf(group.Name)] = countOf(group) + } + + return counts, nil } // parseJob reads a job file the way the cluster does, and returns what of it diff --git a/internal/nomad/edit_test.go b/internal/nomad/edit_test.go index 384355c..c44505b 100644 --- a/internal/nomad/edit_test.go +++ b/internal/nomad/edit_test.go @@ -81,7 +81,7 @@ func TestSubmitJob_JSON(t *testing.T) { client, asked := clusterServer(t, map[string]string{"/v1/jobs": `{"EvalID": "eval-1"}`}) source := `{"ID": "web", "Name": "web"}` - r.NoError(client.SubmitJob(context.Background(), "production", source, nomad.JobVariables{}, 0)) + r.NoError(client.SubmitJob(context.Background(), "production", source, nomad.JobVariables{}, 0, nil)) // A file that is already JSON goes straight to the cluster, and is saved // with the new version: the next edit opens it again. @@ -98,7 +98,7 @@ func TestSubmitJob_JSONUnderAJobKey(t *testing.T) { client, asked := clusterServer(t, map[string]string{"/v1/jobs": `{"EvalID": "eval-1"}`}) source := `{"Job": {"ID": "web", "Name": "web"}}` - r.NoError(client.SubmitJob(context.Background(), "production", source, nomad.JobVariables{}, 0)) + r.NoError(client.SubmitJob(context.Background(), "production", source, nomad.JobVariables{}, 0, nil)) // A JSON job file holds the job or wraps it in a Job key, and the nomad // command keeps either one as it was written. Read as the job itself, @@ -118,7 +118,7 @@ func TestSubmitJob_HCLGoesThroughTheCluster(t *testing.T) { "/v1/jobs": `{"EvalID": "eval-1"}`, }) - r.NoError(client.SubmitJob(context.Background(), "production", "job \"web\" {}", nomad.JobVariables{}, 0)) + r.NoError(client.SubmitJob(context.Background(), "production", "job \"web\" {}", nomad.JobVariables{}, 0, nil)) // HCL is read by Nomad itself, urga does not parse job files. r.Len(*asked, 2) @@ -134,7 +134,7 @@ func TestSubmitJob_ParsesInTheNamespace(t *testing.T) { "/v1/jobs": `{"EvalID": "eval-1"}`, }) - r.NoError(client.SubmitJob(context.Background(), "production", "job \"web\" {}", nomad.JobVariables{}, 0)) + r.NoError(client.SubmitJob(context.Background(), "production", "job \"web\" {}", nomad.JobVariables{}, 0, nil)) // A token may parse jobs in its own namespace only. Asking without one // asks in the default namespace, where it is refused. @@ -155,7 +155,7 @@ func TestSubmitJob_HCLKeepsTheSourceAndVariables(t *testing.T) { File: "count = 3\n", } - r.NoError(client.SubmitJob(context.Background(), "production", source, vars, 0)) + r.NoError(client.SubmitJob(context.Background(), "production", source, vars, 0, nil)) // The cluster reads variables only as a file, so the flags are written // into one next to the file the job was run with. @@ -181,7 +181,7 @@ func TestSubmitJob_FlagValuesStayText(t *testing.T) { vars := nomad.JobVariables{Flags: map[string]string{"greeting": "say \"hi\"\n${name} %{ok}\\"}} - r.NoError(client.SubmitJob(context.Background(), "production", "job \"web\" {}", vars, 0)) + r.NoError(client.SubmitJob(context.Background(), "production", "job \"web\" {}", vars, 0, nil)) // A flag was typed as text. Written into HCL as it is, a quote ends the // string early and ${ reads another variable instead of the characters. @@ -193,7 +193,7 @@ func TestSubmitJob_BrokenJSON(t *testing.T) { client, _ := recorder(t, `{}`) - err := client.SubmitJob(context.Background(), "production", "{not json", nomad.JobVariables{}, 0) + err := client.SubmitJob(context.Background(), "production", "{not json", nomad.JobVariables{}, 0, nil) r.ErrorContains(err, "not valid JSON") } @@ -308,7 +308,7 @@ func variablesSent(t *testing.T, vars nomad.JobVariables) string { "/v1/jobs": `{"EvalID": "eval-1"}`, }) - require.NoError(t, client.SubmitJob(context.Background(), "production", "job \"web\" {}", vars, 0)) + require.NoError(t, client.SubmitJob(context.Background(), "production", "job \"web\" {}", vars, 0, nil)) text, ok := (*asked)[0].body["Variables"].(string) require.True(t, ok) @@ -386,7 +386,7 @@ func TestSubmitJob_VariablesFileThatDoesNotParse(t *testing.T) { client, _ := recorder(t, `{}`) err := client.SubmitJob(context.Background(), "production", "job \"web\" {}", - nomad.JobVariables{Flags: map[string]string{"a": "b"}, File: "count = = 3"}, 0) + nomad.JobVariables{Flags: map[string]string{"a": "b"}, File: "count = = 3"}, 0, nil) r.ErrorContains(err, "the variables file of the job does not parse") } @@ -407,7 +407,7 @@ func TestSubmitJob_JSONVariablesFileThatDoesNotParse(t *testing.T) { nomad.JobVariables{ Flags: map[string]string{"a": "b"}, File: "{\"image\": \"v1\"}\n{\"count\": 3}\n", - }, 0) + }, 0, nil) require.ErrorContains(t, err, "the variables file of the job does not parse") } @@ -417,3 +417,20 @@ func TestSubmitJob_FileAloneIsSentAsWrittenWhateverItHolds(t *testing.T) { require.Equal(t, file, variablesSent(t, nomad.JobVariables{File: file})) } } + +func TestJobCounts_AGroupWithoutACountHasOne(t *testing.T) { + r := require.New(t) + + client, asked := clusterServer(t, map[string]string{"/v1/jobs/parse": `{"ID": "web", "TaskGroups": [ + {"Name": "web", "Count": 3}, + {"Name": "api"} + ]}`}) + + counts, err := client.JobCounts(context.Background(), "production", "job \"web\" {}", nomad.JobVariables{}) + r.NoError(err) + + r.Equal(map[string]int{"web": 3, "api": 1}, counts) + + // Read the way the plan reads it: the cluster fills in what is left out. + r.Equal(true, (*asked)[0].body["Canonicalize"]) +} diff --git a/internal/nomad/plan.go b/internal/nomad/plan.go index 89bcca8..209b7e1 100644 --- a/internal/nomad/plan.go +++ b/internal/nomad/plan.go @@ -29,6 +29,9 @@ type Plan struct { Warnings string + // Kept are the groups that keep the count the job runs with. + Kept []KeptCount + // Index is the one the job had when it was planned. Submitting at it // fails when the job changed in between. Index uint64 @@ -55,9 +58,11 @@ type PlanGroup struct { Ignore int } -// PlanJob asks the cluster what submitting the job file would do. -func (c *Client) PlanJob(ctx context.Context, namespace, source string, vars JobVariables) (Plan, error) { - job, _, err := c.jobOf(ctx, namespace, source, vars) +// PlanJob asks the cluster what submitting the job file would do. The counts +// the file had when it was opened decide which counts the job keeps, as they +// do for SubmitJob; nil keeps none. +func (c *Client) PlanJob(ctx context.Context, namespace, source string, vars JobVariables, opened map[string]int) (Plan, error) { + job, _, kept, err := c.jobOf(ctx, namespace, source, vars, opened) if err != nil { return Plan{}, err } @@ -67,7 +72,10 @@ func (c *Client) PlanJob(ctx context.Context, namespace, source string, vars Job return Plan{}, err } - return newPlan(answer), nil + plan := newPlan(answer) + plan.Kept = kept + + return plan, nil } func newPlan(answer *api.JobPlanResponse) Plan { diff --git a/internal/nomad/plan_test.go b/internal/nomad/plan_test.go index 8b2c95d..3b5a407 100644 --- a/internal/nomad/plan_test.go +++ b/internal/nomad/plan_test.go @@ -2,8 +2,10 @@ package nomad_test import ( "context" + "fmt" "net/http" "net/http/httptest" + "strings" "testing" "github.com/stretchr/testify/require" @@ -35,7 +37,7 @@ func TestPlanJob(t *testing.T) { client, asked := clusterServer(t, map[string]string{"/v1/job/web/plan": webPlan}) - plan, err := client.PlanJob(context.Background(), "production", `{"ID": "web", "Name": "web"}`, nomad.JobVariables{}) + plan, err := client.PlanJob(context.Background(), "production", `{"ID": "web", "Name": "web"}`, nomad.JobVariables{}, nil) r.NoError(err) // The plan is requested in the namespace of the job, with the diff: @@ -74,7 +76,7 @@ func TestPlanJob_HCLIsReadByTheClusterFirst(t *testing.T) { vars := nomad.JobVariables{Flags: map[string]string{"image": "nginx:1.27"}} - _, err := client.PlanJob(context.Background(), "production", "job \"web\" {}", vars) + _, err := client.PlanJob(context.Background(), "production", "job \"web\" {}", vars, nil) r.NoError(err) // The file is planned the way it would be submitted: with its variables. @@ -89,7 +91,7 @@ func TestSubmitJob_AtTheIndexOfItsPlan(t *testing.T) { client, asked := clusterServer(t, map[string]string{"/v1/jobs": `{"EvalID": "eval-1"}`}) - r.NoError(client.SubmitJob(context.Background(), "production", `{"ID": "web"}`, nomad.JobVariables{}, 42)) + r.NoError(client.SubmitJob(context.Background(), "production", `{"ID": "web"}`, nomad.JobVariables{}, 42, nil)) // What was planned is what is submitted, or nothing: a job that changed // since the plan is refused. @@ -111,7 +113,7 @@ func TestSubmitJob_WhenTheJobChangedSinceItsPlan(t *testing.T) { client, err := nomad.New(nomad.Config{Address: server.URL}) r.NoError(err) - err = client.SubmitJob(context.Background(), "production", `{"ID": "web"}`, nomad.JobVariables{}, 42) + err = client.SubmitJob(context.Background(), "production", `{"ID": "web"}`, nomad.JobVariables{}, 42, nil) r.ErrorIs(err, nomad.ErrJobChanged) } @@ -226,3 +228,217 @@ func TestRevertJobTo_WhenTheJobChangedSinceItsPlan(t *testing.T) { r.ErrorIs(client.RevertJobTo(context.Background(), "production", "web", 4, 5), nomad.ErrJobChanged) } + +// runningWeb is web as it runs: scaled to 5 after its file said 1. +const runningWeb = `{"ID": "web", "Name": "web", "TaskGroups": [{"Name": "web", "Count": 5}]}` + +// fileOfWeb is what the cluster reads out of the file of web. +func fileOfWeb(counts ...int) string { + groups := make([]string, len(counts)) + for i, count := range counts { + name := []string{"web", "api"}[i] + groups[i] = fmt.Sprintf(`{"Name": %q, "Count": %d}`, name, count) + } + + return `{"ID": "web", "Name": "web", "TaskGroups": [` + strings.Join(groups, ",") + `]}` +} + +// countsSent are the counts of the groups in the job a request carried. +func countsSent(t *testing.T, body map[string]any) map[string]float64 { + t.Helper() + + job, ok := body["Job"].(map[string]any) + require.True(t, ok) + + groups, _ := job["TaskGroups"].([]any) + counts := map[string]float64{} + + for _, group := range groups { + g, _ := group.(map[string]any) + counts[g["Name"].(string)], _ = g["Count"].(float64) + } + + return counts +} + +func keepCountsServer(t *testing.T, file string) (*nomad.Client, *[]sent) { + t.Helper() + + return clusterServer(t, map[string]string{ + "/v1/jobs/parse": file, + "/v1/job/web": runningWeb, + "/v1/job/web/plan": webPlan, + "/v1/jobs": `{"EvalID": "eval-1"}`, + }) +} + +func TestPlanAndSubmitJob_KeepTheCountTheJobRunsWith(t *testing.T) { + r := require.New(t) + + client, asked := keepCountsServer(t, fileOfWeb(1)) + opened := map[string]int{"web": 1} + + plan, err := client.PlanJob(context.Background(), "production", "job \"web\" {}", nomad.JobVariables{}, opened) + r.NoError(err) + + r.Equal([]nomad.KeptCount{{Group: "web", Kept: 5, File: 1}}, plan.Kept) + + r.NoError(client.SubmitJob(context.Background(), "production", "job \"web\" {}", nomad.JobVariables{}, 42, opened)) + + // The plan and the submit send the same counts: what was planned is + // what runs. + var bodies []map[string]any + + for _, req := range *asked { + if req.path == "/v1/job/web/plan" || req.path == "/v1/jobs" { + bodies = append(bodies, req.body) + } + } + + r.Len(bodies, 2) + + for _, body := range bodies { + r.Equal(map[string]float64{"web": 5}, countsSent(t, body)) + } +} + +func TestPlanJob_ACountChangedInTheEditorIsTheCountOfTheFile(t *testing.T) { + r := require.New(t) + + client, asked := keepCountsServer(t, fileOfWeb(3)) + + plan, err := client.PlanJob(context.Background(), "production", "job \"web\" {}", nomad.JobVariables{}, map[string]int{"web": 1}) + r.NoError(err) + + r.Empty(plan.Kept) + r.Equal(map[string]float64{"web": 3}, countsSent(t, plannedRequest(t, *asked))) +} + +func TestPlanJob_ANewGroupHasTheCountOfTheFile(t *testing.T) { + r := require.New(t) + + client, asked := keepCountsServer(t, fileOfWeb(1, 2)) + + plan, err := client.PlanJob(context.Background(), "production", "job \"web\" {}", nomad.JobVariables{}, map[string]int{"web": 1}) + r.NoError(err) + + r.Equal([]nomad.KeptCount{{Group: "web", Kept: 5, File: 1}}, plan.Kept) + r.Equal(map[string]float64{"web": 5, "api": 2}, countsSent(t, plannedRequest(t, *asked))) +} + +// planWith plans file against a job that runs as running, with the counts +// the editor opened. +func planWith(t *testing.T, file, running string, opened map[string]int) (nomad.Plan, []sent) { + t.Helper() + + client, asked := clusterServer(t, map[string]string{ + "/v1/jobs/parse": file, + "/v1/job/web": running, + "/v1/job/web/plan": webPlan, + }) + + plan, err := client.PlanJob(context.Background(), "production", "job \"web\" {}", nomad.JobVariables{}, opened) + require.NoError(t, err) + + return plan, *asked +} + +func TestPlanJob_AJobThatCannotBeReadIsAnError(t *testing.T) { + r := require.New(t) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + switch req.URL.Path { + case "/v1/jobs/parse": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(fileOfWeb(1))) + case "/v1/job/web": + http.Error(w, "no leader", http.StatusInternalServerError) + default: + http.Error(w, "not found", http.StatusNotFound) + } + })) + t.Cleanup(server.Close) + + client, err := nomad.New(nomad.Config{Address: server.URL}) + r.NoError(err) + + // Planning without the counts it runs would reset them: it says why + // instead. + _, err = client.PlanJob(context.Background(), "production", "job \"web\" {}", nomad.JobVariables{}, map[string]int{"web": 1}) + r.ErrorContains(err, "no leader") +} + +func TestPlanJob_AJobThatIsGoneHasTheCountsOfTheFile(t *testing.T) { + r := require.New(t) + + client, asked := clusterServer(t, map[string]string{ + "/v1/jobs/parse": fileOfWeb(1), + "/v1/job/web/plan": webPlan, + }) + + plan, err := client.PlanJob(context.Background(), "production", "job \"web\" {}", nomad.JobVariables{}, map[string]int{"web": 1}) + r.NoError(err) + + r.Empty(plan.Kept) + r.Equal(map[string]float64{"web": 1}, countsSent(t, plannedRequest(t, *asked))) +} + +func TestPlanJob_AGroupTheJobDoesNotRunHasTheCountOfTheFile(t *testing.T) { + r := require.New(t) + + plan, asked := planWith(t, fileOfWeb(1, 1), runningWeb, map[string]int{"web": 1, "api": 1}) + + r.Equal([]nomad.KeptCount{{Group: "web", Kept: 5, File: 1}}, plan.Kept) + r.Equal(map[string]float64{"web": 5, "api": 1}, countsSent(t, plannedRequest(t, asked))) +} + +func TestPlanJob_ACountThatIsTheCountOfTheFileKeepsNothing(t *testing.T) { + r := require.New(t) + + running := `{"ID": "web", "Name": "web", "TaskGroups": [{"Name": "web", "Count": 1}]}` + plan, asked := planWith(t, fileOfWeb(1), running, map[string]int{"web": 1}) + + r.Empty(plan.Kept) + r.Equal(map[string]float64{"web": 1}, countsSent(t, plannedRequest(t, asked))) +} + +func TestPlanJob_AGroupTheFileAddsHasItsCountEvenWhenTheJobRunsOneOfThatName(t *testing.T) { + r := require.New(t) + + running := `{"ID": "web", "Name": "web", "TaskGroups": [{"Name": "web", "Count": 5}, {"Name": "api", "Count": 3}]}` + plan, asked := planWith(t, fileOfWeb(1, 0), running, map[string]int{"web": 1}) + + r.Equal([]nomad.KeptCount{{Group: "web", Kept: 5, File: 1}}, plan.Kept) + r.Equal(map[string]float64{"web": 5, "api": 0}, countsSent(t, plannedRequest(t, asked))) +} + +func TestPlanJob_WithoutOpenedCountsEveryCountIsAsWritten(t *testing.T) { + r := require.New(t) + + client, asked := keepCountsServer(t, fileOfWeb(1)) + + plan, err := client.PlanJob(context.Background(), "production", "job \"web\" {}", nomad.JobVariables{}, nil) + r.NoError(err) + + r.Empty(plan.Kept) + r.Equal(map[string]float64{"web": 1}, countsSent(t, plannedRequest(t, *asked))) + + // A new job asks nothing about a job that runs. + for _, req := range *asked { + r.NotEqual("/v1/job/web", req.path) + } +} + +func plannedRequest(t *testing.T, asked []sent) map[string]any { + t.Helper() + + for _, req := range asked { + if req.path == "/v1/job/web/plan" { + return req.body + } + } + + t.Fatal("no plan was asked for") + + return nil +} diff --git a/internal/ui/app_test.go b/internal/ui/app_test.go index 0c0eba8..dbf9e5a 100644 --- a/internal/ui/app_test.go +++ b/internal/ui/app_test.go @@ -70,6 +70,13 @@ type fakeClient struct { submittedSource string submittedVars nomad.JobVariables submittedIndex uint64 + submittedOpened map[string]int + + // counts are what the job file read says about its groups, openedWith what + // the plan was last asked to keep counts for. + counts map[string]int + countsErr error + openedWith map[string]int plan nomad.Plan target nomad.JobTarget @@ -873,8 +880,16 @@ func (f *fakeClient) TaskGroups(_ context.Context, namespace, jobID string) ([]n return f.groups, f.err } -func (f *fakeClient) PlanJob(_ context.Context, namespace, source string, vars nomad.JobVariables) (nomad.Plan, error) { - f.askedNamespace, f.plannedSource, f.plannedVars = namespace, source, vars +func (f *fakeClient) JobCounts(context.Context, string, string, nomad.JobVariables) (map[string]int, error) { + if f.countsErr != nil { + return nil, f.countsErr + } + + return f.counts, f.err +} + +func (f *fakeClient) PlanJob(_ context.Context, namespace, source string, vars nomad.JobVariables, opened map[string]int) (nomad.Plan, error) { + f.askedNamespace, f.plannedSource, f.plannedVars, f.openedWith = namespace, source, vars, opened f.planCalls++ return f.plan, f.refused(f.err) @@ -886,8 +901,9 @@ func (f *fakeClient) JobTarget(_ context.Context, namespace, _ string) (nomad.Jo return f.target, f.refused(f.err) } -func (f *fakeClient) SubmitJob(_ context.Context, namespace, source string, vars nomad.JobVariables, index uint64) error { +func (f *fakeClient) SubmitJob(_ context.Context, namespace, source string, vars nomad.JobVariables, index uint64, opened map[string]int) error { f.wrote("SubmitJob") + f.submittedOpened = opened f.askedNamespace, f.submittedSource, f.submittedVars = namespace, source, vars f.submittedIndex = index f.submitted++ diff --git a/internal/ui/edit.go b/internal/ui/edit.go index 59aacf8..eaec524 100644 --- a/internal/ui/edit.go +++ b/internal/ui/edit.go @@ -119,6 +119,10 @@ func jobFile(client jobsClient, job nomad.Job) load { return file{}, err } + // The cluster parses a job without the files it reads: a file that + // calls file() still opens, and its plan says the counts are its own. + opened, countsErr := client.JobCounts(ctx, job.Namespace, spec.Source, spec.Variables) + // The editor gets the file only, the values of the variables are // sent back with it. The edited job is planned first: a changed // file can restart every allocation of the job. @@ -133,6 +137,8 @@ func jobFile(client jobsClient, job nomad.Job) load { source: source, vars: spec.Variables, read: spec.Index, + opened: opened, + uncounted: countsErr != nil, }) })}, nil } diff --git a/internal/ui/edit_test.go b/internal/ui/edit_test.go index f9d0056..cdb2da2 100644 --- a/internal/ui/edit_test.go +++ b/internal/ui/edit_test.go @@ -1,6 +1,7 @@ package ui import ( + "errors" "os" "path/filepath" "testing" @@ -137,6 +138,56 @@ func TestEdit_KeepsTheVariables(t *testing.T) { r.Equal(vars, client.submittedVars) } +func TestEdit_KeepsTheCountsOfTheFileAsOpened(t *testing.T) { + r := require.New(t) + + client := &fakeClient{ + jobs: twoJobs(), + spec: nomad.JobSource{Source: "job \"web\" {}"}, + counts: map[string]int{"web": 1}, + } + m, editor := editModel(t, client) + editor.replace = "job \"web\" {\n type = \"batch\"\n}" + + m, cmd := m.update(key('e')) + m = follow(m, cmd, 6) + + // The counts of the file as it was opened go to the plan and to the + // submit: they tell which counts the editor changed. + r.Equal(client.counts, client.openedWith) + + m, cmd = m.update(key('y')) + drain(m, cmd) + + r.Equal(client.counts, client.submittedOpened) +} + +func TestEdit_OpensAFileWhoseCountsCannotBeRead(t *testing.T) { + r := require.New(t) + + // The cluster parses a job without the files it reads, so a file that + // calls file() opens and is planned once the call is gone. + client := &fakeClient{ + jobs: twoJobs(), + spec: nomad.JobSource{Source: "job \"web\" {}", Index: 42}, + countsErr: errors.New("filesystem function disabled"), + plan: changedEnv(), + } + m, editor := editModel(t, client) + editor.replace = "job \"web\" {\n type = \"batch\"\n}" + + m, cmd := m.update(key('e')) + m = follow(m, cmd, 6) + + r.NotEmpty(editor.opened) + r.IsType(planPage{}, m.screen.page) + r.Nil(client.openedWith) + + out := plain(m.render()) + r.Contains(out, "Counts as the file says them") + r.Contains(out, "Submitting sets every count to the one in the file.") +} + func TestEdit_UnchangedFileChangesNothing(t *testing.T) { r := require.New(t) diff --git a/internal/ui/newjob_test.go b/internal/ui/newjob_test.go index d294a68..295b5d5 100644 --- a/internal/ui/newjob_test.go +++ b/internal/ui/newjob_test.go @@ -81,6 +81,10 @@ func TestNewJob_GoesToTheNamespaceTheFileNames(t *testing.T) { r.Equal("staging", client.askedNamespace) r.Equal(1, client.submitted) + + // A new job has no counts to keep. + r.Nil(client.openedWith) + r.Nil(client.submittedOpened) } func TestNewJob_FromAllNamespacesGoesToTheOneTheClientDecided(t *testing.T) { diff --git a/internal/ui/plan.go b/internal/ui/plan.go index 7a18649..558435a 100644 --- a/internal/ui/plan.go +++ b/internal/ui/plan.go @@ -31,6 +31,14 @@ type planState struct { read uint64 index uint64 + // opened are the counts of the groups in the file as it was opened. Nil + // for a new job and a revert: they keep no counts. + opened map[string]int + + // uncounted says the counts of the file as opened could not be read, so + // none is kept. + uncounted bool + // revert goes back to the version to, from the version the job had when // it was planned. A nil to means the version before the one that runs. revert bool @@ -87,6 +95,10 @@ func planOf(plan nomad.Plan, state planState) planPage { lines = append(changedSinceRead(state.jobID), lines...) } + if state.uncounted { + lines = append(countsOfTheFile(), lines...) + } + return planPage{planState: state, content: paintedText(lines).textContent} } @@ -102,6 +114,18 @@ func changedSinceRead(jobID string) []paintedLine { } } +// countsOfTheFile warns that no count is kept: the file could not be read for +// its counts when it was opened. +func countsOfTheFile() []paintedLine { + return []paintedLine{ + {text: "Counts as the file says them", style: &styleWarn}, + {}, + {text: "The counts of the file could not be read when it was opened."}, + {text: "Submitting sets every count to the one in the file."}, + {}, + } +} + // existsAlready warns that a new job has the name of one that is there. func existsAlready(jobID string) []paintedLine { return []paintedLine{ @@ -245,7 +269,7 @@ func planFor(client jobsClient, state planState) tea.Cmd { return client.PlanRevert(ctx, state.namespace, state.jobID, state.to) } - return client.PlanJob(ctx, state.namespace, state.source, state.vars) + return client.PlanJob(ctx, state.namespace, state.source, state.vars, state.opened) }, func(plan nomad.Plan) tea.Msg { return openMsg{planOf(plan, state)} }) } @@ -272,7 +296,7 @@ func planNewJob(ctx context.Context, client jobsClient, namespace, source string fresh: true, } - plan, err := client.PlanJob(ctx, state.namespace, state.source, state.vars) + plan, err := client.PlanJob(ctx, state.namespace, state.source, state.vars, nil) return planOf(plan, state), err } @@ -295,7 +319,7 @@ func submitPlan(p planPage, e env) (planPage, outcome) { said = fmt.Sprintf("Job %s reverted to version %d.", state.jobID, *state.to) err = client.RevertJobTo(ctx, state.namespace, state.jobID, *state.to, state.from) } else { - err = client.SubmitJob(ctx, state.namespace, state.source, state.vars, submitIndex(state)) + err = client.SubmitJob(ctx, state.namespace, state.source, state.vars, submitIndex(state), state.opened) } switch { @@ -329,7 +353,7 @@ func replan(p planPage, e env) (planPage, outcome) { } // planText is a plan as a page: first the placement failures and the -// warnings, then what the scheduler would do to each group, then what would +// warnings, the counts kept, then what the scheduler would do to each group, then what would // change, line by line. func planText(plan nomad.Plan, revert bool) []paintedLine { lines := []paintedLine{} @@ -346,6 +370,16 @@ func planText(plan nomad.Plan, revert bool) []paintedLine { lines = append(lines, paintedLine{}) } + if len(plan.Kept) > 0 { + lines = append(lines, paintedLine{text: "Counts kept as the job runs them", style: &styleWarn}, paintedLine{}) + + for _, kept := range plan.Kept { + lines = append(lines, paintedLine{text: fmt.Sprintf(" Task group %q: %d, the file says %d", kept.Group, kept.Kept, kept.File)}) + } + + lines = append(lines, paintedLine{}) + } + lines = append(lines, paintedLine{text: fmt.Sprintf("What will happen when you %s this job:", strings.ToLower(planVerb(revert)))}, paintedLine{}) if len(plan.Groups) == 0 { diff --git a/internal/ui/plan_test.go b/internal/ui/plan_test.go index 4f28a6c..ba04fc7 100644 --- a/internal/ui/plan_test.go +++ b/internal/ui/plan_test.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" "testing" "time" @@ -101,6 +102,23 @@ func TestPlanText(t *testing.T) { r.Equal(opening(styleMuted), style(" No changes - 3 allocations will remain unchanged")) } +func TestPlanText_CountsKept(t *testing.T) { + r := require.New(t) + + plan := changedEnv() + plan.Kept = []nomad.KeptCount{{Group: "web", Kept: 5, File: 1}} + + lines := texts(planText(plan, false)) + + // The kept counts come above what will happen, after the warnings. + i := slices.Index(lines, "Counts kept as the job runs them") + r.Positive(i) + r.Equal([]string{"Counts kept as the job runs them", "", ` Task group "web": 5, the file says 1`, ""}, lines[i:i+4]) + r.Equal("What will happen when you submit this job:", lines[i+4]) + + r.NotContains(strings.Join(texts(planText(changedEnv(), false)), "\n"), "Counts kept") +} + func TestPlanText_EveryUpdate(t *testing.T) { r := require.New(t) diff --git a/internal/ui/roles.go b/internal/ui/roles.go index 49a0b6d..a10b127 100644 --- a/internal/ui/roles.go +++ b/internal/ui/roles.go @@ -57,10 +57,11 @@ type ( DescribeJob(ctx context.Context, namespace, jobID string) (string, error) JobSpec(ctx context.Context, namespace, jobID string) (nomad.JobSource, error) TaskGroups(ctx context.Context, namespace, jobID string) ([]nomad.TaskGroup, error) - PlanJob(ctx context.Context, namespace, source string, vars nomad.JobVariables) (nomad.Plan, error) + PlanJob(ctx context.Context, namespace, source string, vars nomad.JobVariables, opened map[string]int) (nomad.Plan, error) + JobCounts(ctx context.Context, namespace, source string, vars nomad.JobVariables) (map[string]int, error) JobTarget(ctx context.Context, namespace, source string) (nomad.JobTarget, error) PlanRevert(ctx context.Context, namespace, jobID string, to *uint64) (nomad.Plan, error) - SubmitJob(ctx context.Context, namespace, source string, vars nomad.JobVariables, index uint64) error + SubmitJob(ctx context.Context, namespace, source string, vars nomad.JobVariables, index uint64, opened map[string]int) error StartJob(ctx context.Context, namespace, jobID string) error StopJob(ctx context.Context, namespace, jobID string) error PurgeJob(ctx context.Context, namespace, jobID string) error diff --git a/internal/ui/writes_test.go b/internal/ui/writes_test.go index f090bed..5cc96e3 100644 --- a/internal/ui/writes_test.go +++ b/internal/ui/writes_test.go @@ -32,7 +32,7 @@ var ( clientReads = []string{ "Address", "Region", "Agent", "Regions", "Datacenters", "AllocationChecks", "Files", "File", "Deployment", "JobDeployments", "DeploymentAllocations", "Token", "ServiceInstances", - "Jobs", "NextLaunch", "JobSpec", "JobVersions", "JobVersionDiff", "TaskGroups", "PlanJob", "PlanRevert", "JobTarget", + "Jobs", "NextLaunch", "JobSpec", "JobVersions", "JobVersionDiff", "TaskGroups", "PlanJob", "PlanRevert", "JobTarget", "JobCounts", "DescribeJob", "DescribeAllocation", "DescribeDeployment", "DescribeService", "Allocations", "NodeAllocations", "Allocation", "Logs", "Usage", "AllocationUsage", "TaskUsage", "NodeUsage",