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

Add a runtime flag to spec files #116

Closed

Conversation

andy-paine
Copy link

@andy-paine andy-paine commented Aug 13, 2020

Only one of CONCOURSE_CONTAINERD_* and CONCOURSE_GARDEN_* sets of
options should be set. Add a flag in the spec file that the envgen
helper uses to generate ERB files that correctly represent this need.
Defaults to Guardian since that is the implicit default in Concourse.

Fixes #115

Questions:

  • In the README it recommends don't define a default value that mirrors a default set by the Concourse binary but I don't really see any way to do this without mirroring the default runtime
  • I only added the env vars for a specific runtime since (currently), the only runtime specific configuration is env vars. Should I include the env files too just in case someone adds one in the future?

Only one of CONCOURSE_CONTAINERD_* and CONCOURSE_GARDEN_* sets of
options should be set. Add a flag in the spec file that the envgen
helper uses to generate ERB files that correctly represent this need.
Defaults to Guardian since that is the implicit default in Concourse.

Fixes concourse#115

Signed-off-by: Andy Paine <[email protected]>
@muntac
Copy link
Contributor

muntac commented Sep 8, 2020

@andy-paine Thanks a lot for the PR! We're considering an alternative approach where we don't set the default values in the BOSH release, and instead do so in the binary.

Your blog post about the max container limit was very helpful in thinking about the problem. Thanks for that! I'll come back to this PR once we've come to a conclusion on the other approach.

Update:
We settled on the alternative approach of having the defaults in the binary instead. Closing this PR.

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.

Garden properties are always included even when using containerd runtime
3 participants