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

RFC: add a --var(iable) flag to hab sup start #1938

Closed
bixu opened this issue Mar 10, 2017 · 24 comments
Closed

RFC: add a --var(iable) flag to hab sup start #1938

bixu opened this issue Mar 10, 2017 · 24 comments

Comments

@bixu
Copy link
Contributor

bixu commented Mar 10, 2017

I've run into a number of cases when I really wanted a syntax like hab sup start --var port=22 when running a package - cases where, for various reasons, reading in or setting environment variables is a bit awkward (for example variable names that include . characters, which can't be handled by Bash).

@mwrock
Copy link
Contributor

mwrock commented Mar 10, 2017

Have you tried this syntax:

HAB_MYTUTORIALAPP='message = "Habitat rocks!"' hab start <origin>/<packagename>

As mentioned on https://www.habitat.sh/docs/run-packages-apply-config-updates/ ?

@bixu
Copy link
Contributor Author

bixu commented Mar 10, 2017 via email

@eeyun
Copy link
Contributor

eeyun commented Mar 14, 2017

In our discussion during triage we all felt like this Issue really touches on something deeper in our UX and usability. Even if perhaps the implementation suggested here isn't what we go with I think we're all in agreement that theres some pain around this.

@smurawski
Copy link
Contributor

Since it can be tough in some orchestration systems to pass environment variables with valid TOML (which requires line breaks). We could look at some convention for taking JSON and converting that to TOML (we should have the conversion functions already in the codebase), but I'm sure there are scenarios with valid json that isn't valid toml, etc..

@reset reset added this to the Help Wanted milestone Mar 21, 2017
@bodymindarts
Copy link
Contributor

I agree that there is an issue here.
I had a discussion this morning where we were wishing there would be a way to specify individual settings via env var like:

HAB_SERVICE_USER="foo"
HAB_SERVICE_PASS="bar"

Either way this is implemented it is clear that needing to pass in a toml file as 1 variable has its disadvantages.

@bodymindarts
Copy link
Contributor

@bixu @eeyun

I have written a little alternative entrypoint script that can be used to pass in individual setting via environment variables.
It can be used by mounting the script into the container and then running docker with --entrypoint /path/to/hab_entry.sh
See more info here

@eeyun
Copy link
Contributor

eeyun commented Apr 25, 2017

I'm glad that theres a thing that folks can use on the interim but I sort of feel like since compose files are the primary interface for interacting with Swarm, we might want to consider a native feature to resolve this as opposed to an external bolt on.

@eeyun
Copy link
Contributor

eeyun commented Aug 3, 2017

I've got a PR that I'm currently testing that would allow the ENV var interface to accept JSON as well as TOML which would effectively provide a solution to some of the pain folks are trying to solve with this RFC. But in talking with some of the core maintainers I'm just not 100% certain it's the right abstraction. @adamhjk made a great point about habitat at large which was that we (so far) have done a super good job of keeping our user facing APIs tightly focused - e.g. not a bajillion different interfaces to do the same things.

I bring this up because I'd like to hear some input from the folks who care about this RFC before I open this PR for review. If there are other lightweight ideas that folks have thought up but haven't had time to implement I'm happy to look into doing so but if we have a consensus that having json support ONLY FOR ENV VAR CONFIGURATION INJECTION is a reasonable change then i'll go ahead and get this in.

@srenatus
Copy link
Contributor

srenatus commented Aug 4, 2017

