Skip to content

Commit

Permalink
TaggedVersion now VersionTag (#24066)
Browse files Browse the repository at this point in the history
  • Loading branch information
philrenaud authored Sep 25, 2024
1 parent 282db55 commit a750d71
Show file tree
Hide file tree
Showing 23 changed files with 80 additions and 81 deletions.
12 changes: 6 additions & 6 deletions api/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ func (j *Jobs) Versions(jobID string, diffs bool, q *QueryOptions) ([]*Job, []*J
return j.VersionsOpts(jobID, opts, q)
}

// VersionByTag is used to retrieve a job version by its TaggedVersion name.
// VersionByTag is used to retrieve a job version by its VersionTag name.
func (j *Jobs) VersionByTag(jobID, tag string, q *QueryOptions) (*Job, *QueryMeta, error) {
versions, _, qm, err := j.Versions(jobID, false, q)
if err != nil {
Expand All @@ -279,7 +279,7 @@ func (j *Jobs) VersionByTag(jobID, tag string, q *QueryOptions) (*Job, *QueryMet

// Find the version with the matching tag
for _, version := range versions {
if version.TaggedVersion != nil && version.TaggedVersion.Name == tag {
if version.VersionTag != nil && version.VersionTag.Name == tag {
return version, qm, nil
}
}
Expand Down Expand Up @@ -1030,18 +1030,18 @@ func (j *JobUILink) Copy() *JobUILink {
}
}

type JobTaggedVersion struct {
type JobVersionTag struct {
Name string
Description string
TaggedTime int64
}

func (j *JobTaggedVersion) Copy() *JobTaggedVersion {
func (j *JobVersionTag) Copy() *JobVersionTag {
if j == nil {
return nil
}

return &JobTaggedVersion{
return &JobVersionTag{
Name: j.Name,
Description: j.Description,
TaggedTime: j.TaggedTime,
Expand Down Expand Up @@ -1126,7 +1126,7 @@ type Job struct {
CreateIndex *uint64
ModifyIndex *uint64
JobModifyIndex *uint64
TaggedVersion *JobTaggedVersion
VersionTag *JobVersionTag
}

// IsPeriodic returns whether a job is periodic.
Expand Down
16 changes: 8 additions & 8 deletions command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ func (s *HTTPServer) jobVersionApplyTag(resp http.ResponseWriter, req *http.Requ
JobID: jobID,
Version: args.Version,
Name: name,
Tag: &structs.JobTaggedVersion{
Tag: &structs.JobVersionTag{
Name: name,
Description: args.Description,
},
Expand Down Expand Up @@ -1113,7 +1113,7 @@ func ApiJobToStructJob(job *api.Job) *structs.Job {
Constraints: ApiConstraintsToStructs(job.Constraints),
Affinities: ApiAffinitiesToStructs(job.Affinities),
UI: ApiJobUIConfigToStructs(job.UI),
TaggedVersion: ApiJobTaggedVersionToStructs(job.TaggedVersion),
VersionTag: ApiJobVersionTagToStructs(job.VersionTag),
}

// Update has been pushed into the task groups. stagger and max_parallel are
Expand Down Expand Up @@ -2218,15 +2218,15 @@ func ApiJobUIConfigToStructs(jobUI *api.JobUIConfig) *structs.JobUIConfig {
}
}

func ApiJobTaggedVersionToStructs(jobTaggedVersion *api.JobTaggedVersion) *structs.JobTaggedVersion {
if jobTaggedVersion == nil {
func ApiJobVersionTagToStructs(jobVersionTag *api.JobVersionTag) *structs.JobVersionTag {
if jobVersionTag == nil {
return nil
}

return &structs.JobTaggedVersion{
Name: jobTaggedVersion.Name,
Description: jobTaggedVersion.Description,
TaggedTime: jobTaggedVersion.TaggedTime,
return &structs.JobVersionTag{
Name: jobVersionTag.Name,
Description: jobVersionTag.Description,
TaggedTime: jobVersionTag.TaggedTime,
}
}

Expand Down
16 changes: 8 additions & 8 deletions command/agent/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4449,34 +4449,34 @@ func TestConversion_ApiJobUIConfigToStructs(t *testing.T) {
})
}

func TestConversion_ApiJobTaggedVersionToStructs(t *testing.T) {
func TestConversion_ApiJobVersionTagToStructs(t *testing.T) {
t.Run("nil tagged version", func(t *testing.T) {
must.Nil(t, ApiJobTaggedVersionToStructs(nil))
must.Nil(t, ApiJobVersionTagToStructs(nil))
})

t.Run("empty tagged version", func(t *testing.T) {
taggedVersion := &api.JobTaggedVersion{}
expected := &structs.JobTaggedVersion{
versionTag := &api.JobVersionTag{}
expected := &structs.JobVersionTag{
Name: "",
Description: "",
TaggedTime: 0,
}
result := ApiJobTaggedVersionToStructs(taggedVersion)
result := ApiJobVersionTagToStructs(versionTag)
must.Eq(t, expected, result)
})

t.Run("tagged version with tag and version", func(t *testing.T) {
taggedVersion := &api.JobTaggedVersion{
versionTag := &api.JobVersionTag{
Name: "low-latency",
Description: "Low latency version",
TaggedTime: 1234567890,
}
expected := &structs.JobTaggedVersion{
expected := &structs.JobVersionTag{
Name: "low-latency",
Description: "Low latency version",
TaggedTime: 1234567890,
}
result := ApiJobTaggedVersionToStructs(taggedVersion)
result := ApiJobVersionTagToStructs(versionTag)
must.Eq(t, expected, result)
})
}
8 changes: 4 additions & 4 deletions command/job_history.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,10 @@ func (c *JobHistoryCommand) formatJobVersion(job *api.Job, diff *api.JobDiff, fu
fmt.Sprintf("Submit Date|%v", formatTime(time.Unix(0, *job.SubmitTime))),
}
// if tagged version is not nil
if job.TaggedVersion != nil {
basic = append(basic, fmt.Sprintf("Tag Name|%v", *&job.TaggedVersion.Name))
if job.TaggedVersion.Description != "" {
basic = append(basic, fmt.Sprintf("Tag Description|%v", *&job.TaggedVersion.Description))
if job.VersionTag != nil {
basic = append(basic, fmt.Sprintf("Tag Name|%v", *&job.VersionTag.Name))
if job.VersionTag.Description != "" {
basic = append(basic, fmt.Sprintf("Tag Description|%v", *&job.VersionTag.Description))
}
}

Expand Down
2 changes: 1 addition & 1 deletion command/job_history_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func TestJobHistoryCommand_Diffs(t *testing.T) {

v1 := v0.Copy()
v1.TaskGroups[0].Count = 2
v1.TaggedVersion = &structs.JobTaggedVersion{
v1.VersionTag = &structs.JobVersionTag{
Name: "example-tag",
Description: "example-description",
}
Expand Down
2 changes: 1 addition & 1 deletion command/job_revert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func TestJobRevertCommand_VersionTag(t *testing.T) {

v1 := v0.Copy()
v1.TaskGroups[0].Count = 2
v1.TaggedVersion = &structs.JobTaggedVersion{
v1.VersionTag = &structs.JobVersionTag{
Name: "v1-tag",
Description: "Version 1 tag",
}
Expand Down
4 changes: 2 additions & 2 deletions command/job_tag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ func TestJobTagApplyCommand_Run(t *testing.T) {
code = cmd.Run([]string{"-address=" + url, "-name=test2", v0.ID})
apiJob, _, err := client.Jobs().Info(v0.ID, nil)
must.NoError(t, err)
must.NotNil(t, apiJob.TaggedVersion)
must.Eq(t, "test2", apiJob.TaggedVersion.Name)
must.NotNil(t, apiJob.VersionTag)
must.Eq(t, "test2", apiJob.VersionTag.Name)
must.StrContains(t, ui.OutputWriter.String(), "Job version 3 tagged with name \"test2\"")
must.Zero(t, code)
ui.OutputWriter.Reset()
Expand Down
2 changes: 1 addition & 1 deletion nomad/core_sched.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ OUTER:
continue
}
for _, v := range versions {
if v.TaggedVersion != nil {
if v.VersionTag != nil {
continue OUTER
}
}
Expand Down
4 changes: 2 additions & 2 deletions nomad/core_sched_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ func TestCoreScheduler_EvalGC_Batch(t *testing.T) {
}

// A job that has any of its versions tagged should not be GC-able.
func TestCoreScheduler_EvalGC_JobTaggedVersion(t *testing.T) {
func TestCoreScheduler_EvalGC_JobVersionTag(t *testing.T) {
ci.Parallel(t)

s1, cleanupS1 := TestServer(t, nil)
Expand Down Expand Up @@ -661,7 +661,7 @@ func TestCoreScheduler_EvalGC_JobTaggedVersion(t *testing.T) {
&structs.JobApplyTagRequest{
JobID: job.ID,
Name: name,
Tag: &structs.JobTaggedVersion{
Tag: &structs.JobVersionTag{
Name: name,
Description: desc,
},
Expand Down
4 changes: 2 additions & 2 deletions nomad/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -656,8 +656,8 @@ func (j *Job) Revert(args *structs.JobRevertRequest, reply *structs.JobRegisterR
revJob.VaultToken = args.VaultToken // use vault token from revert to perform (re)registration
revJob.ConsulToken = args.ConsulToken // use consul token from revert to perform (re)registration

// Clear out the TaggedVersion to prevent tag duplication
revJob.TaggedVersion = nil
// Clear out the VersionTag to prevent tag duplication
revJob.VersionTag = nil

reg := &structs.JobRegisterRequest{
Job: revJob,
Expand Down
2 changes: 1 addition & 1 deletion nomad/state/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ func jobIsGCable(obj interface{}) (bool, error) {
}

// job versions that are tagged should be kept
if j.TaggedVersion != nil {
if j.VersionTag != nil {
return false, nil
}

Expand Down
2 changes: 1 addition & 1 deletion nomad/state/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func Test_jobIsGCable(t *testing.T) {
{
name: "tagged",
inputObj: &structs.Job{
TaggedVersion: &structs.JobTaggedVersion{Name: "any"},
VersionTag: &structs.JobVersionTag{Name: "any"},
},
expectedOutput: false,
expectedOutputError: nil,
Expand Down
12 changes: 6 additions & 6 deletions nomad/state/state_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -2191,7 +2191,7 @@ func (s *StateStore) upsertJobVersion(index uint64, job *structs.Job, txn *txn)
// Find the oldest non-tagged version to delete
deleteIdx := -1
for i := len(all) - 1; i >= max; i-- {
if all[i].TaggedVersion == nil {
if all[i].VersionTag == nil {
deleteIdx = i
break
}
Expand Down Expand Up @@ -2325,7 +2325,7 @@ func (s *StateStore) JobVersionByTagName(ws memdb.WatchSet, namespace, id string
return nil, err
}
for _, j := range versions {
if j.TaggedVersion != nil && j.TaggedVersion.Name == tagName {
if j.VersionTag != nil && j.VersionTag.Name == tagName {
return j, nil
}
}
Expand Down Expand Up @@ -4952,7 +4952,7 @@ func (s *StateStore) UpdateJobVersionTag(index uint64, namespace string, req *st
return txn.Commit()
}

func (s *StateStore) updateJobVersionTagImpl(index uint64, namespace, jobID string, jobVersion uint64, tag *structs.JobTaggedVersion, txn *txn) error {
func (s *StateStore) updateJobVersionTagImpl(index uint64, namespace, jobID string, jobVersion uint64, tag *structs.JobVersionTag, txn *txn) error {
// Note: could use JobByIDAndVersion to get the specific version we want here,
// but then we'd have to make a second lookup to make sure we're not applying a duplicate tag name
versions, err := s.JobVersionsByID(nil, namespace, jobID)
Expand All @@ -4964,7 +4964,7 @@ func (s *StateStore) updateJobVersionTagImpl(index uint64, namespace, jobID stri

for _, version := range versions {
// Allow for a tag to be updated (new description, for example) but otherwise don't allow a same-tagname to a different version.
if version.TaggedVersion != nil && version.TaggedVersion.Name == tag.Name && version.Version != jobVersion {
if version.VersionTag != nil && version.VersionTag.Name == tag.Name && version.Version != jobVersion {
return fmt.Errorf("tag %q already exists on a different version of job %q", tag.Name, jobID)
}
if version.Version == jobVersion {
Expand All @@ -4977,7 +4977,7 @@ func (s *StateStore) updateJobVersionTagImpl(index uint64, namespace, jobID stri
}

versionCopy := job.Copy()
versionCopy.TaggedVersion = tag
versionCopy.VersionTag = tag
versionCopy.ModifyIndex = index

latestJob, err := s.JobByID(nil, namespace, jobID)
Expand All @@ -5003,7 +5003,7 @@ func (s *StateStore) unsetJobVersionTagImpl(index uint64, namespace, jobID strin
}

versionCopy := job.Copy()
versionCopy.TaggedVersion = nil
versionCopy.VersionTag = nil
versionCopy.ModifyIndex = index
latestJob, err := s.JobByID(nil, namespace, jobID)
if err != nil {
Expand Down
16 changes: 8 additions & 8 deletions nomad/state/state_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2901,11 +2901,11 @@ func TestStateStore_DeleteJobTxn_BatchDeletes(t *testing.T) {
require.Equal(t, deletionIndex, index)
}

// TestStatestore_JobTaggedVersion tests that job versions which are tagged
// TestStatestore_JobVersionTag tests that job versions which are tagged
// do not count against the configured server.job_tracked_versions count,
// do not get deleted when new versions are created,
// and *do* get deleted immediately when its tag is removed.
func TestStatestore_JobTaggedVersion(t *testing.T) {
func TestStatestore_JobVersionTag(t *testing.T) {
ci.Parallel(t)

state := testStateStore(t)
Expand All @@ -2928,7 +2928,7 @@ func TestStatestore_JobTaggedVersion(t *testing.T) {
req := &structs.JobApplyTagRequest{
JobID: job.ID,
Name: name,
Tag: &structs.JobTaggedVersion{
Tag: &structs.JobVersionTag{
Name: name,
Description: desc,
},
Expand All @@ -2940,8 +2940,8 @@ func TestStatestore_JobTaggedVersion(t *testing.T) {
got, err := state.JobVersionByTagName(nil, job.Namespace, job.ID, name)
must.NoError(t, err)
must.Eq(t, version, got.Version)
must.Eq(t, name, got.TaggedVersion.Name)
must.Eq(t, desc, got.TaggedVersion.Description)
must.Eq(t, name, got.VersionTag.Name)
must.Eq(t, desc, got.VersionTag.Description)
}
unsetTag := func(t *testing.T, name string) {
t.Helper()
Expand Down Expand Up @@ -3018,23 +3018,23 @@ func TestStatestore_JobTaggedVersion(t *testing.T) {
// job does not exist
err := state.UpdateJobVersionTag(nextIndex(state), job.Namespace, &structs.JobApplyTagRequest{
JobID: "non-existent-job",
Tag: &structs.JobTaggedVersion{Name: "tag name"},
Tag: &structs.JobVersionTag{Name: "tag name"},
Version: 0,
})
must.ErrorContains(t, err, `job "non-existent-job" version 0 not found`)

// version does not exist
err = state.UpdateJobVersionTag(nextIndex(state), job.Namespace, &structs.JobApplyTagRequest{
JobID: job.ID,
Tag: &structs.JobTaggedVersion{Name: "tag name"},
Tag: &structs.JobVersionTag{Name: "tag name"},
Version: 999,
})
must.ErrorContains(t, err, fmt.Sprintf("job %q version 999 not found", job.ID))

// tag name already exists
err = state.UpdateJobVersionTag(nextIndex(state), job.Namespace, &structs.JobApplyTagRequest{
JobID: job.ID,
Tag: &structs.JobTaggedVersion{Name: "v0"},
Tag: &structs.JobVersionTag{Name: "v0"},
Version: 10,
})
must.ErrorContains(t, err, fmt.Sprintf(`"v0" already exists on a different version of job %q`, job.ID))
Expand Down
Loading

0 comments on commit a750d71

Please sign in to comment.