diff --git a/integration/delete_test.go b/integration/delete_test.go index 19224079b56..7e07703e926 100644 --- a/integration/delete_test.go +++ b/integration/delete_test.go @@ -17,10 +17,16 @@ limitations under the License. package integration import ( + "context" "testing" "time" + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/filters" + "github.com/GoogleContainerTools/skaffold/v2/integration/skaffold" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/docker" + "github.com/GoogleContainerTools/skaffold/v2/testutil" ) func TestDelete(t *testing.T) { @@ -75,6 +81,57 @@ func TestDelete(t *testing.T) { } } +func TestDeleteDockerDeployer(t *testing.T) { + tests := []struct { + description string + dir string + args []string + deployedContainers []string + }{ + { + description: "run with one container", + dir: "testdata/docker-deploy", + args: []string{"-p", "one-container"}, + deployedContainers: []string{"docker-bert-img-1"}, + }, + { + description: "run with more than one container", + dir: "testdata/docker-deploy", + args: []string{"-p", "more-than-one-container"}, + deployedContainers: []string{"docker-bert-img-2", "docker-ernie-img-2"}, + }, + } + + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + MarkIntegrationTest(t.T, CanRunWithoutGcp) + ctx := context.Background() + skaffold.Run(test.args...).InDir(test.dir).RunOrFail(t.T) + skaffold.Delete(test.args...).InDir(test.dir).RunOrFail(t.T) + + client := SetupDockerClient(t.T) + cs := getContainers(ctx, t, test.deployedContainers, client) + t.CheckDeepEqual(0, len(cs)) + }) + } +} + +func getContainers(ctx context.Context, t *testutil.T, deployedContainers []string, client docker.LocalDaemon) []types.Container { + t.Helper() + + containersFilters := []filters.KeyValuePair{} + for _, c := range deployedContainers { + containersFilters = append(containersFilters, filters.Arg("name", c)) + } + + cl, err := client.ContainerList(ctx, types.ContainerListOptions{ + Filters: filters.NewArgs(containersFilters...), + }) + t.CheckNoError(err) + + return cl +} + func TestDeleteNonExistedHelmResource(t *testing.T) { var tests = []struct { description string diff --git a/integration/testdata/docker-deploy/skaffold.yaml b/integration/testdata/docker-deploy/skaffold.yaml index bf964f7e91b..4c61073c750 100644 --- a/integration/testdata/docker-deploy/skaffold.yaml +++ b/integration/testdata/docker-deploy/skaffold.yaml @@ -11,3 +11,28 @@ build: deploy: docker: images: [bert, ernie] + +profiles: + - name: one-container + build: + local: + push: false + artifacts: + - image: docker-bert-img-1 + context: bert + deploy: + docker: + images: [docker-bert-img-1] + + - name: more-than-one-container + build: + local: + push: false + artifacts: + - image: docker-bert-img-2 + context: bert + - image: docker-ernie-img-2 + context: ernie + deploy: + docker: + images: [docker-bert-img-2, docker-ernie-img-2] \ No newline at end of file diff --git a/pkg/skaffold/actions/docker/exec_env.go b/pkg/skaffold/actions/docker/exec_env.go index a56215fd61a..e75071283f5 100644 --- a/pkg/skaffold/actions/docker/exec_env.go +++ b/pkg/skaffold/actions/docker/exec_env.go @@ -108,7 +108,7 @@ func newExecEnv(ctx context.Context, cfg dockerutil.Config, labeller *label.Defa func (e ExecEnv) PrepareActions(ctx context.Context, out io.Writer, allbuilds, _ []graph.Artifact, acsNames []string) ([]actions.Action, error) { if e.shouldCreateNetwork { - if err := e.client.NetworkCreate(ctx, e.network); err != nil { + if err := e.client.NetworkCreate(ctx, e.network, nil); err != nil { return nil, fmt.Errorf("creating skaffold network %s: %w", e.network, err) } } diff --git a/pkg/skaffold/actions/docker/exec_env_test.go b/pkg/skaffold/actions/docker/exec_env_test.go index 623805bc7d8..516edd3e7c3 100644 --- a/pkg/skaffold/actions/docker/exec_env_test.go +++ b/pkg/skaffold/actions/docker/exec_env_test.go @@ -38,7 +38,7 @@ type fakeDockerDaemon struct { ImgsInDaemon map[string]string } -func (fd *fakeDockerDaemon) NetworkCreate(ctx context.Context, name string) error { +func (fd *fakeDockerDaemon) NetworkCreate(ctx context.Context, name string, labels map[string]string) error { return nil } diff --git a/pkg/skaffold/deploy/docker/deploy.go b/pkg/skaffold/deploy/docker/deploy.go index 59d1141db09..16ff20e29f2 100644 --- a/pkg/skaffold/deploy/docker/deploy.go +++ b/pkg/skaffold/deploy/docker/deploy.go @@ -20,10 +20,15 @@ import ( "context" "fmt" "io" + "regexp" "sync" + "time" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/mount" + "github.com/docker/docker/api/types/volume" "github.com/docker/go-connections/nat" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/pkg/errors" @@ -46,6 +51,7 @@ import ( "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/status" pkgsync "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/sync" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util/stringslice" ) @@ -62,10 +68,12 @@ type Deployer struct { portManager *dockerport.PortManager // functions as Accessor client dockerutil.LocalDaemon network string + networkDeployed bool globalConfig string insecureRegistries map[string]bool resources []*latest.PortForwardResource once sync.Once + labeller *label.DefaultLabeller } func NewDeployer(ctx context.Context, cfg dockerutil.Config, labeller *label.DefaultLabeller, d *latest.DockerDeploy, resources []*latest.PortForwardResource, configName string) (*Deployer, error) { @@ -94,6 +102,7 @@ func NewDeployer(ctx context.Context, cfg dockerutil.Config, labeller *label.Def cfg: d, client: client, network: fmt.Sprintf("skaffold-network-%s", labeller.GetRunID()), + networkDeployed: false, resources: resources, globalConfig: cfg.GlobalConfig(), insecureRegistries: cfg.GetInsecureRegistries(), @@ -103,6 +112,7 @@ func NewDeployer(ctx context.Context, cfg dockerutil.Config, labeller *label.Def logger: l, monitor: &status.NoopMonitor{}, syncer: pkgsync.NewContainerSyncer(), + labeller: labeller, }, nil } @@ -121,7 +131,8 @@ func (d *Deployer) TrackContainerFromBuild(artifact graph.Artifact, container tr func (d *Deployer) Deploy(ctx context.Context, out io.Writer, builds []graph.Artifact, _ manifest.ManifestListByConfig) error { var err error d.once.Do(func() { - err = d.client.NetworkCreate(ctx, d.network) + err = d.client.NetworkCreate(ctx, d.network, d.labeller.Labels()) + d.networkDeployed = true }) if err != nil { return fmt.Errorf("creating skaffold network %s: %w", d.network, err) @@ -225,6 +236,8 @@ func (d *Deployer) setupDebugging(ctx context.Context, out io.Writer, artifact g for the active daemon if this is ever extended to support multiple active Docker daemons. */ for _, c := range initContainers { + labels := d.labeller.DebugLabels() + if d.debugger.HasMount(c.Image) { // skip duplication of init containers continue @@ -233,9 +246,22 @@ func (d *Deployer) setupDebugging(ctx context.Context, out io.Writer, artifact g if err := d.client.Pull(ctx, out, c.Image, v1.Platform{}); err != nil { return nil, errors.Wrap(err, "pulling init container image") } + + // create the volume used by the init container + v, err := d.client.VolumeCreate(ctx, volume.VolumeCreateBody{ + Labels: labels, + }) + if err != nil { + return nil, err + } + + m := d.createMount(v, labels) + // create the init container + c.Labels = labels _, _, id, err := d.client.Run(ctx, out, dockerutil.ContainerCreateOpts{ ContainerConfig: c, + Mounts: []mount.Mount{m}, }) if err != nil { return nil, errors.Wrap(err, "creating container in local docker") @@ -248,7 +274,7 @@ func (d *Deployer) setupDebugging(ctx context.Context, out io.Writer, artifact g olog.Entry(ctx).Warnf("unable to retrieve mount from debug init container: debugging may not work correctly!") } // we know there is only one mount point, since we generated the init container config ourselves - d.debugger.AddSupportMount(c.Image, r.Mounts[0].Name) + d.debugger.AddSupportMount(c.Image, m) } bindings := make(nat.PortMap) @@ -265,12 +291,26 @@ func (d *Deployer) setupDebugging(ctx context.Context, out io.Writer, artifact g return bindings, nil } +func (d *Deployer) createMount(v types.Volume, labels map[string]string) mount.Mount { + return mount.Mount{ + Type: mount.TypeVolume, + Source: v.Name, + Target: "/dbg", + VolumeOptions: &mount.VolumeOptions{ + Labels: labels, + }, + } +} + func (d *Deployer) containerConfigFromImage(ctx context.Context, taggedImage string) (*container.Config, error) { config, _, err := d.client.ImageInspectWithRaw(ctx, taggedImage) if err != nil { return nil, err } + + config.Config.Labels = d.labeller.Labels() config.Config.Image = taggedImage // the client replaces this with an image ID. put back the originally provided tagged image + return config.Config, err } @@ -317,8 +357,155 @@ func (d *Deployer) Cleanup(ctx context.Context, out io.Writer, dryRun bool, _ ma } } - err := d.client.NetworkRemove(ctx, d.network) - return errors.Wrap(err, "cleaning up skaffold created network") + if err := d.client.NetworkRemove(ctx, d.network); d.networkDeployed && err != nil { + return errors.Wrap(err, "cleaning up skaffold created network") + } + + return d.cleanPreviousDeployments(ctx) +} + +func (d *Deployer) cleanPreviousDeployments(ctx context.Context) error { + runIDLabelFilter := filters.Arg("label", label.RunIDLabel) + + ctd, err := d.containersToDelete(ctx, runIDLabelFilter) + if err != nil { + return err + } + + ntd, err := d.networksToDelete(ctx, runIDLabelFilter, ctd) + if err != nil { + return err + } + + vtd := d.volumesToDelete(ctd) + + dctd, err := d.debugContainersToDelete(ctx, runIDLabelFilter, vtd) + if err != nil { + return err + } + + for _, c := range append(ctd, dctd...) { + d.client.Stop(ctx, c.ID, util.Ptr(time.Second*0)) + if err := d.client.Remove(ctx, c.ID); err != nil { + return err + } + } + + for _, n := range ntd { + if err := d.client.NetworkRemove(ctx, n.Name); err != nil { + return err + } + } + + for v := range vtd { + if err := d.client.VolumeRemove(ctx, v); err != nil { + return err + } + } + + return nil +} + +func (d *Deployer) containersToDelete(ctx context.Context, runIDLabelFilter filters.KeyValuePair) ([]types.Container, error) { + csToDelete := []types.Container{} + for _, img := range d.cfg.Images { + cs, err := d.getContainersCreated(ctx, img, runIDLabelFilter) + if err != nil { + return nil, err + } + csToDelete = append(csToDelete, cs...) + } + + return csToDelete, nil +} + +func (d *Deployer) getContainersCreated(ctx context.Context, img string, runIDLabelFilter filters.KeyValuePair) ([]types.Container, error) { + nameFilter := filters.Arg("name", img) + cl, err := d.client.ContainerList(ctx, types.ContainerListOptions{ + All: true, + Filters: filters.NewArgs(runIDLabelFilter, nameFilter), + }) + if err != nil { + return nil, err + } + // Docker will return all the containers whose name includes the img name, so here we make sure we filter + // only the containers starting with the img name. Regex support in docker filters is undocumented. + return d.filterByName(cl, img) +} + +func (d *Deployer) filterByName(cl []types.Container, cName string) ([]types.Container, error) { + nameMatchR, err := regexp.Compile(fmt.Sprintf("^/?%v(-\\d+)?$", cName)) + if err != nil { + return nil, errors.Wrap(err, "compiling name match regex") + } + + containers := []types.Container{} + for _, c := range cl { + for _, n := range c.Names { + if nameMatchR.MatchString(n) { + containers = append(containers, c) + break + } + } + } + + return containers, nil +} + +func (d *Deployer) networksToDelete(ctx context.Context, runIDLabelFilter filters.KeyValuePair, containers []types.Container) ([]types.NetworkResource, error) { + ns, err := d.client.NetworkList(ctx, types.NetworkListOptions{ + Filters: filters.NewArgs(runIDLabelFilter), + }) + if err != nil { + return nil, err + } + + containersNetworks := make(map[string]bool) + for _, c := range containers { + for n := range c.NetworkSettings.Networks { + containersNetworks[n] = true + } + } + + nsToDelete := []types.NetworkResource{} + for _, n := range ns { + if _, found := containersNetworks[n.Name]; found { + nsToDelete = append(nsToDelete, n) + } + } + + return nsToDelete, nil +} + +func (d *Deployer) volumesToDelete(containers []types.Container) map[string]bool { + vtd := make(map[string]bool) + for _, c := range containers { + for _, m := range c.Mounts { + _, found := vtd[m.Name] + if m.Destination == "/dbg" && !found { + vtd[m.Name] = true + } + } + } + + return vtd +} + +func (d *Deployer) debugContainersToDelete(ctx context.Context, runIDLabelFilter filters.KeyValuePair, volumes map[string]bool) ([]types.Container, error) { + containersFilters := []filters.KeyValuePair{runIDLabelFilter, filters.Arg("label", label.DebugContainerLabel)} + for v := range volumes { + containersFilters = append(containersFilters, filters.Arg("volume", v)) + } + + cl, err := d.client.ContainerList(ctx, types.ContainerListOptions{ + All: true, + Filters: filters.NewArgs(containersFilters...), + }) + if err != nil { + return nil, err + } + + return cl, nil } func (d *Deployer) GetAccessor() access.Accessor { diff --git a/pkg/skaffold/deploy/label/labeller.go b/pkg/skaffold/deploy/label/labeller.go index 0b5cdfe1e13..223be2d851d 100644 --- a/pkg/skaffold/deploy/label/labeller.go +++ b/pkg/skaffold/deploy/label/labeller.go @@ -22,7 +22,8 @@ import ( ) const ( - RunIDLabel = "skaffold.dev/run-id" + RunIDLabel = "skaffold.dev/run-id" + DebugContainerLabel = "skaffold.dev/debug" ) type Config interface { @@ -63,6 +64,13 @@ func (d *DefaultLabeller) Labels() map[string]string { return labels } +func (d *DefaultLabeller) DebugLabels() map[string]string { + labels := d.Labels() + labels[DebugContainerLabel] = "true" + + return labels +} + func (d *DefaultLabeller) RunIDSelector() string { return fmt.Sprintf("%s=%s", RunIDLabel, d.Labels()[RunIDLabel]) } diff --git a/pkg/skaffold/docker/debugger/debug.go b/pkg/skaffold/docker/debugger/debug.go index 2bd094bd481..ae08f428933 100644 --- a/pkg/skaffold/docker/debugger/debug.go +++ b/pkg/skaffold/docker/debugger/debug.go @@ -64,14 +64,10 @@ func (d *DebugManager) ConfigurationForImage(image string) types.ContainerDebugC return d.configurations[image] } -func (d *DebugManager) AddSupportMount(image, mountID string) { +func (d *DebugManager) AddSupportMount(image string, m mount.Mount) { d.mountLock.Lock() defer d.mountLock.Unlock() - d.supportMounts[image] = mount.Mount{ - Type: mount.TypeVolume, - Source: mountID, - Target: "/dbg", - } + d.supportMounts[image] = m } func (d *DebugManager) HasMount(image string) bool { diff --git a/pkg/skaffold/docker/image.go b/pkg/skaffold/docker/image.go index bf75fdddd13..7d4933990c3 100644 --- a/pkg/skaffold/docker/image.go +++ b/pkg/skaffold/docker/image.go @@ -73,6 +73,7 @@ type ContainerCreateOpts struct { Mounts []mount.Mount ContainerConfig *container.Config VerifyTestName string + Labels map[string]string } // LocalDaemon talks to a local Docker API. @@ -98,8 +99,9 @@ type LocalDaemon interface { ImageRemove(ctx context.Context, image string, opts types.ImageRemoveOptions) ([]types.ImageDeleteResponseItem, error) ImageExists(ctx context.Context, ref string) bool ImageList(ctx context.Context, ref string) ([]types.ImageSummary, error) - NetworkCreate(ctx context.Context, name string) error + NetworkCreate(ctx context.Context, name string, labels map[string]string) error NetworkRemove(ctx context.Context, name string) error + NetworkList(ctx context.Context, opts types.NetworkListOptions) ([]types.NetworkResource, error) Prune(ctx context.Context, images []string, pruneChildren bool) ([]string, error) DiskUsage(ctx context.Context) (uint64, error) RawClient() client.CommonAPIClient @@ -242,7 +244,7 @@ func (l *localDaemon) Run(ctx context.Context, out io.Writer, opts ContainerCrea return nil, nil, c.ID, nil } -func (l *localDaemon) NetworkCreate(ctx context.Context, name string) error { +func (l *localDaemon) NetworkCreate(ctx context.Context, name string, labels map[string]string) error { nr, err := l.apiClient.NetworkList(ctx, types.NetworkListOptions{}) if err != nil { return err @@ -253,7 +255,7 @@ func (l *localDaemon) NetworkCreate(ctx context.Context, name string) error { } } - r, err := l.apiClient.NetworkCreate(ctx, name, types.NetworkCreate{}) + r, err := l.apiClient.NetworkCreate(ctx, name, types.NetworkCreate{Labels: labels}) if err != nil { return err } @@ -267,6 +269,10 @@ func (l *localDaemon) NetworkRemove(ctx context.Context, name string) error { return l.apiClient.NetworkRemove(ctx, name) } +func (l *localDaemon) NetworkList(ctx context.Context, opts types.NetworkListOptions) ([]types.NetworkResource, error) { + return l.apiClient.NetworkList(ctx, opts) +} + // ServerVersion retrieves the version information from the server. func (l *localDaemon) ServerVersion(ctx context.Context) (types.Version, error) { return l.apiClient.ServerVersion(ctx) diff --git a/pkg/skaffold/verify/docker/verify.go b/pkg/skaffold/verify/docker/verify.go index 0f0b904ca59..92248de49e6 100644 --- a/pkg/skaffold/verify/docker/verify.go +++ b/pkg/skaffold/verify/docker/verify.go @@ -119,7 +119,7 @@ func (v *Verifier) Verify(ctx context.Context, out io.Writer, allbuilds []graph. if !v.networkFlagPassed { v.once.Do(func() { - err = v.client.NetworkCreate(ctx, v.network) + err = v.client.NetworkCreate(ctx, v.network, nil) }) if err != nil { return fmt.Errorf("creating skaffold network %s: %w", v.network, err)