Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add PreserveCounts to Job.Register #8168

Merged
merged 6 commits into from
Jun 17, 2020
Merged

Conversation

cgbaker
Copy link
Contributor

@cgbaker cgbaker commented Jun 15, 2020

expands job registration API to include the option to mutate a job, while leaving the task group counts as they are. this allow one system (e.g., CI/CD) to change job definition while another system (e.g., autoscaler) is changing job counts, without having to explicitly read-mutate-write.

in the process, this PR gets rid of a mostly-redundant struct in the api package, RegisterJobRequest, which served as a serialization struct and (occasionally) a stand-in for JobRegisterRequest (neither of which need to be exported from api)

resolves #8158

@cgbaker cgbaker changed the title add preserve_counts to Job.Register add PreserveCounts to Job.Register Jun 16, 2020
@cgbaker cgbaker force-pushed the f-8158-add-preserve-counts branch 2 times, most recently from 3ff5c35 to a3803e8 Compare June 16, 2020 15:30
@cgbaker cgbaker force-pushed the f-8158-add-preserve-counts branch from a3803e8 to f0c4984 Compare June 16, 2020 17:55
@cgbaker cgbaker force-pushed the f-8158-add-preserve-counts branch from f0c4984 to 0284698 Compare June 16, 2020 19:22
@@ -105,17 +106,16 @@ func (j *Jobs) EnforceRegister(job *Job, modifyIndex uint64, q *WriteOptions) (*
// of the evaluation, along with any errors encountered.
func (j *Jobs) RegisterOpts(job *Job, opts *RegisterOptions, q *WriteOptions) (*JobRegisterResponse, *WriteMeta, error) {
// Format the request
req := &RegisterJobRequest{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JobRegisterRequest has WriteRequest, but we don't use it. Otherwise, has the same shape as RegisterJobRequest

Comment on lines +216 to +220
req := struct {
Job *api.Job
}{
Job: job,
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need a class for deserializing a Job: -embedded job spec, without all of the other job registration options.

Comment on lines +165 to +169
req := struct {
Job *api.Job
}{
Job: job,
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need a class for deserializing a Job: -embedded job spec, without all of the other job registration options.

@cgbaker cgbaker marked this pull request as ready for review June 16, 2020 19:34
@cgbaker cgbaker requested review from notnoop and schmichael June 16, 2020 19:35
@cgbaker cgbaker added this to the 0.12.0 milestone Jun 16, 2020
@cgbaker cgbaker added the theme/autoscaling Issues related to supporting autoscaling label Jun 16, 2020
@@ -943,7 +943,10 @@ func decodeBody(resp *http.Response, out interface{}) error {
}
}

// encodeBody is used to encode a request body
// encodeBody prepares the reader to serve as the request body.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait, are Nomad vendoring itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm going to let @shoenig answer this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's more or less a mandate from Go 1.14

When the main module contains a top-level vendor directory and its go.mod file specifies go 1.14 or higher, the go command now defaults to -mod=vendor for operations that accept that flag.

Without setting -mod=mod, the vendor directory must be in the expected state, including vendoring of any imported submodules.

There is an open issue to improve the ergonomics around this use case. We're also working behind the scenes on the feasibility of removing the vendor directory entirely.

@cgbaker cgbaker merged commit 2cbfc83 into master Jun 17, 2020
@cgbaker cgbaker deleted the f-8158-add-preserve-counts branch June 17, 2020 12:28
@github-actions
Copy link

github-actions bot commented Jan 3, 2023

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
theme/autoscaling Issues related to supporting autoscaling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support PreserveCounts in job registration API
4 participants