-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat: compatibility with BatchGetJobEntity API changes for jobRunAsUser #133
Conversation
user = job_run_as_user_posix["user"] | ||
group = job_run_as_user_posix["group"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If posix
isn't in the job_run_as_user_data
, I think we get a KeyError
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good eye! I think that code path should not reachable because of earlier input validation:
Regardless, I have made this code more defensive in case someone accidentally changes/removes the validation code or my assumption is incorrect.
"runAs": "QUEUE_CONFIGURED_USER", | ||
}, | ||
}, | ||
id="invalid new-style jobRunAsUser - empty user", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, I think we're trying to refrain from using invalid
and to use non valid
whenever possible. Applies elsewhere in this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing this out. I've replaced all instances of invalid
→ nonvalid
in the repo.
40fcd7c
to
adb41ee
Compare
- provides forwards/backwards compatibility with the current API usage of the BatchGetJobEntity "jobDetails" entity responses - Updates some developer test code and configuration to accomodate API changes to CreateQueue and to be able to end-to-end test these changes Signed-off-by: Josh Usiskin <56369778+jusiskin@users.noreply.github.com>
adb41ee
to
c50caa9
Compare
…er (#133) - provides forwards/backwards compatibility with the current API usage of the BatchGetJobEntity "jobDetails" entity responses - Updates some developer test code and configuration to accomodate API changes to CreateQueue and to be able to end-to-end test these changes Signed-off-by: Josh Usiskin <56369778+jusiskin@users.noreply.github.com>
…er (#133) - provides forwards/backwards compatibility with the current API usage of the BatchGetJobEntity "jobDetails" entity responses - Updates some developer test code and configuration to accomodate API changes to CreateQueue and to be able to end-to-end test these changes Signed-off-by: Josh Usiskin <56369778+jusiskin@users.noreply.github.com> Signed-off-by: Graeme McHale <gmchale@amazon.com>
…tten (aws-deadline#133) Problem: When starting a worker using PosixInstanceWorker it can sometimes be the case that we query for the Worker's id before the worker.json file has been written to disk. If this happens then the test will fail. Solution: Repeatedly query for the worker.json file in a delaying loop, up to a maximum of 10 queries after about a minute. Signed-off-by: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>
What was the problem/requirement? (What/Why)
The
BatchGetJobEntity
API response shape is changing. Specifically when the response contains"jobDetails"
entities, the"jobRunAsUser"
field will return a"runAs"
field. For example:or if the job's queue has no queue configured user:
This API change has not been deployed in the service yet. We need the worker agent to accomodate
the current API shape and the future API shape (forwards/backwards compatibility).
What was the solution? (How)
Updated the code to accomodate the future API change while still working with the current API
responses.
I've also updated some of the related developer scripts for testing this including the POSIX Docker
containers and the scripts to create service resources.
What is the impact of this change?
BatchGetJobEntity
API changes thatare about to land
functional code that uses the queue
jobRunAsUser
feature.How was this change tested?
"backwards" compatibility testing
Ran the Worker Agent integration tests, which assert that the
jobRunAsUser
worker agentfunctionality works against the current service API responses.
"forwards" compatibility testing
scripts/create_service_resources.sh
script to add the"runAs"
when creating queues.This is now required in order to create a queue-fleet association, otherwise the API returns
validation errors
posix_local_multiuser
containerexcept that the job user is not overridden
Used the
src/deadline_worker_agent/boto/shim.py
to mutate the API responses and return the newAPI shape (includes the
"runAs"
field in theBatchGetJobEntity
API responses).I've also updated the unit tests to test both forwards and backwards compatibility.
Was this change documented?
No
Is this a breaking change?
No