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

feature: Add new field render_templates on restart block #18054

Merged
merged 5 commits into from
Jul 28, 2023

Conversation

nvanthao
Copy link
Contributor

This feature is necessary when user want to explicitly re-render all templates on task restart. E.g. to fetch all new secrets from Vault, even if the lease on the existing secrets has not been expired.

This feature is necessary when user want to explicitly re-render all templates on task restart.
E.g. to fetch all new secrets from Vault, even if the lease on the existing secrets has not been expired.
@schmichael
Copy link
Member

Looks like TestJobGetter_HTTPServer might still be a real failure. Let me know if you want help with the tests; I know they're a lot (and we have some flakes).

@nvanthao
Copy link
Contributor Author

My apologies @schmichael, next PRs will be better! I have updated the tests.

@schmichael
Copy link
Member

My apologies @schmichael, next PRs will be better! I have updated the tests.

You've clearly never looked at how many tries it takes me on my PRs then. 😅 No worries. This is looking great, and jobspec additions have an obnoxious amount of plumbing to track down. 👍

Copy link
Member

@schmichael schmichael left a comment

Choose a reason for hiding this comment

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

Well that one set of tests just refuses to finish in time, but things look good to me. Thanks for sticking with this!

It will get released in the next 1.6.x release, but we don't generally backport new features.

@schmichael schmichael merged commit 9e98d69 into main Jul 28, 2023
23 of 24 checks passed
@schmichael schmichael deleted the f-restart-render-template branch July 28, 2023 18:53
@nvanthao
Copy link
Contributor Author

Many thanks for reviewing @schmichael!

tgross added a commit that referenced this pull request Jul 31, 2023
In #18054 we introduced a new field `render_templates` in the `restart`
block. Previously changes to the `restart` block were always non-destructive in
the scheduler but we now need to check the new field so that we can update the
template runner. The check assumed that the block was always non-nil, which
causes panics in our scheduler tests.
tgross added a commit that referenced this pull request Jul 31, 2023
In #18054 we introduced a new field `render_templates` in the `restart`
block. Previously changes to the `restart` block were always non-destructive in
the scheduler but we now need to check the new field so that we can update the
template runner. The check assumed that the block was always non-nil, which
causes panics in our scheduler tests.
tgross added a commit that referenced this pull request Jul 31, 2023
…18100)

In #18054 we introduced a new field `render_templates` in the `restart`
block. Previously changes to the `restart` block were always non-destructive in
the scheduler but we now need to check the new field so that we can update the
template runner. The check assumed that the block was always non-nil, which
causes panics in our scheduler tests.
tgross added a commit that referenced this pull request Jul 31, 2023
…18100)

In #18054 we introduced a new field `render_templates` in the `restart`
block. Previously changes to the `restart` block were always non-destructive in
the scheduler but we now need to check the new field so that we can update the
template runner. The check assumed that the block was always non-nil, which
causes panics in our scheduler tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.6.x backport to 1.6.x release line theme/template
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants