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.

Sign up for GitHub

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

Jump to bottom

default client.template.wait configuration is not passed to template runner #20164

Closed
tgross opened this issue Mar 19, 2024 · 0 comments · Fixed by #20165
Closed

default client.template.wait configuration is not passed to template runner #20164

tgross opened this issue Mar 19, 2024 · 0 comments · Fixed by #20165
Assignees
@tgross
Labels
theme/config theme/consul-template theme/template type/bug
Milestone
1.7.x

Comments

@tgross
Copy link
Member

tgross commented Mar 19, 2024

The documentation for client.template.wait says that the default value is { min = "5s" max = "4m" } but while debugging #20163 I discovered that the default value is not being passed into the template runner. This potentially results in extra load to upstream dependencies, in particular Vault which doesn't have blocking queries.
@tgross tgross added this to the 1.7.x milestone Mar 19, 2024
@tgross tgross self-assigned this Mar 19, 2024
@tgross tgross mentioned this issue Mar 19, 2024
Merged
tgross added a commit that referenced this issue Mar 19, 2024
@tgross
config: fix client.template config merging with defaults
9eda3bf 
When loading the client configuration, the user-specified `client.template`
block was not properly merged with the default values. As a result, if the user
set any `client.template` field, all the other field defaulted to their zero
values instead of the documented defaults.

This changeset:
* Adds the missing `Merge` method for the client template config and ensures
  it's called.
* Makes a single source of truth for the default template configuration,
  instead of two different constructors.
* Extends the tests to cover the merge of a partial block better.

Fixes: #20164
tgross added a commit that referenced this issue Mar 20, 2024
@tgross
config: fix client.template config merging with defaults (#20165)
7b9bce2 
When loading the client configuration, the user-specified `client.template`
block was not properly merged with the default values. As a result, if the user
set any `client.template` field, all the other field defaulted to their zero
values instead of the documented defaults.

This changeset:
* Adds the missing `Merge` method for the client template config and ensures
  it's called.
* Makes a single source of truth for the default template configuration,
  instead of two different constructors.
* Extends the tests to cover the merge of a partial block better.

Fixes: #20164
This was referenced Mar 20, 2024
Merged
Merged
Merged
tgross added a commit that referenced this issue Mar 20, 2024
@tgross
config: fix client.template config merging with defaults (#20165)
b949f22 
When loading the client configuration, the user-specified `client.template`
block was not properly merged with the default values. As a result, if the user
set any `client.template` field, all the other field defaulted to their zero
values instead of the documented defaults.

This changeset:
* Adds the missing `Merge` method for the client template config and ensures
  it's called.
* Makes a single source of truth for the default template configuration,
  instead of two different constructors.
* Extends the tests to cover the merge of a partial block better.

Fixes: #20164
tgross added a commit that referenced this issue Mar 20, 2024
@tgross
config: fix client.template config merging with defaults (#20165)
f3ed726 
When loading the client configuration, the user-specified `client.template`
block was not properly merged with the default values. As a result, if the user
set any `client.template` field, all the other field defaulted to their zero
values instead of the documented defaults.

This changeset:
* Adds the missing `Merge` method for the client template config and ensures
  it's called.
* Makes a single source of truth for the default template configuration,
  instead of two different constructors.
* Extends the tests to cover the merge of a partial block better.

Fixes: #20164
tgross added a commit that referenced this issue Mar 20, 2024
@tgross
config: fix client.template config merging with defaults (#20165)
bb6fff4 
When loading the client configuration, the user-specified `client.template`
block was not properly merged with the default values. As a result, if the user
set any `client.template` field, all the other field defaulted to their zero
values instead of the documented defaults.

This changeset:
* Adds the missing `Merge` method for the client template config and ensures
  it's called.
* Makes a single source of truth for the default template configuration,
  instead of two different constructors.
* Extends the tests to cover the merge of a partial block better.

Fixes: #20164
tgross added a commit that referenced this issue Mar 20, 2024
@hc-github-team-nomad-core @tgross
Backport of config: fix client.template config merging with defaults …
ffdfd6d 
…into release/1.7.x (#20170)

When loading the client configuration, the user-specified `client.template`
block was not properly merged with the default values. As a result, if the user
set any `client.template` field, all the other field defaulted to their zero
values instead of the documented defaults.

This changeset:
* Adds the missing `Merge` method for the client template config and ensures
  it's called.
* Makes a single source of truth for the default template configuration,
  instead of two different constructors.
* Extends the tests to cover the merge of a partial block better.

Fixes: #20164

Co-authored-by: Tim Gross <[email protected]>
tgross added a commit that referenced this issue Mar 20, 2024
@hc-github-team-nomad-core @tgross
Backport of config: fix client.template config merging with defaults …
b3e15c8 
…into release/1.6.x (#20169)

When loading the client configuration, the user-specified `client.template`
block was not properly merged with the default values. As a result, if the user
set any `client.template` field, all the other field defaulted to their zero
values instead of the documented defaults.

This changeset:
* Adds the missing `Merge` method for the client template config and ensures
  it's called.
* Makes a single source of truth for the default template configuration,
  instead of two different constructors.
* Extends the tests to cover the merge of a partial block better.

Fixes: #20164

Co-authored-by: Tim Gross <[email protected]>
tgross added a commit that referenced this issue Mar 20, 2024
@hc-github-team-nomad-core @tgross
Backport of config: fix client.template config merging with defaults …
ea60aac 
…into release/1.5.x (#20168)

When loading the client configuration, the user-specified `client.template`
block was not properly merged with the default values. As a result, if the user
set any `client.template` field, all the other field defaulted to their zero
values instead of the documented defaults.

This changeset:
* Adds the missing `Merge` method for the client template config and ensures
  it's called.
* Makes a single source of truth for the default template configuration,
  instead of two different constructors.
* Extends the tests to cover the merge of a partial block better.

Fixes: #20164

Co-authored-by: Tim Gross <[email protected]>
philrenaud pushed a commit that referenced this issue Apr 18, 2024
@tgross @philrenaud
config: fix client.template config merging with defaults (#20165)
c882006 
When loading the client configuration, the user-specified `client.template`
block was not properly merged with the default values. As a result, if the user
set any `client.template` field, all the other field defaulted to their zero
values instead of the documented defaults.

This changeset:
* Adds the missing `Merge` method for the client template config and ensures
  it's called.
* Makes a single source of truth for the default template configuration,
  instead of two different constructors.
* Extends the tests to cover the merge of a partial block better.

Fixes: #20164
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tgross tgross

Labels
theme/config theme/consul-template theme/template type/bug
Projects
None yet
Milestone
1.7.x
Development

Successfully merging a pull request may close this issue.

config: fix client.template config merging with defaults hashicorp/nomad
1 participant
@tgross