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

[17070] Correct the status description and modify time of canceled evals. #17071

Conversation

stswidwinski
Copy link
Contributor

Fix for #17070. Corrected the status description and modify time of evals which are canceled due to another eval having completed in the meantime.

Tests added. It seems that the testing has originally been written a rather long time before the new-style of testing started within the Nomad codebase. Please let me know if you'd rather I moved the entire test to the new style, or if added if clauses seem like a reasonable tradeoff given the size of the change.

nomad/state/state_store.go Outdated Show resolved Hide resolved
Ensure modification time of completed and canceled evaluations is the same

Co-authored-by: Tim Gross <tgross@hashicorp.com>
@stswidwinski
Copy link
Contributor Author

It seems that the tests failed on unrelated test cases. I don't seem to have the permission to rerun them. Do you mind kicking it off for me (feel free to tell me I'm wrong and direct me to the right UI -- I am not confident that I am looking at the right place)

@tgross
Copy link
Member

tgross commented May 4, 2023

It seems that the tests failed on unrelated test cases. I don't seem to have the permission to rerun them. Do you mind kicking it off for me (feel free to tell me I'm wrong and direct me to the right UI -- I am not confident that I am looking at the right place)

Done. The failed test is a known flake we've got another issue open to fix in progress.

@jrasell jrasell added backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line labels May 5, 2023
Copy link
Member

@jrasell jrasell left a comment

Choose a reason for hiding this comment

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

LGTM and thanks @stswidwinski!

Would you be able to run make cl to add a changelog entry for this fix; with that I will get this merged.

… of github.com:stswidwinski/nomad into 17070-canceled_evaluation_provides_circular_debug_info
@stswidwinski
Copy link
Contributor Author

All done. Thank you!

@tgross
Copy link
Member

tgross commented May 8, 2023

Backport assistant screwed up all the backports. I'll do those by hand.

tgross pushed a commit that referenced this pull request May 8, 2023
…7071)

Fix for #17070. Corrected the status description and modify time of evals which are canceled due to another eval having completed in the meantime.
tgross pushed a commit that referenced this pull request May 8, 2023
…7071)

Fix for #17070. Corrected the status description and modify time of evals which are canceled due to another eval having completed in the meantime.
tgross pushed a commit that referenced this pull request May 8, 2023
…7071)

Fix for #17070. Corrected the status description and modify time of evals which are canceled due to another eval having completed in the meantime.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants