tyler.durden
/
compose
mirror of https://github.com/docker/compose.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

up: handle various attach use cases better 

By default, `compose up` attaches to all services (i.e.
shows log output from every associated container). If
a service is specified, e.g. `compose up foo`, then
only `foo`'s logs are tailed. The `--attach-dependencies`
flag can also be used, so that if `foo` depended upon
`bar`, then `bar`'s logs would also be followed. It's
also possible to use `--no-attach` to filter out one
or more services explicitly, e.g. `compose up --no-attach=noisy`
would launch all services, including `noisy`, and would
show log output from every service _except_ `noisy`.
Lastly, it's possible to use `up --attach` to explicitly
restrict to a subset of services (or their dependencies).

How these flags interact with each other is also worth
thinking through.

There were a few different connected issues here, but
the primary issue was that running `compose up foo` was
always attaching dependencies regardless of `--attach-dependencies`.

The filtering logic here has been updated so that it
behaves predictably both when launching all services
(`compose up`) or a subset (`compose up foo`) as well
as various flag combinations on top of those.

Notably, this required making some changes to how it
watches containers. The logic here between attaching
for logs and monitoring for lifecycle changes is
tightly coupled, so some changes were needed to ensure
that the full set of services being `up`'d are _watched_
and the subset that should have logs shown are _attached_.
(This does mean faking the attach with an event but not
actually doing it.)

While handling that, I adjusted the context lifetimes
here, which improves error handling that gets shown to
the user and should help avoid potential leaks by getting
rid of a `context.Background()`.

Signed-off-by: Milas Bowman <milas.bowman@docker.com>
This commit is contained in:
Milas Bowman 2023-08-17 17:43:13 -04:00 committed by Nicolas De loof
parent 792afb8d13
commit caad72713b
6 changed files with 182 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

84
cmd/compose/up.go
View File

