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

Support merging multiple configs within a containerpilot.d directory #236

Open
jasonpincin opened this issue Oct 20, 2016 · 11 comments
Open

Comments

@jasonpincin
Copy link

jasonpincin commented Oct 20, 2016

Proposing that ContainerPilot supports reading multiple configuration files from a directory and merging them into a single JSON document to be used as it's config. I thought about implementing this as a pre-step in a container, but ultimately cannot due to the use of GOLANG templating. An example use case for this:

The Nginx ContainerPilot configuration has a lot of intelligence. It can configure itself for a Consul agent or going direct to a Consul server, it provides telemetry, and it will start co-processes required for LetsEncrypt automatically if the ACME_DOMAIN environment variable is set. Most likely however, a user will need to specify backends which will require them to override the entire configuration, losing the ability to easily configure LetsEncrypt as an example, as now they have to have an intimate understanding of how it's implemented in the container so that they can maintain the configuration themselves.

If instead, they could create a file along the lines of /etc/containerpilot.d/backends.json with the contents of:

{ 
  "backends": [
    { "name": "example", "poll": 7, "onChange": "reload" }
  ]
} 

The backends.json name is unimportant, any .json file in this directory should be parsed (in alphabetical order) and merged with the primary configuration file. They should be merged in a non-destructive manner - such that if two files defined backends, the backends arrays in each would be concatenated, not replaced.

This obviously would not work for non-array values, such as preStart, consul, etc. Perhaps as part of this refactor, it would be worth considering adding support for arrays on app-wide scripts like preStart, preStop, etc. A scalar value could be automatically converted to a single element array.

@jasonpincin jasonpincin changed the title proposal: conf.d style support for configuration file proposal: support containerpilot.d config directory Oct 20, 2016
@tgross
Copy link
Contributor

tgross commented Nov 17, 2016

After a lot of sidebar discussion we've come to the following proposal.

  • Abandon JSON in favor of a more human-friendly configuration language. HCL from Hashicorp has both composability, reasonable backwards compatibility with JSON, and human-friendliness.
  • During ContainerPilot configuration loading, we can check for files in /etc/containerpilot.d/ (a configurable location via the -config flag) and concatenate them together. Items are merged together using the existing semantics provided by HCL.

New Configuration Example

The full example configuration for ContainerPilot found in the docs would look like the following:

consul {
    host = consul.example.com:8500
    agent = true
    retryInterval = 10s
}

preStart {
    command = /usr/local/bin/preStart-script.sh {{.ENV_VAR_NAME}}
}

logging {
    level = INFO
    format = default
    output = stdout
}

stopTimeout = 5

preStop {
    command = /usr/local/bin/preStop-script.sh
}

postStop {
    command = /usr/local/bin/postStop-script.sh
}

service "app" {
    port = 80
    tll = 10
    heartbeat = 3
    tags = ["app", "prod"]
    interfaces = [
        "eth0",
        "eth1[1]",
        "192.168.0.0/16",
        "2001:db8::/64",
        "eth2:inet",
        "eth2:inet6",
        "inet",
        "inet6",
        "static:192.168.1.100"
        ]
}

health {
    service = "app"
    check = "/usr/bin/curl --fail -s -o /dev/null http://localhost/app"
    poll = 5
    timeout = "5s"
    onEvent {
        condition = "firstFail"
        command = "/usr/local/bin/do-on-first-fail"
    }
}

backend "nginx" {
    onChange = "/usr/local/bin/reload-nginx.sh"
    poll = 30
    timeout = "30s"
}

backend "app" {
    onChange = "/usr/local/bin/reload-app.sh"
    poll = 10
    timeout = "10s"
}

telemetry {
    port = 9090
    interfaces = ["eth0"]
}

sensor {
    name = "metric_id"
    help = "help text"
    type = "counter"
    poll = 5
    check = "/usr/local/bin/sensor.sh"
}

task "task1" {
    command = "/usr/local/bin/tash.sh arg1"
    frequency = "1500ms"
    timeout = "100ms"
}

coprocess "consul-template" {
    command = "consul-template -consul consul -template /tmp/template.ctmpl:/tmp/result"
    restarts = "unlimited"
}

