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

Backport of template: fix panic in change_mode=script on client restart into release/1.8.x #24061

Conversation

hc-github-team-nomad-core
Copy link
Contributor

Backport

This PR is auto-generated from #24057 to be assessed for backporting due to the inclusion of the label backport/1.8.x.

The below text is copied from the body of the original PR.


When we introduced change_mode=script to templates, we passed the driver handle down into the template manager so we could call its Exec method directly. But the lifecycle of the driver handle is managed by the taskrunner and isn't available when the template manager is first created. This has led to a series of patches trying to fixup the behavior (#15915, #15192, #23663, #23917). Part of the challenge in getting this right is using an interface to avoid the circular import of the driver handle.

But the taskrunner already has a way to deal with this problem using a "lazy handle". The other template change modes already use this indirectly through the Lifecycle interface. Change the driver handle Exec call in the template manager to a new Lifecycle.Exec call that reuses the existing behavior. This eliminates the need for the template manager to know anything at all about the handle state.

Fixes: #24051


Overview of commits

@hc-github-team-nomad-core hc-github-team-nomad-core force-pushed the backport/b-panic-template-change-script/monthly-funky-wasp branch from 6b76e5f to 6d90dd4 Compare September 25, 2024 12:59
@tgross tgross merged commit bd88eb2 into release/1.8.x Sep 25, 2024
20 checks passed
@tgross tgross deleted the backport/b-panic-template-change-script/monthly-funky-wasp branch September 25, 2024 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants