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

Perserve device ids in in-place alloc updates #7762

Merged
merged 3 commits into from
Apr 21, 2020

Conversation

notnoop
Copy link
Contributor

@notnoop notnoop commented Apr 21, 2020

When an alloc is updated in-place, ensure that any devices that are allocated to it are preserved and carried over to the new allocation struct.

I've added the test first, so one can see the failure in https://circleci.com/gh/hashicorp/nomad/60422, which is green in the follow up fix.

This builds up upon the work of @xsikor in #7697 . Thanks!

Fix #7696
Closes #7697

Mahmood Ali added 2 commits April 21, 2020 08:56
When an alloc is updated in-place, test that the allocated devices are
preserved in new alloc struct.
When an alloc is updated in-place, ensure that the allocated device are
preserved and carried over to new alloc.
@notnoop notnoop requested a review from tgross April 21, 2020 13:24
@notnoop notnoop self-assigned this Apr 21, 2020
[ci skip]
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

LGTM

if existing.AllocatedResources != nil {
if tr, ok := existing.AllocatedResources.Tasks[task]; ok {
networks = tr.Networks
devices = tr.Devices
}
} else if tr, ok := existing.TaskResources[task]; ok {
networks = tr.Networks
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In 0.11, existing.AllocatedResources should always be non-nil. The type of existing.TaskResources[task].Devices is different, so I didn't bother making the compatibility path.

@notnoop notnoop merged commit 15b7474 into master Apr 21, 2020
@notnoop notnoop deleted the b-in-place-update-deviceids branch April 21, 2020 13:31
@github-actions
Copy link

github-actions bot commented Jan 9, 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 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changing DeviceIDs after job updated in allocation
2 participants