Note the following decisions in the parsing of the combined file:

  • An end-user can add preStart, preStop, and postStop commands simply by adding a new file with the command. They’ll be run in order, in lexigraphical order of the files where they were added
  • Health checks are disengaged from the service definition. This allows multiple health checks to be added (ref Allow multiple health checks per service #245). If any of them fail then ContainerPilot will send an immediate Fail message to Consul and the state of the service will be switched to “failing”.
  • Overriding the service definition can be done by adding another file with the same service definition.
  • This configuration also includes the onEvent and consul-agent backend proposals.

@tgross tgross changed the title proposal: support containerpilot.d config directory Support containerpilot.d config directory Nov 17, 2016
This was referenced Nov 17, 2016
@fitz123
Copy link

fitz123 commented Nov 18, 2016

Quite brave decision. Not sure how good all of it would be. Looks like much more complication and magic is added. And I'm not really fan of both.
But nonetheless looking forward to try v3 and wish you all the best with it!

@tgross
Copy link
Contributor

tgross commented Nov 18, 2016

Looks like much more complication and magic is added. And I'm not really fan of both.

This was definitely my biggest worry in our internal discussions, but I think this design allows the "simple case" to remain simple and adds the complexity only as the end-user asks for it.

@tgross
Copy link
Contributor

tgross commented Nov 22, 2016

Removing the 3.x tag from this and moving it back into proposals after a discussion with @misterbisson. There's reason to believe we may want to go in a different direction in the long term, and without more specific use case examples it doesn't make sense to do this in the 3.x release.

@misterbisson
Copy link
Contributor

@jasonpincin I think you're right to look for ways to solve the problem. My hesitation here is about the full definition of the problem and wanting more use cases for us to consider before we build out something based on our current understanding.

@tgross
Copy link
Contributor

tgross commented Feb 17, 2017

Latest proposal is in RFD86

@tgross
Copy link
Contributor

tgross commented Apr 20, 2017

Now that #321 has been implemented for 3.0.0, I'm realizing that this feature does not have to be a blocker for 3.0.0 because it can be implemented backwards-compatibly. The config value can refer to either a file, in which case the behavior we have in the master branch currently will be preserved, or it can refer to a directory, in which case the not-yet-implemented behavior in this issue can be performed.

As such, I'm updating this issue to be v3.x

@tgross tgross removed their assignment May 22, 2017
@tgross tgross mentioned this issue Jun 2, 2017
@asteven
Copy link

asteven commented Aug 26, 2017

Do you have a ETA for this feature?
It would be very useful to have a base containerpilot+consul+consul-template container which not only has the binaries but also some initial config.

@jwreagor jwreagor changed the title Support containerpilot.d config directory Support merging multiple configs within a containerpilot.d directory Oct 10, 2017
@jwreagor
Copy link
Contributor

A key point in this is the merging aspect which arguable is something I haven't kept in mind when considering if this feature is required. Maybe I'll find the time soon to work up a hardened proposal for how the merging would work. If anyone has any ideas or libraries that do something similar, that would be much appreciated!

@gbmeuk
Copy link

gbmeuk commented Dec 19, 2017

We have another use case for this. We have some Ansible playbooks to allow people to easily create integrated Autopilotpattern (actually, it's our extension of it - Concierge Paradigm) containers easily, ready to go with an integrated environment.

The problem we have is that we generate a containerpilot.json5 file based on the variables given when running the playbook. This works well within the constraints of the code but in the provided functionality which gives people the ability to be able to provide their own custom templates for generating this file, the user has to copy the template we use to the user-defined directory and then amend it. We'd like to be able to keep all the platform integration part separate and simple allow people to extend upon what is generated (or overwrite all together still).

Originally I looked into seeing if there was a way to pass -config twice. E.g. containerpilot -config platform_config.json5 -config additional_config.json5. This is how I came across this issue.

Basically we'd like to have the ability to have a separation of concerns but still allow people to collaborate.

@Smithx10
Copy link

I've ran into this issue now. I have a base image that has common jobs, Currently I pass the boilerplate around to every consumer of my base image. Was there any new thoughts about implementing this?

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

No branches or pull requests

8 participants