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

Discussion: feature to allow passing environment variables from Pebble's own environment to services #125

Closed
jnsgruk opened this issue Jul 20, 2022 · 16 comments

Comments

@jnsgruk
Copy link
Member

jnsgruk commented Jul 20, 2022

This is part request, part discussion...!

Could we/should we support the ability for Pebble to pass elements of its own environment to running services?

For example. if I were to start pebble with SOME_VAR=foobar pebble run --hold, it would be interesting to allow a mechanism for passing SOMEVAR to the services it manages?

@jameinel
Copy link
Member

jameinel commented Jul 20, 2022 via email

@benhoyt
Copy link
Contributor

benhoyt commented Jul 20, 2022

We were planning a feature where we'd allow $SOMEVAR style interpolation in Pebble's environment variables, but I don't think we ever created an issue for it. Pebble config would allow such interpolations in the command and the other environment values. For example:

services:
  test:
    override: replace
    command: webserver -db $DBNAME -port $WEBPORT
    environment:
        DBUSER: bobby
        DBPASS: $DBPASS

Just a silly example, but that would run webserver with the command line options set from DBNAME and WEBPORT, as well as setting DBUSER to bobby and passing through DBPASS. Interpolations in environment variables in environment could reference other environment variables (regardless of order), and it would interpolate them in the correct order.

Strictly speaking this might break backwards compatibility, if people really do have $ in their commands or environment values, but that seems unlikely.

Thoughts?

@jnsgruk
Copy link
Member Author

jnsgruk commented Jul 21, 2022

I think the interpolation in other fields is probably a slightly different concern. The $ is probably fine provided we allow people to escape it? Using \$ for a literal $ seems pretty commonplace.

I was actually envisaging something less fancy at this point which could be a field in the service spec named inherit-env. This could either be a bool (i.e. inherit all environment variables from the environment in which pebble was launched), or a list of environment variables to inherit?

My use case is actually for firing things up on my desktop and inheriting variables like SWAYSOCK, but it has application for Kubernetes pods, too.

@hpidcock
Copy link
Member

hpidcock commented Jul 21, 2022

I think I would actually prefer something like:

   env-from: pebble | another-service
   environment:
     MY_OVERRIDE: new-value

or

  env-from:
    *: pebble
    SOME_KEY: other-service
  environment:
     MY_OVERRIDE: new-value

@jnsgruk
Copy link
Member Author

jnsgruk commented Jul 21, 2022

Yeh env-from: pebble is pretty cool. The env-from: another-service might be a lot harder, but would be very useful in some contexts.

@benhoyt
Copy link
Contributor

benhoyt commented Jul 22, 2022

I can see that env-from is "cool", but I find it a bit confusing and over-complicated. I'd like to keep things as simple as possible, and relatively explicit.

What about inherit-env: true | false for when you want to inherit all of Pebble's environment variables.

And then as a related but separate feature, add support for $SOMEVAR interpolation to the command and environment value fields. To me that reads fairly obviously when you're reading layer config, and you can still be explicit if you want to only inherit some variables by doing the following (inheriting only FOO and BAR):

environment:
  FOO: $FOO
  BAR: $BAR

@benhoyt
Copy link
Contributor

benhoyt commented Jul 22, 2022

Ha, I just looked at the code (as well as testing it) and rediscovered that Pebble already inherits the parent environment for services. The code starts with os.Environ() and then adds any vars from the service config's environment map:

	s.cmd.Env = os.Environ()
	for k, v := range s.config.Environment {
		s.cmd.Env = append(s.cmd.Env, k+"="+v)
	}

So inherit-env: true is already the default. Were you not seeing this work, @jnsgruk? I tested it just now with the following layer config:

services:
  test:
    override: replace
    command: /bin/sh -c 'echo FOO=$FOO BAR=$BAR >/home/ben/w/pebble/env.txt; sleep 300'
    startup: enabled
    environment:
      BAR: bar

Then you run Pebble (FOO=f BAR=b PEBBLE=~/pebble go run ./cmd/pebble run) and in a separate terminal, show the output file:

$ cat /home/ben/w/pebble/env.txt
FOO=f BAR=bar

Side note: I edited the above comments as I realized that the config field is environment, not env.

@benhoyt
Copy link
Contributor

benhoyt commented Jul 22, 2022

FWIW, @hpidcock initially suggested the $SOMEVAR interpolation for environment values in this comment ... I guess I never added that follow-up PR.

@jnsgruk
Copy link
Member Author

jnsgruk commented Jul 22, 2022

Ah, that's interesting. I'll need to go back and check. Indeed, I think interpolation will get us there in that case.

@jameinel
Copy link
Member

jameinel commented Jul 22, 2022 via email

@benhoyt
Copy link
Contributor

benhoyt commented Jul 22, 2022

I don't think we want a behavior which is "inherit everything without thinking about it", do we? It's convenient, but a very large hammer.

@jameinel Perhaps, though see above -- it is what we have now! :-)

@benhoyt
Copy link
Contributor

benhoyt commented Jul 29, 2022

In a voice discussion today, @hpidcock suggested that for one-shot commands and for exec health checks we could add a context: <service name> key, which would tell Pebble to inherit the "context" of the given service, including environment variables, user/group, and working directory.

We could have a special value like context: parent of context: - to mean inherit from Pebble itself.

@jnsgruk
Copy link
Member Author

jnsgruk commented Jul 29, 2022

In a voice discussion today, @hpidcock suggested that for one-shot commands and for exec health checks we could add a context: <service name> key, which would tell Pebble to inherit the "context" of the given service, including environment variables, user/group, and working directory.

We could have a special value like context: parent of context: - to mean inherit from Pebble itself.

That sounds sensible. Would tie in well with our discussions around debugging where we could allow users to get a shell in the context of a service for debugging purposes.

@gregory-schiano
Copy link

I went through the need of inheriting from the container environment during an exec when working on a charm. Basically the original docker container relies on one-shot shell scripts executed at startup. These scripts uses env vars declared with default value in the Dockerfile
Currently I had to reproduce all the env vars used by this scripts in the environment of my charm even if I didn't override their values.
Docker images and Kubernetes pods relies a lot on injected environment variables and many scripts (entrypoints) make use of them, without inheritance it leaves us 2 choices:

  • Reproduce the environment variables requires in our charm (which create kind of duplication)
  • Make our own docker image with scripts that doesn't rely on env vars (which creates more work for charm management)

I really like the idea of giving the possibility to pass the context, or select variables we'd like to pass from the context

@benhoyt
Copy link
Contributor

benhoyt commented Oct 18, 2022

We discussed some of this with Gustavo last night, and there was general agreement that it'd be good to add an API parameter context to pebble exec (command line arg --context), which would be the name of the service whose context to run the command in. It would use all the relevant context from the service definition, specifically environment variables and user and group.

We should also add a working-dir option to the service definition, like we have for pebble exec and exec health checks, and then this new context would use that too. Separate issue opened to add working-dir: #158

However, note that this doesn't resolve this specific issue, which is about services inheriting Pebble's own environment variables. So I'll open another new issue for the context feature. (#159)

@benhoyt
Copy link
Contributor

benhoyt commented Jul 12, 2023

Closing this. Per above, service execution already inherits environment variables (and #234 made that happen for exec and health checks too). Then in #246 we added "service context" for exec and health checks, allowing those command executions to run in the context of (inherit env vars, working-dir, and user/group) a specified service.

Separately, at some point we plan to add $VAR style interpolation to environment variable handling: #182

@benhoyt benhoyt closed this as completed Jul 12, 2023
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

No branches or pull requests

5 participants