Skip to content

Conversation

@ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Jul 8, 2024

What I did
restore setEnvWithDotEnv to load .env into os.Env so the many places we rely on os.LookupEnv aren't borken
this also restore support for configuring buildx using env var set in .env

includes e2e test + a tiny fix to include (stopped) one-off containers as "orphans"

Related issue
closes #11967

(not mandatory) A picture of a cute animal, if possible in relation to what you did
image

@ndeloof ndeloof requested review from a team, glours and jhrotko and removed request for a team July 8, 2024 08:17
@ndeloof ndeloof force-pushed the setEnvWithDotEnv branch 4 times, most recently from 258b86e to 68d5d03 Compare July 8, 2024 14:45
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloof ndeloof force-pushed the setEnvWithDotEnv branch from 68d5d03 to 1a28c93 Compare July 8, 2024 15:04
services := append(project.ServiceNames(), project.DisabledServiceNames()...)
return func(c moby.Container) bool {
// One-off container
v, ok := c.Labels[api.OneoffLabel]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: what is the oneOffLabel?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is used to distinguish containers created by docker compose run vs actual services created by up

lines = Lines(res.Stdout())
assert.Equal(t, lines[len(lines)-1], "Hello one more time", res.Stdout())
assert.Assert(t, !strings.Contains(res.Combined(), "orphan"))
assert.Assert(t, strings.Contains(res.Combined(), "orphan"))
Copy link
Contributor

@jhrotko jhrotko Jul 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: was this a bug before your chnages? :o

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more or less. We only considered "orphans" for service container not declared in current compose model. This PR also consider stopped run containers as orphaned

Copy link
Contributor

@jhrotko jhrotko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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.

[BUG] 2.28.1 multiple COMPOSE_* vars no longer read from .env file

3 participants