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

job: prevent partial update on error #412

Merged
merged 2 commits into from
Dec 19, 2023
Merged

Conversation

lgfa29
Copy link
Contributor

@lgfa29 lgfa29 commented Dec 19, 2023

The default behaviour of the Terraform SDK is to copy the plan result into state which could result in partial state updates, where Terraform state is updated, but the actual resource state is not, in case of an error during the apply.

This is normally not an issue because resources are expected to undo these changes on state refresh. Any partial update is reconciled with the actual resource state.

But due to #1, the nomad_job resource is not able to properly reconcile on refresh, causing the partial update to prevent further applies unless the configuration is also changed.

This commit uses the d.Partial() method to signal to Terraform that any state changes should be rolledback in case of an error.

Closes #385

Copy link
Member

@gulducat gulducat left a comment

Choose a reason for hiding this comment

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

⛅ (this is a "partly sunny" emoji 😋)

CHANGELOG.md Outdated Show resolved Hide resolved
@@ -409,6 +410,10 @@ func resourceJobRegister(d *schema.ResourceData, meta interface{}) error {
return fmt.Errorf("error applying jobspec: %s", err)
}

if !d.IsNewResource() {
d.Partial(false)
Copy link
Member

Choose a reason for hiding this comment

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

This is after the job is successfully registered, but before its deployment has succeeded (assuming detach = false). I suppose this makes sense, because the resource is a job, not a deployment, but I guess I want to flag that a failing deployment would still show "no changes" on a subsequent apply, right? So in that case, one would need to taint or otherwise destroy and re-apply the job, if the fix for the failing deployment was something other than the job resource config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right, but that's Nomad's behaviour. The goal of the TF provider is to update the job state in Nomad. Once that's complete Nomad is responsible for the deployment. Applying again has not effect the same way as running nomad job run would also not have any effect.

The default behaviour of the Terraform SDK is to copy the plan result
into state which could result in partial state updates, where Terraform
state is updated, but the actual resource state is not, in case of an
error during the apply.

This is normally not an issue because resources are expected to undo
these changes on state refresh. Any partial update is reconciled with
the actual resource state.

But due to #1, the `nomad_job` resource is not able to properly
reconcile on refresh, causing the partial update to prevent further
applies unless the configuration is also changed.

This commit uses the `d.Partial()` method to signal to Terraform that
any state changes should be rolledback in case of an error.
@lgfa29 lgfa29 force-pushed the b-prevent-partial-job-update branch from 658adcb to 9230318 Compare December 19, 2023 20:54
@lgfa29 lgfa29 merged commit db1898a into main Dec 19, 2023
3 checks passed
@lgfa29 lgfa29 deleted the b-prevent-partial-job-update branch December 19, 2023 21:00
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.

nomad_job : provider updates state on failure
2 participants