Skip to content

Order of env variables in sidecontainers changed #979

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

Closed
spohner opened this issue May 18, 2020 · 3 comments
Closed

Order of env variables in sidecontainers changed #979

spohner opened this issue May 18, 2020 · 3 comments

Comments

@spohner
Copy link
Contributor

spohner commented May 18, 2020

We use https://github.com/wrouesnel/postgres_exporter as a global sidecar to export metrics. In the manifest file we define environment variables based on the injected environment variables like this:

env:
- name: DATA_SOURCE_URI
  value: localhost/postgres?sslmode=disable
- name: DATA_SOURCE_USER
  value: $(POSTGRES_USER)
- name: DATA_SOURCE_PASS
  value: $(POSTGRES_PASSWORD)

With the merge of #890 the order of which environment variables are added changed. The order of the env variables is actually a part of the API spec according to kubernetes/kubernetes#40373 (could not find it in the spec docs directly).

By adding the common fields last in the env array a pattern like the one above is no longer supported and this could break some apps.

Should be an easy fix by swapping the order in the append method here

mergedEnv := append(container.Env, env...)
.

I will create a simple PR for this ASAP.

@FxKu
Copy link
Member

FxKu commented May 20, 2020

@fischerman can you also comment on this, as you are using are probably using this feature in prod?

@fischerman
Copy link
Contributor

Makes sense to have the operator environment variables first. Didn't think about that. PR from @spohner looks good!

@frittentheke
Copy link
Contributor

I sent in the PR #946 which sources env variables from a secret. There I particularly allow to "override" certain variables related to backups and buckets - but in general those used by the operator for core functionality should come first if there is not use-case and this would rather create unintended effects.

@FxKu FxKu closed this as completed Jun 4, 2020
teutonet pushed a commit to teutonet/postgres_exporter that referenced this issue Jun 18, 2020
workaround for a better postgres-operator-integration until we get the fix from zalando/postgres-operator#979
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

4 participants