-
Notifications
You must be signed in to change notification settings - Fork 522
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
Add "parameterize" setting generator #637
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗝
This push fixes the typo caught by zmrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
☃️
31024c3
to
61959c2
Compare
This push just rebases on the changes in #636. |
This push updates the settings-committer docstring I missed, as caught by bcressey. |
This push addresses #637 (comment) by moving to handlebars-based templating inside the template metadata key, rather than using substring replacement from the (now-removed) "parameters" metadata key. It also addresses his naming request by renaming the generator to "schnauzer" :P Since this is nearly identical to what thar-be-settings did, I moved the common functions into schnauzer's library, which t-b-s and the schnauzer binary now use. Testing still healthy, as in description. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
🏋
61959c2
to
846a545
Compare
This push just rebases on a typo fix from #636. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🍟
This push addresses @bcressey's concern by changing the API path to "templates". I confirmed that new AMIs and the new path worked. |
This lets settings be generated based on a template that reference other settings. The first example is regional container image URIs, where we replace part with the value of settings.aws.region. The template is stored as a new metadata key, next to the setting-generator metadata, which schnauzer will read from the API. For example, if the template is "foo-{{ settings.bar }}" and "settings.bar" is set to "baz", then the setting output will be "foo-baz". To be able to reference other settings from sundog-spawned generators, we need to commit settings that have already been patched, e.g. from user data. To do this, this commit eliminates the settings-committer service, instead using the settings-committer binary as an ExecStartPre on any service that requires committed settings.
This push just rebased on develop; the only change was removing the first-party exception for schnauzer from cargo-deny. |
This lets settings be generated based on a template and parameters that
reference other settings. The first example is regional container image URIs,
where we replace part with the value of settings.aws.region.
The template and parameters are stored as new metadata keys, next to the
setting-generator metadata, which parameterize will read from the API. For
example, if the template is "foo-BAR" and the parameters are '{"BAR":
"settings.bar"}' and "settings.bar" is set to "baz", then the setting output
will be "foo-baz".
To be able to reference other settings from sundog-spawned generators, we need
to commit settings that have already been patched, e.g. from user data. To do
this, this commit eliminates the settings-committer service, instead using the
settings-committer binary as an ExecStartPre on any service that requires
committed settings.
Builds on #636.
Testing done:
Tested with the following PR that enables this feature for container image URIs.
Instances in us-west-2 and us-east-1 were both healthy, pods ran OK, and I saw no errors in the logs.
In both regions, parameterize correctly used the region to build image URIs and they launched OK.
us-east-1:
us-west-2: