Skip to content

add template-error-fatal#1420

Merged
eikenb merged 1 commit into
hashicorp:masterfrom
phemmer:template-error-fatal
Feb 3, 2022
Merged

add template-error-fatal#1420
eikenb merged 1 commit into
hashicorp:masterfrom
phemmer:template-error-fatal

Conversation

@phemmer
Copy link
Copy Markdown
Contributor

@phemmer phemmer commented Oct 29, 2020

This adds a few template-error-fatal options to allow consul-template to continue running in the event of an error while rendering a template.
The option is exposed individually within each template, globally within the config, and as a global command line flag. The individual template setting overrides the global config setting, and the command line flag overrides all.

No tests yet, but I can create them upon review.

Closes #1419 #1289

@hashicorp-cla
Copy link
Copy Markdown

hashicorp-cla commented Oct 29, 2020

CLA assistant check
All committers have signed the CLA.

@phemmer phemmer force-pushed the template-error-fatal branch from 00191db to 8b2067e Compare October 29, 2020 20:12
Comment thread manager/runner.go
runCtx.depsMap[d.String()] = d
}
}
event.UsedDeps = lastEvent.UsedDeps
Copy link
Copy Markdown
Contributor Author

@phemmer phemmer Oct 29, 2020

Choose a reason for hiding this comment

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

The whole handling of event & lastEvent in error cases could probably use extra scrutiny. Everything seems to work fine, but there could be something I missed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This isn't the prettiest thing in the world but, given the code it must interact with, it is probably as good as it can be. In other words, I think this works fine. It should also get refactored as part of the work to port consul-template to use the hcat library, as that library manages dependencies differently. I think it won't even be needed.

Anyways... extra scrutiny given and 👍.

@eikenb eikenb requested a review from a team October 29, 2020 20:16
@crypt1d
Copy link
Copy Markdown

crypt1d commented Nov 20, 2020

hi team,

Any chance we can have this feature soon? We had another incident caused by a failed consul template and honestly I'm pretty close to dropping the tool altogether.

@lornasong
Copy link
Copy Markdown
Contributor

@phemmer, thanks so much for your contribution! Sorry for the delay. This is definitely a great feature that the community will benefit from.

@crypt1d, apologies for the delay as well. We definitely don’t want consul-template to be a source of pain for our users so appreciate your feedback on the urgency of this feature.

I took a first pass reviewing this and it looks good to me and works as expected. I’ll leave this to others on my team for any further feedback. Thanks!

@eikenb
Copy link
Copy Markdown
Contributor

eikenb commented Apr 29, 2021

Hey @phemmer,

I'm almost ready to merge this and only the conflict needs to be handled. The conflict is in the README changes as we moved the relevant documentation from the readme to docs/configuration.md since the PR was submitted.

Would you like to make this change or would you prefer me to do it? I ask as I don't think I can preserve attribution when making that change and don't want to steal that from you. So I want to give you the opportunity to make the changes.

Thanks again for working on this.

Copy link
Copy Markdown
Contributor

@eikenb eikenb left a comment

Choose a reason for hiding this comment

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

Once the conflict is resolved, this will be ready to merge.

@phemmer
Copy link
Copy Markdown
Contributor Author

phemmer commented Feb 4, 2022

Oh, hey, I somehow either missed that comment, or it completely fell off my radar, cause I don't remember it. Sorry about that. Thanks for taking care of it though.

@phemmer phemmer deleted the template-error-fatal branch February 4, 2022 00:43
@eikenb
Copy link
Copy Markdown
Contributor

eikenb commented Feb 7, 2022

No problem @phemmer, happy to take care of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Template errors should not be fatal

5 participants