@ -18,7 +18,9 @@ package compose
import (
"context"
"errors"
"fmt"
"strings"
"time"
"github.com/compose-spec/compose-go/types"
@ -83,7 +85,10 @@ func upCommand(p *ProjectOptions, streams api.Streams, backend api.Service) *cob
RunE: p.WithServices(func(ctx context.Context, project *types.Project, services []string) error {
create.ignoreOrphans = utils.StringToBool(project.Environment[ComposeIgnoreOrphans])
if create.ignoreOrphans && create.removeOrphans {
return fmt.Errorf("%s and --remove-orphans cannot be combined", ComposeIgnoreOrphans)
return fmt.Errorf("cannot combine %s and --remove-orphans", ComposeIgnoreOrphans)
}
if len(up.attach) != 0 && up.attachDependencies {
return errors.New("cannot combine --attach and --attach-dependencies")
}
return runUp(ctx, streams, backend, create, up, project, services)
}),
@ -108,12 +113,12 @@ func upCommand(p *ProjectOptions, streams api.Streams, backend api.Service) *cob
flags.BoolVar(&up.noDeps, "no-deps", false, "Don't start linked services.")
flags.BoolVar(&create.recreateDeps, "always-recreate-deps", false, "Recreate dependent containers. Incompatible with --no-recreate.")
flags.BoolVarP(&create.noInherit, "renew-anon-volumes", "V", false, "Recreate anonymous volumes instead of retrieving data from the previous containers.")
flags.BoolVar(&up.attachDependencies, "attach-dependencies", false, "Attach to dependent containers.")
flags.BoolVar(&create.quietPull, "quiet-pull", false, "Pull without printing progress information.")
flags.StringArrayVar(&up.attach, "attach", []string{}, "Attach to service output.")
flags.StringArrayVar(&up.noAttach, "no-attach", []string{}, "Don't attach to specified service.")
flags.StringArrayVar(&up.attach, "attach", []string{}, "Restrict attaching to the specified services. Incompatible with --attach-dependencies.")
flags.StringArrayVar(&up.noAttach, "no-attach", []string{}, "Do not attach (stream logs) to the specified services.")
flags.BoolVar(&up.attachDependencies, "attach-dependencies", false, "Automatically attach to log output of dependent services.")
flags.BoolVar(&up.wait, "wait", false, "Wait for services to be running|healthy. Implies detached mode.")
flags.IntVar(&up.waitTimeout, "wait-timeout", 0, "timeout waiting for application to be running|healthy.")
flags.IntVar(&up.waitTimeout, "wait-timeout", 0, "Maximum duration to wait for the project to be running|healthy.")
return upCmd
}
@ -158,37 +163,6 @@ func runUp(ctx context.Context, streams api.Streams, backend api.Service, create
return err
}
var consumer api.LogConsumer
if !upOptions.Detach {
consumer = formatter.NewLogConsumer(ctx, streams.Out(), streams.Err(), !upOptions.noColor, !upOptions.noPrefix, upOptions.timestamp)
}
attachTo := utils.Set[string]{}
if len(upOptions.attach) > 0 {
attachTo.AddAll(upOptions.attach...)
}
if upOptions.attachDependencies {
if err := project.WithServices(attachTo.Elements(), func(s types.ServiceConfig) error {
if s.Attach == nil || *s.Attach {
attachTo.Add(s.Name)
}
return nil
}); err != nil {
return err
}
}
if len(attachTo) == 0 {
if err := project.WithServices(services, func(s types.ServiceConfig) error {
if s.Attach == nil || *s.Attach {
attachTo.Add(s.Name)
}
return nil
}); err != nil {
return err
}
}
attachTo.RemoveAll(upOptions.noAttach...)
create := api.CreateOptions{
Services: services,
RemoveOrphans: createOptions.removeOrphans,
@ -204,14 +178,48 @@ func runUp(ctx context.Context, streams api.Streams, backend api.Service, create
return backend.Create(ctx, project, create)
}
timeout := time.Duration(upOptions.waitTimeout) * time.Second
var consumer api.LogConsumer
var attach []string
if !upOptions.Detach {
consumer = formatter.NewLogConsumer(ctx, streams.Out(), streams.Err(), !upOptions.noColor, !upOptions.noPrefix, upOptions.timestamp)
var attachSet utils.Set[string]
if len(upOptions.attach) != 0 {
// services are passed explicitly with --attach, verify they're valid and then use them as-is
attachSet = utils.NewSet(upOptions.attach...)
unexpectedSvcs := attachSet.Diff(utils.NewSet(project.ServiceNames()...))
if len(unexpectedSvcs) != 0 {
return fmt.Errorf("cannot attach to services not included in up: %s", strings.Join(unexpectedSvcs.Elements(), ", "))
}
} else {
// mark services being launched (and potentially their deps) for attach
// if they didn't opt-out via Compose YAML
attachSet = utils.NewSet[string]()
var dependencyOpt types.DependencyOption = types.IgnoreDependencies
if upOptions.attachDependencies {
dependencyOpt = types.IncludeDependencies
}
if err := project.WithServices(services, func(s types.ServiceConfig) error {
if s.Attach == nil || *s.Attach {
attachSet.Add(s.Name)
}
return nil
}, dependencyOpt); err != nil {
return err
}
}
// filter out any services that have been explicitly marked for ignore with `--no-attach`
attachSet.RemoveAll(upOptions.noAttach...)
attach = attachSet.Elements()
}
timeout := time.Duration(upOptions.waitTimeout) * time.Second
return backend.Up(ctx, project, api.UpOptions{
Create: create,
Start: api.StartOptions{
Project: project,
Attach: consumer,
AttachTo: attachTo.Elements(),
AttachTo: attach,
ExitCodeFrom: upOptions.exitCodeFrom,
CascadeStop: upOptions.cascadeStop,
Wait: upOptions.wait,

8
docs/reference/compose_up.md
View File

@ -9,14 +9,14 @@ Create and start containers
|:-----------------------------|:--------------|:----------|:---------------------------------------------------------------------------------------------------------|
| `--abort-on-container-exit` | | | Stops all containers if any container was stopped. Incompatible with -d |
| `--always-recreate-deps` | | | Recreate dependent containers. Incompatible with --no-recreate. |
| `--attach` | `stringArray` | | Attach to service output. |
| `--attach-dependencies` | | | Attach to dependent containers. |
| `--attach` | `stringArray` | | Restrict attaching to the specified services. Incompatible with --attach-dependencies. |
| `--attach-dependencies` | | | Automatically attach to log output of dependent services. |
| `--build` | | | Build images before starting containers. |
| `-d`, `--detach` | | | Detached mode: Run containers in the background |
| `--dry-run` | | | Execute command in dry run mode |
| `--exit-code-from` | `string` | | Return the exit code of the selected service container. Implies --abort-on-container-exit |
| `--force-recreate` | | | Recreate containers even if their configuration and image haven't changed. |
| `--no-attach` | `stringArray` | | Don't attach to specified service. |
| `--no-attach` | `stringArray` | | Do not attach (stream logs) to the specified services. |
| `--no-build` | | | Don't build an image, even if it's missing. |
| `--no-color` | | | Produce monochrome output. |
| `--no-deps` | | | Don't start linked services. |
@ -31,7 +31,7 @@ Create and start containers
| `-t`, `--timeout` | `int` | `0` | Use this timeout in seconds for container shutdown when attached or when containers are already running. |
| `--timestamps` | | | Show timestamps. |
| `--wait` | | | Wait for services to be running\|healthy. Implies detached mode. |
| `--wait-timeout` | `int` | `0` | timeout waiting for application to be running\|healthy. |
| `--wait-timeout` | `int` | `0` | Maximum duration to wait for the project to be running\|healthy. |
<!---MARKER_GEN_END-->

9
docs/reference/docker_compose_up.yaml
View File

@ -48,7 +48,8 @@ options:
- option: attach
value_type: stringArray
default_value: '[]'
description: Attach to service output.
description: |
Restrict attaching to the specified services. Incompatible with --attach-dependencies.
deprecated: false
hidden: false
experimental: false
@ -58,7 +59,7 @@ options:
- option: attach-dependencies
value_type: bool
default_value: "false"
description: Attach to dependent containers.
description: Automatically attach to log output of dependent services.
deprecated: false
hidden: false
experimental: false
@ -110,7 +111,7 @@ options:
- option: no-attach
value_type: stringArray
default_value: '[]'
description: Don't attach to specified service.
description: Do not attach (stream logs) to the specified services.
deprecated: false
hidden: false
experimental: false
@ -266,7 +267,7 @@ options:
- option: wait-timeout
value_type: int
default_value: "0"
description: timeout waiting for application to be running|healthy.
description: Maximum duration to wait for the project to be running|healthy.
deprecated: false
hidden: false
experimental: false

55
pkg/compose/start.go
View File

@ -56,17 +56,48 @@ func (s *composeService) start(ctx context.Context, projectName string, options
}
}
eg, ctx := errgroup.WithContext(ctx)
// use an independent context tied to the errgroup for background attach operations
// the primary context is still used for other operations
// this means that once any attach operation fails, all other attaches are cancelled,
// but an attach failing won't interfere with the rest of the start
eg, attachCtx := errgroup.WithContext(ctx)
if listener != nil {
attached, err := s.attach(ctx, project, listener, options.AttachTo)
_, err := s.attach(attachCtx, project, listener, options.AttachTo)
if err != nil {
return err
}
eg.Go(func() error {
return s.watchContainers(context.Background(), project.Name, options.AttachTo, options.Services, listener, attached,
// it's possible to have a required service whose log output is not desired
// (i.e. it's not in the attach set), so watch everything and then filter
// calls to attach; this ensures that `watchContainers` blocks until all
// required containers have exited, even if their output is not being shown
attachTo := utils.NewSet[string](options.AttachTo...)
required := utils.NewSet[string](options.Services...)
toWatch := attachTo.Union(required).Elements()
containers, err := s.getContainers(ctx, projectName, oneOffExclude, true, toWatch...)
if err != nil {
return err
}
// N.B. this uses the parent context (instead of attachCtx) so that the watch itself can
// continue even if one of the log streams fails
return s.watchContainers(ctx, project.Name, toWatch, required.Elements(), listener, containers,
func(container moby.Container, _ time.Time) error {
return s.attachContainer(ctx, container, listener)
svc := container.Labels[api.ServiceLabel]
if attachTo.Has(svc) {
return s.attachContainer(attachCtx, container, listener)
}
// HACK: simulate an "attach" event
listener(api.ContainerEvent{
Type: api.ContainerEventAttach,
Container: getContainerNameWithoutProject(container),
ID: container.ID,
Service: svc,
})
return nil
}, func(container moby.Container, _ time.Time) error {
listener(api.ContainerEvent{
Type: api.ContainerEventAttach,
@ -156,6 +187,13 @@ func (s *composeService) watchContainers(ctx context.Context, //nolint:gocyclo
required = services
}
unexpected := utils.NewSet[string](required...).Diff(utils.NewSet[string](services...))
if len(unexpected) != 0 {
return fmt.Errorf(`required service(s) "%s" not present in watched service(s) "%s"`,
strings.Join(unexpected.Elements(), ", "),
strings.Join(services, ", "))
}
// predicate to tell if a container we receive event for should be considered or ignored
ofInterest := func(c moby.Container) bool {
if len(services) > 0 {
@ -190,6 +228,12 @@ func (s *composeService) watchContainers(ctx context.Context, //nolint:gocyclo
err := s.Events(ctx, projectName, api.EventsOptions{
Services: services,
Consumer: func(event api.Event) error {
defer func() {
// after consuming each event, check to see if we're done
if len(expected) == 0 {
stop()
}
}()
inspected, err := s.apiClient().ContainerInspect(ctx, event.Container)
if err != nil {
if errdefs.IsNotFound(err) {
@ -291,9 +335,6 @@ func (s *composeService) watchContainers(ctx context.Context, //nolint:gocyclo
}
}
}
if len(expected) == 0 {
stop()
}
return nil
},
})

38
pkg/utils/set.go
View File

@ -16,6 +16,23 @@ package utils
type Set[T comparable] map[T]struct{}
func NewSet[T comparable](v ...T) Set[T] {
if len(v) == 0 {
return make(Set[T])
}
out := make(Set[T], len(v))
for i := range v {
out.Add(v[i])
}
return out
}
func (s Set[T]) Has(v T) bool {
_, ok := s[v]
return ok
}
func (s Set[T]) Add(v T) {
s[v] = struct{}{}
}
@ -53,3 +70,24 @@ func (s Set[T]) RemoveAll(elements ...T) {
s.Remove(e)
}
}
func (s Set[T]) Diff(other Set[T]) Set[T] {
out := make(Set[T])
for k := range s {
if _, ok := other[k]; !ok {
out[k] = struct{}{}
}
}
return out
}
func (s Set[T]) Union(other Set[T]) Set[T] {
out := make(Set[T])
for k := range s {
out[k] = struct{}{}
}
for k := range other {
out[k] = struct{}{}
}
return out
}

41
pkg/utils/set_test.go Normal file
View File

@ -0,0 +1,41 @@
/*
Copyright 2022 Docker Compose CLI authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package utils
import (
"testing"
"github.com/stretchr/testify/require"
)
func TestSet_Has(t *testing.T) {
x := NewSet[string]("value")
require.True(t, x.Has("value"))
require.False(t, x.Has("VALUE"))
}
func TestSet_Diff(t *testing.T) {
a := NewSet[int](1, 2)
b := NewSet[int](2, 3)
require.ElementsMatch(t, []int{1}, a.Diff(b).Elements())
require.ElementsMatch(t, []int{3}, b.Diff(a).Elements())
}
func TestSet_Union(t *testing.T) {
a := NewSet[int](1, 2)
b := NewSet[int](2, 3)
require.ElementsMatch(t, []int{1, 2, 3}, a.Union(b).Elements())
require.ElementsMatch(t, []int{1, 2, 3}, b.Union(a).Elements())
}