Update consul-template to v0.24.1 and remove deprecated vault grace #7170
Conversation
|
@schmichael I needed the new template features for consul connect in a presentation. |
There was a problem hiding this comment.
Thanks @fredrikhgrelland !
For the next release we should emit a warning before removing the field entirely. I made comments that should guide you through the required changes.
Let me know if you don't have time to make the changes, and I can do it.
Thanks again!
|
We'll also need to add a changelog entry and section to https://github.com/hashicorp/nomad/blob/master/website/pages/guides/upgrade/upgrade-specific.mdx We can always do that in a followup PR. |
fredrikhgrelland
force-pushed
the
consul_template_upgrade
branch
from
a92beba
to
a4f98ab
Compare
fredrikhgrelland
force-pushed
the
consul_template_upgrade
branch
from
a4f98ab
to
01e9814
Compare
|
@schmichael I implemented the suggested changes and added the documentation. Let me know if there is anything else. |
|
Found a panic when posting job. Will investigate. |
| @@ -737,9 +737,6 @@ func (tmpl *Template) Canonicalize() { | |||
| if tmpl.Envvars == nil { | |||
| tmpl.Envvars = boolToPtr(false) | |||
| } | |||
| if tmpl.VaultGrace == nil { | |||
| tmpl.VaultGrace = timeToPtr(15 * time.Second) | |||
There was a problem hiding this comment.
Removing this is probably the source of panics/issues. It ensured VaultGrace was always initialized (never nil) in the HTTP API.
Removing this code is the right thing to do. We just need to fix any code that depended on VaultGrace always being non-nil. I'll investigate and try to help.
There was a problem hiding this comment.
Sorry haven't found time today to fix this up. The really sneaky place that converts public api structs to internal nomad/structs structs is hidden in the job endpoint here: https://github.com/hashicorp/nomad/blob/v0.10.4/command/agent/job_endpoint.go#L620
That may need updating to convert nil api VaultGrace values to 0 nomad/struct VaultGrace values.
There was a problem hiding this comment.
See comment below.
fredrikhgrelland
force-pushed
the
consul_template_upgrade
branch
from
01e9814
to
fb61a83
Compare
| @@ -374,6 +374,7 @@ func parseTemplates(result *[]*api.Template, list *ast.ObjectList) error { | |||
| ChangeMode: helper.StringToPtr("restart"), | |||
| Splay: helper.TimeToPtr(5 * time.Second), | |||
| Perms: helper.StringToPtr("0644"), | |||
| VaultGrace: helper.TimeToPtr(0), | |||
There was a problem hiding this comment.
Added default value in parser. Time.period can not be nil.
This allows for silent parsing when vault_grace is not set or set to 0. Warning when set to a non zero value.
fredrikhgrelland
force-pushed
the
consul_template_upgrade
branch
from
fb61a83
to
611d4e7
Compare
fredrikhgrelland
force-pushed
the
consul_template_upgrade
branch
from
611d4e7
to
24abfb0
Compare
|
@schmichael Apologies for all the pushes. I have added warnings to templates, and made changes to the documentation and change log to match the current situation. |
fredrikhgrelland
force-pushed
the
consul_template_upgrade
branch
from
24abfb0
to
26cca14
Compare
There was a problem hiding this comment.
Thanks for your hard work on this @fredrikhgrelland ! I can take over any test failures from here. I really appreciate you sticking with this. There is a lot of plumbing involved in jobspec handling.
Followup to #7170 - Moved canonicalization of VaultGrace back into `api/` package. - Fixed tests. - Made docs styling consistent.
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fixes #6946
Needed consul connect ca features of consul-template in nomad.