I, too, would like getting HAB_APPNAME set in a docker-compose.yml. (I'm not hijacking this thread, it's just that @eeyun's JSON-in-ENV proposal also presents a solution for "my" issue here)

I'm also not sure if I'd like to have JSON in the mix, just for that one thing. (But I could imagine a wrapper that creates the docker-compose.yml for me, converting some hab_service_name.toml files it finds... 🤔 )

Other alternatives could be:

  • base64-encoding/decoding of the TOML for HAB_APPNAME
  • fixing docker-compose -- there's an issue for multiline env vars here: support newlines in env files docker/compose#3527, it would indeed be nice to just write:
        environment:
         - HAB_PKGNAME: |
             foo = "bar"
             [web]
             http = "0.0.0.0:11111"

Also, there's another exception, it seems: if you have a table, you can pass in multiple fields of that table using this TOML syntax in docker-compose.yml:

  pkgname:
    image: $HAB_ORIGIN/pkgname
    environment:
      - HAB_PKGNAME=web={ http = "0.0.0.0:11111", bar = "baz" }

However, I couldn't get anything like that work for a TOML with "top-level keys", for example

foo = "bar"
[web]
http = "0.0.0.0:11111"

So, this restriction seems like an odd workaround :/

Also, I've tried using env files, but it didn't make any difference.

@eeyun
Copy link
Contributor

eeyun commented Aug 4, 2017 via email

@bixu
Copy link
Contributor Author

bixu commented Aug 4, 2017

I think I’m missing something - how does JSON support ease the Cli experience the way a new flag would?

@eeyun
Copy link
Contributor

eeyun commented Aug 4, 2017 via email

@scotthain
Copy link
Contributor

After thinking about this I don't have a strong leaning either way (full JSON vs command line flags). From a beginner's debugging/testing/figuring stuff out perspective, I do think that having command line options to pass config in would be super nice, if nothing else to verify that the plan.sh and your application structure is working as expected. That's not a very convincing argument to break the overall design though :)

@eeyun
Copy link
Contributor

eeyun commented Aug 7, 2017

We do have alternative ways to pass in configuration options @scotthain. You can hab config apply if a custom TOML file is available, or you can --config-from which makes the same assumption as config apply.

I think to clarify my thoughts on the issue this is specifically a detriment to anyone following 12 Factor patterns for deploying their apps. Lots of folks use that environment variable API to do this kind of configuration but with our current implementation, it's effectively useless. I'm also OK with having this API remain in its current state but I think from the perspective of adoption we want to maximize the operability of the tool and adding something to provide a similar experience/outcome is pretty important.

@scotthain
Copy link
Contributor

@eeyun I think what might help (at least from a beginner's perspective) is having a documented path that will work for the scenario spelled out, and options given after that for the other ways that are possible. Given that the 2 main ways I've been learning (enter studio, build, export variables, svc start or enter studio, build, export, docker-compose) have slightly different idiosyncrasies, it would be cool if there was a documented way (somewhere, not sure where that belongs) to make it work since I doubt having 2+ variables is going to be uncommon.

@eeyun
Copy link
Contributor

eeyun commented Aug 7, 2017

100% Agreement on that. Also, I don't think that arbitrates the pain that users are experiencing here so we should attempt to decide on an implementation to help with that pain point.

@adamhjk
Copy link
Contributor

adamhjk commented Aug 7, 2017

I favor the JSON approach. Here is my thinking:

  1. Anything you can express in TOML you can express in JSON.
  2. People understand how to read/write it.
  3. In general, it's easily quotable by the shell.

The idea of command line flags doesn't work very well for me, since you immediately get into "how do I pass to complex flags?". If the answer here is "put json in this spot" then at least we have one single answer.

@scotthain
Copy link
Contributor

JSON seems most extensible, and now that I think about using it more it seems pretty nice. My use case would like sorta like this:

HAB_MYTHING={port='9000',app={header='foo',response='bar'}}

Which is super easy to reason about. (or am I misinterpreting what y'alls idea is?)

@adamhjk
Copy link
Contributor

adamhjk commented Aug 7, 2017

yes - that's it exactly. Of course, JSON sucks as a format, so those single quotes aren't valid. This is why my heart hurts.

But for this use case, its the right compromise.

@scotthain
Copy link
Contributor

@adamhjk we should just switch to fully typedef'd xml :trollface:

All joking aside, I'm 👍 on JSON

@eeyun
Copy link
Contributor

eeyun commented Aug 7, 2017 via email

@eeyun
Copy link
Contributor

eeyun commented Aug 10, 2017

Resolved by #2887

@eeyun eeyun closed this as completed Aug 10, 2017
@srenatus
Copy link
Contributor

For future reference, found a way to use TOML in the env var in docker-compose after all:

What I've tried to use, but what didn't work:

  fruit-service:
    image: srenatus/fruit-service:latest
    environment:
      HAB_FRUIT_SERVICE: |
      [[fruits]]
      type = "something"

Now, I've been shown a working example, and from that I gathered you need two extra spaces indentation -- this works:

  fruit-service:
    image: srenatus/fruit-service:latest
    environment:
      HAB_FRUIT_SERVICE: |
        [[fruits]]
        type = "something"

@eeyun
Copy link
Contributor

eeyun commented Aug 22, 2017 via email

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

9 participants