From 39cf7e12b3bb1ea13cf3a7d49252f1b45fbb0371 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Thu, 10 May 2018 05:45:11 -0400 Subject: [PATCH 01/10] refactor integration tests under integration/service/create_test.go to use swarm.CreateService Signed-off-by: Arash Deshmeh Upstream-commit: e9a687146a1e2721e19d858927283c35f8c2da97 Component: engine --- .../engine/integration/service/create_test.go | 165 ++++++------------ 1 file changed, 52 insertions(+), 113 deletions(-) diff --git a/components/engine/integration/service/create_test.go b/components/engine/integration/service/create_test.go index 36de77316b..68af6e825a 100644 --- a/components/engine/integration/service/create_test.go +++ b/components/engine/integration/service/create_test.go @@ -34,15 +34,14 @@ func TestCreateServiceMultipleTimes(t *testing.T) { overlayID := netResp.ID var instances uint64 = 4 - serviceSpec := swarmServiceSpec("TestService", instances) - serviceSpec.TaskTemplate.Networks = append(serviceSpec.TaskTemplate.Networks, swarmtypes.NetworkAttachmentConfig{Target: overlayName}) - serviceResp, err := client.ServiceCreate(context.Background(), serviceSpec, types.ServiceCreateOptions{ - QueryRegistry: false, - }) - assert.NilError(t, err) + serviceSpec := []swarm.ServiceSpecOpt{ + swarm.ServiceWithReplicas(instances), + swarm.ServiceWithName("TestService"), + swarm.ServiceWithNetwork(overlayName), + } - serviceID := serviceResp.ID + serviceID := swarm.CreateService(t, d, serviceSpec...) poll.WaitOn(t, serviceRunningTasksCount(client, serviceID, instances), swarm.ServicePoll) _, _, err = client.ServiceInspectWithRaw(context.Background(), serviceID, types.ServiceInspectOptions{}) @@ -54,12 +53,7 @@ func TestCreateServiceMultipleTimes(t *testing.T) { poll.WaitOn(t, serviceIsRemoved(client, serviceID), swarm.ServicePoll) poll.WaitOn(t, noTasks(client), swarm.ServicePoll) - serviceResp, err = client.ServiceCreate(context.Background(), serviceSpec, types.ServiceCreateOptions{ - QueryRegistry: false, - }) - assert.NilError(t, err) - - serviceID2 := serviceResp.ID + serviceID2 := swarm.CreateService(t, d, serviceSpec...) poll.WaitOn(t, serviceRunningTasksCount(client, serviceID2, instances), swarm.ServicePoll) err = client.ServiceRemove(context.Background(), serviceID2) @@ -100,25 +94,25 @@ func TestCreateWithDuplicateNetworkNames(t *testing.T) { // Create Service with the same name var instances uint64 = 1 - serviceSpec := swarmServiceSpec("top", instances) - serviceSpec.TaskTemplate.Networks = append(serviceSpec.TaskTemplate.Networks, swarmtypes.NetworkAttachmentConfig{Target: name}) + serviceID := swarm.CreateService(t, d, + swarm.ServiceWithReplicas(instances), + swarm.ServiceWithName("top"), + swarm.ServiceWithNetwork(name), + ) - service, err := client.ServiceCreate(context.Background(), serviceSpec, types.ServiceCreateOptions{}) - assert.NilError(t, err) + poll.WaitOn(t, serviceRunningTasksCount(client, serviceID, instances), swarm.ServicePoll) - poll.WaitOn(t, serviceRunningTasksCount(client, service.ID, instances), swarm.ServicePoll) - - resp, _, err := client.ServiceInspectWithRaw(context.Background(), service.ID, types.ServiceInspectOptions{}) + resp, _, err := client.ServiceInspectWithRaw(context.Background(), serviceID, types.ServiceInspectOptions{}) assert.NilError(t, err) assert.Check(t, is.Equal(n3.ID, resp.Spec.TaskTemplate.Networks[0].Target)) // Remove Service - err = client.ServiceRemove(context.Background(), service.ID) + err = client.ServiceRemove(context.Background(), serviceID) assert.NilError(t, err) // Make sure task has been destroyed. - poll.WaitOn(t, serviceIsRemoved(client, service.ID), swarm.ServicePoll) + poll.WaitOn(t, serviceIsRemoved(client, serviceID), swarm.ServicePoll) // Remove networks err = client.NetworkRemove(context.Background(), n3.ID) @@ -153,44 +147,26 @@ func TestCreateServiceSecretFileMode(t *testing.T) { assert.NilError(t, err) var instances uint64 = 1 - serviceSpec := swarmtypes.ServiceSpec{ - Annotations: swarmtypes.Annotations{ - Name: "TestService", - }, - TaskTemplate: swarmtypes.TaskSpec{ - ContainerSpec: &swarmtypes.ContainerSpec{ - Image: "busybox:latest", - Command: []string{"/bin/sh", "-c", "ls -l /etc/secret || /bin/top"}, - Secrets: []*swarmtypes.SecretReference{ - { - File: &swarmtypes.SecretReferenceFileTarget{ - Name: "/etc/secret", - UID: "0", - GID: "0", - Mode: 0777, - }, - SecretID: secretResp.ID, - SecretName: "TestSecret", - }, - }, + serviceID := swarm.CreateService(t, d, + swarm.ServiceWithReplicas(instances), + swarm.ServiceWithName("TestService"), + swarm.ServiceWithCommand([]string{"/bin/sh", "-c", "ls -l /etc/secret || /bin/top"}), + swarm.ServiceWithSecret(&swarmtypes.SecretReference{ + File: &swarmtypes.SecretReferenceFileTarget{ + Name: "/etc/secret", + UID: "0", + GID: "0", + Mode: 0777, }, - }, - Mode: swarmtypes.ServiceMode{ - Replicated: &swarmtypes.ReplicatedService{ - Replicas: &instances, - }, - }, - } + SecretID: secretResp.ID, + SecretName: "TestSecret", + }), + ) - serviceResp, err := client.ServiceCreate(ctx, serviceSpec, types.ServiceCreateOptions{ - QueryRegistry: false, - }) - assert.NilError(t, err) - - poll.WaitOn(t, serviceRunningTasksCount(client, serviceResp.ID, instances), swarm.ServicePoll) + poll.WaitOn(t, serviceRunningTasksCount(client, serviceID, instances), swarm.ServicePoll) filter := filters.NewArgs() - filter.Add("service", serviceResp.ID) + filter.Add("service", serviceID) tasks, err := client.TaskList(ctx, types.TaskListOptions{ Filters: filter, }) @@ -207,10 +183,10 @@ func TestCreateServiceSecretFileMode(t *testing.T) { assert.NilError(t, err) assert.Check(t, is.Contains(string(content), "-rwxrwxrwx")) - err = client.ServiceRemove(ctx, serviceResp.ID) + err = client.ServiceRemove(ctx, serviceID) assert.NilError(t, err) - poll.WaitOn(t, serviceIsRemoved(client, serviceResp.ID), swarm.ServicePoll) + poll.WaitOn(t, serviceIsRemoved(client, serviceID), swarm.ServicePoll) poll.WaitOn(t, noTasks(client), swarm.ServicePoll) err = client.SecretRemove(ctx, "TestSecret") @@ -234,44 +210,26 @@ func TestCreateServiceConfigFileMode(t *testing.T) { assert.NilError(t, err) var instances uint64 = 1 - serviceSpec := swarmtypes.ServiceSpec{ - Annotations: swarmtypes.Annotations{ - Name: "TestService", - }, - TaskTemplate: swarmtypes.TaskSpec{ - ContainerSpec: &swarmtypes.ContainerSpec{ - Image: "busybox:latest", - Command: []string{"/bin/sh", "-c", "ls -l /etc/config || /bin/top"}, - Configs: []*swarmtypes.ConfigReference{ - { - File: &swarmtypes.ConfigReferenceFileTarget{ - Name: "/etc/config", - UID: "0", - GID: "0", - Mode: 0777, - }, - ConfigID: configResp.ID, - ConfigName: "TestConfig", - }, - }, + serviceID := swarm.CreateService(t, d, + swarm.ServiceWithName("TestService"), + swarm.ServiceWithCommand([]string{"/bin/sh", "-c", "ls -l /etc/config || /bin/top"}), + swarm.ServiceWithReplicas(instances), + swarm.ServiceWithConfig(&swarmtypes.ConfigReference{ + File: &swarmtypes.ConfigReferenceFileTarget{ + Name: "/etc/config", + UID: "0", + GID: "0", + Mode: 0777, }, - }, - Mode: swarmtypes.ServiceMode{ - Replicated: &swarmtypes.ReplicatedService{ - Replicas: &instances, - }, - }, - } + ConfigID: configResp.ID, + ConfigName: "TestConfig", + }), + ) - serviceResp, err := client.ServiceCreate(ctx, serviceSpec, types.ServiceCreateOptions{ - QueryRegistry: false, - }) - assert.NilError(t, err) - - poll.WaitOn(t, serviceRunningTasksCount(client, serviceResp.ID, instances)) + poll.WaitOn(t, serviceRunningTasksCount(client, serviceID, instances)) filter := filters.NewArgs() - filter.Add("service", serviceResp.ID) + filter.Add("service", serviceID) tasks, err := client.TaskList(ctx, types.TaskListOptions{ Filters: filter, }) @@ -288,35 +246,16 @@ func TestCreateServiceConfigFileMode(t *testing.T) { assert.NilError(t, err) assert.Check(t, is.Contains(string(content), "-rwxrwxrwx")) - err = client.ServiceRemove(ctx, serviceResp.ID) + err = client.ServiceRemove(ctx, serviceID) assert.NilError(t, err) - poll.WaitOn(t, serviceIsRemoved(client, serviceResp.ID)) + poll.WaitOn(t, serviceIsRemoved(client, serviceID)) poll.WaitOn(t, noTasks(client)) err = client.ConfigRemove(ctx, "TestConfig") assert.NilError(t, err) } -func swarmServiceSpec(name string, replicas uint64) swarmtypes.ServiceSpec { - return swarmtypes.ServiceSpec{ - Annotations: swarmtypes.Annotations{ - Name: name, - }, - TaskTemplate: swarmtypes.TaskSpec{ - ContainerSpec: &swarmtypes.ContainerSpec{ - Image: "busybox:latest", - Command: []string{"/bin/top"}, - }, - }, - Mode: swarmtypes.ServiceMode{ - Replicated: &swarmtypes.ReplicatedService{ - Replicas: &replicas, - }, - }, - } -} - func serviceRunningTasksCount(client client.ServiceAPIClient, serviceID string, instances uint64) func(log poll.LogT) poll.Result { return func(log poll.LogT) poll.Result { filter := filters.NewArgs() From c6a8d2ea17abf1eac31054d0e7acb2b0fb80bc4c Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 10 May 2018 16:44:09 -0400 Subject: [PATCH 02/10] Move network operations out of container package These network operations really don't have anything to do with the container but rather are setting up the networking. Ideally these wouldn't get shoved into the daemon package, but doing something else (e.g. extract a network service into a new package) but there's a lot more work to do in that regard. In reality, this probably simplifies some of that work as it moves all the network operations to the same place. Signed-off-by: Brian Goff Upstream-commit: cc8f358c23d307d19b06cd5e7d039d8fad01a947 Component: engine --- components/engine/container/container.go | 377 +----------------- .../engine/daemon/container_operations.go | 52 ++- .../daemon/container_operations_windows.go | 2 +- components/engine/daemon/network.go | 321 ++++++++++++++- components/engine/daemon/oci_windows.go | 2 +- 5 files changed, 367 insertions(+), 387 deletions(-) diff --git a/components/engine/container/container.go b/components/engine/container/container.go index 5a6a9255be..5f31d8df12 100644 --- a/components/engine/container/container.go +++ b/components/engine/container/container.go @@ -6,11 +6,9 @@ import ( "encoding/json" "fmt" "io" - "net" "os" "path/filepath" "runtime" - "strconv" "strings" "sync" "syscall" @@ -19,7 +17,6 @@ import ( "github.com/containerd/containerd/cio" containertypes "github.com/docker/docker/api/types/container" mounttypes "github.com/docker/docker/api/types/mount" - networktypes "github.com/docker/docker/api/types/network" swarmtypes "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/container/stream" "github.com/docker/docker/daemon/exec" @@ -28,7 +25,6 @@ import ( "github.com/docker/docker/daemon/network" "github.com/docker/docker/image" "github.com/docker/docker/layer" - "github.com/docker/docker/opts" "github.com/docker/docker/pkg/containerfs" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/ioutils" @@ -36,15 +32,9 @@ import ( "github.com/docker/docker/pkg/symlink" "github.com/docker/docker/pkg/system" "github.com/docker/docker/restartmanager" - "github.com/docker/docker/runconfig" "github.com/docker/docker/volume" volumemounts "github.com/docker/docker/volume/mounts" - "github.com/docker/go-connections/nat" - units "github.com/docker/go-units" - "github.com/docker/libnetwork" - "github.com/docker/libnetwork/netlabel" - "github.com/docker/libnetwork/options" - "github.com/docker/libnetwork/types" + "github.com/docker/go-units" agentexec "github.com/docker/swarmkit/agent/exec" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -52,11 +42,6 @@ import ( const configFileName = "config.v2.json" -var ( - errInvalidEndpoint = errors.New("invalid endpoint while building port map info") - errInvalidNetwork = errors.New("invalid network settings while building port map info") -) - // ExitStatus provides exit reasons for a container. type ExitStatus struct { // The exit code with which the container exited. @@ -538,366 +523,6 @@ func (container *Container) InitDNSHostConfig() { } } -// GetEndpointInNetwork returns the container's endpoint to the provided network. -func (container *Container) GetEndpointInNetwork(n libnetwork.Network) (libnetwork.Endpoint, error) { - endpointName := strings.TrimPrefix(container.Name, "/") - return n.EndpointByName(endpointName) -} - -func (container *Container) buildPortMapInfo(ep libnetwork.Endpoint) error { - if ep == nil { - return errInvalidEndpoint - } - - networkSettings := container.NetworkSettings - if networkSettings == nil { - return errInvalidNetwork - } - - if len(networkSettings.Ports) == 0 { - pm, err := getEndpointPortMapInfo(ep) - if err != nil { - return err - } - networkSettings.Ports = pm - } - return nil -} - -func getEndpointPortMapInfo(ep libnetwork.Endpoint) (nat.PortMap, error) { - pm := nat.PortMap{} - driverInfo, err := ep.DriverInfo() - if err != nil { - return pm, err - } - - if driverInfo == nil { - // It is not an error for epInfo to be nil - return pm, nil - } - - if expData, ok := driverInfo[netlabel.ExposedPorts]; ok { - if exposedPorts, ok := expData.([]types.TransportPort); ok { - for _, tp := range exposedPorts { - natPort, err := nat.NewPort(tp.Proto.String(), strconv.Itoa(int(tp.Port))) - if err != nil { - return pm, fmt.Errorf("Error parsing Port value(%v):%v", tp.Port, err) - } - pm[natPort] = nil - } - } - } - - mapData, ok := driverInfo[netlabel.PortMap] - if !ok { - return pm, nil - } - - if portMapping, ok := mapData.([]types.PortBinding); ok { - for _, pp := range portMapping { - natPort, err := nat.NewPort(pp.Proto.String(), strconv.Itoa(int(pp.Port))) - if err != nil { - return pm, err - } - natBndg := nat.PortBinding{HostIP: pp.HostIP.String(), HostPort: strconv.Itoa(int(pp.HostPort))} - pm[natPort] = append(pm[natPort], natBndg) - } - } - - return pm, nil -} - -// GetSandboxPortMapInfo retrieves the current port-mapping programmed for the given sandbox -func GetSandboxPortMapInfo(sb libnetwork.Sandbox) nat.PortMap { - pm := nat.PortMap{} - if sb == nil { - return pm - } - - for _, ep := range sb.Endpoints() { - pm, _ = getEndpointPortMapInfo(ep) - if len(pm) > 0 { - break - } - } - return pm -} - -// BuildEndpointInfo sets endpoint-related fields on container.NetworkSettings based on the provided network and endpoint. -func (container *Container) BuildEndpointInfo(n libnetwork.Network, ep libnetwork.Endpoint) error { - if ep == nil { - return errInvalidEndpoint - } - - networkSettings := container.NetworkSettings - if networkSettings == nil { - return errInvalidNetwork - } - - epInfo := ep.Info() - if epInfo == nil { - // It is not an error to get an empty endpoint info - return nil - } - - if _, ok := networkSettings.Networks[n.Name()]; !ok { - networkSettings.Networks[n.Name()] = &network.EndpointSettings{ - EndpointSettings: &networktypes.EndpointSettings{}, - } - } - networkSettings.Networks[n.Name()].NetworkID = n.ID() - networkSettings.Networks[n.Name()].EndpointID = ep.ID() - - iface := epInfo.Iface() - if iface == nil { - return nil - } - - if iface.MacAddress() != nil { - networkSettings.Networks[n.Name()].MacAddress = iface.MacAddress().String() - } - - if iface.Address() != nil { - ones, _ := iface.Address().Mask.Size() - networkSettings.Networks[n.Name()].IPAddress = iface.Address().IP.String() - networkSettings.Networks[n.Name()].IPPrefixLen = ones - } - - if iface.AddressIPv6() != nil && iface.AddressIPv6().IP.To16() != nil { - onesv6, _ := iface.AddressIPv6().Mask.Size() - networkSettings.Networks[n.Name()].GlobalIPv6Address = iface.AddressIPv6().IP.String() - networkSettings.Networks[n.Name()].GlobalIPv6PrefixLen = onesv6 - } - - return nil -} - -type named interface { - Name() string -} - -// UpdateJoinInfo updates network settings when container joins network n with endpoint ep. -func (container *Container) UpdateJoinInfo(n named, ep libnetwork.Endpoint) error { - if err := container.buildPortMapInfo(ep); err != nil { - return err - } - - epInfo := ep.Info() - if epInfo == nil { - // It is not an error to get an empty endpoint info - return nil - } - if epInfo.Gateway() != nil { - container.NetworkSettings.Networks[n.Name()].Gateway = epInfo.Gateway().String() - } - if epInfo.GatewayIPv6().To16() != nil { - container.NetworkSettings.Networks[n.Name()].IPv6Gateway = epInfo.GatewayIPv6().String() - } - - return nil -} - -// UpdateSandboxNetworkSettings updates the sandbox ID and Key. -func (container *Container) UpdateSandboxNetworkSettings(sb libnetwork.Sandbox) error { - container.NetworkSettings.SandboxID = sb.ID() - container.NetworkSettings.SandboxKey = sb.Key() - return nil -} - -// BuildJoinOptions builds endpoint Join options from a given network. -func (container *Container) BuildJoinOptions(n named) ([]libnetwork.EndpointOption, error) { - var joinOptions []libnetwork.EndpointOption - if epConfig, ok := container.NetworkSettings.Networks[n.Name()]; ok { - for _, str := range epConfig.Links { - name, alias, err := opts.ParseLink(str) - if err != nil { - return nil, err - } - joinOptions = append(joinOptions, libnetwork.CreateOptionAlias(name, alias)) - } - for k, v := range epConfig.DriverOpts { - joinOptions = append(joinOptions, libnetwork.EndpointOptionGeneric(options.Generic{k: v})) - } - } - - return joinOptions, nil -} - -// BuildCreateEndpointOptions builds endpoint options from a given network. -func (container *Container) BuildCreateEndpointOptions(n libnetwork.Network, epConfig *networktypes.EndpointSettings, sb libnetwork.Sandbox, daemonDNS []string) ([]libnetwork.EndpointOption, error) { - var ( - bindings = make(nat.PortMap) - pbList []types.PortBinding - exposeList []types.TransportPort - createOptions []libnetwork.EndpointOption - ) - - defaultNetName := runconfig.DefaultDaemonNetworkMode().NetworkName() - - if (!container.EnableServiceDiscoveryOnDefaultNetwork() && n.Name() == defaultNetName) || - container.NetworkSettings.IsAnonymousEndpoint { - createOptions = append(createOptions, libnetwork.CreateOptionAnonymous()) - } - - if epConfig != nil { - ipam := epConfig.IPAMConfig - - if ipam != nil { - var ( - ipList []net.IP - ip, ip6, linkip net.IP - ) - - for _, ips := range ipam.LinkLocalIPs { - if linkip = net.ParseIP(ips); linkip == nil && ips != "" { - return nil, errors.Errorf("Invalid link-local IP address: %s", ipam.LinkLocalIPs) - } - ipList = append(ipList, linkip) - - } - - if ip = net.ParseIP(ipam.IPv4Address); ip == nil && ipam.IPv4Address != "" { - return nil, errors.Errorf("Invalid IPv4 address: %s)", ipam.IPv4Address) - } - - if ip6 = net.ParseIP(ipam.IPv6Address); ip6 == nil && ipam.IPv6Address != "" { - return nil, errors.Errorf("Invalid IPv6 address: %s)", ipam.IPv6Address) - } - - createOptions = append(createOptions, - libnetwork.CreateOptionIpam(ip, ip6, ipList, nil)) - - } - - for _, alias := range epConfig.Aliases { - createOptions = append(createOptions, libnetwork.CreateOptionMyAlias(alias)) - } - for k, v := range epConfig.DriverOpts { - createOptions = append(createOptions, libnetwork.EndpointOptionGeneric(options.Generic{k: v})) - } - } - - if container.NetworkSettings.Service != nil { - svcCfg := container.NetworkSettings.Service - - var vip string - if svcCfg.VirtualAddresses[n.ID()] != nil { - vip = svcCfg.VirtualAddresses[n.ID()].IPv4 - } - - var portConfigs []*libnetwork.PortConfig - for _, portConfig := range svcCfg.ExposedPorts { - portConfigs = append(portConfigs, &libnetwork.PortConfig{ - Name: portConfig.Name, - Protocol: libnetwork.PortConfig_Protocol(portConfig.Protocol), - TargetPort: portConfig.TargetPort, - PublishedPort: portConfig.PublishedPort, - }) - } - - createOptions = append(createOptions, libnetwork.CreateOptionService(svcCfg.Name, svcCfg.ID, net.ParseIP(vip), portConfigs, svcCfg.Aliases[n.ID()])) - } - - if !containertypes.NetworkMode(n.Name()).IsUserDefined() { - createOptions = append(createOptions, libnetwork.CreateOptionDisableResolution()) - } - - // configs that are applicable only for the endpoint in the network - // to which container was connected to on docker run. - // Ideally all these network-specific endpoint configurations must be moved under - // container.NetworkSettings.Networks[n.Name()] - if n.Name() == container.HostConfig.NetworkMode.NetworkName() || - (n.Name() == defaultNetName && container.HostConfig.NetworkMode.IsDefault()) { - if container.Config.MacAddress != "" { - mac, err := net.ParseMAC(container.Config.MacAddress) - if err != nil { - return nil, err - } - - genericOption := options.Generic{ - netlabel.MacAddress: mac, - } - - createOptions = append(createOptions, libnetwork.EndpointOptionGeneric(genericOption)) - } - - } - - // Port-mapping rules belong to the container & applicable only to non-internal networks - portmaps := GetSandboxPortMapInfo(sb) - if n.Info().Internal() || len(portmaps) > 0 { - return createOptions, nil - } - - if container.HostConfig.PortBindings != nil { - for p, b := range container.HostConfig.PortBindings { - bindings[p] = []nat.PortBinding{} - for _, bb := range b { - bindings[p] = append(bindings[p], nat.PortBinding{ - HostIP: bb.HostIP, - HostPort: bb.HostPort, - }) - } - } - } - - portSpecs := container.Config.ExposedPorts - ports := make([]nat.Port, len(portSpecs)) - var i int - for p := range portSpecs { - ports[i] = p - i++ - } - nat.SortPortMap(ports, bindings) - for _, port := range ports { - expose := types.TransportPort{} - expose.Proto = types.ParseProtocol(port.Proto()) - expose.Port = uint16(port.Int()) - exposeList = append(exposeList, expose) - - pb := types.PortBinding{Port: expose.Port, Proto: expose.Proto} - binding := bindings[port] - for i := 0; i < len(binding); i++ { - pbCopy := pb.GetCopy() - newP, err := nat.NewPort(nat.SplitProtoPort(binding[i].HostPort)) - var portStart, portEnd int - if err == nil { - portStart, portEnd, err = newP.Range() - } - if err != nil { - return nil, errors.Wrapf(err, "Error parsing HostPort value (%s)", binding[i].HostPort) - } - pbCopy.HostPort = uint16(portStart) - pbCopy.HostPortEnd = uint16(portEnd) - pbCopy.HostIP = net.ParseIP(binding[i].HostIP) - pbList = append(pbList, pbCopy) - } - - if container.HostConfig.PublishAllPorts && len(binding) == 0 { - pbList = append(pbList, pb) - } - } - - var dns []string - - if len(container.HostConfig.DNS) > 0 { - dns = container.HostConfig.DNS - } else if len(daemonDNS) > 0 { - dns = daemonDNS - } - - if len(dns) > 0 { - createOptions = append(createOptions, - libnetwork.CreateOptionDNS(dns)) - } - - createOptions = append(createOptions, - libnetwork.CreateOptionPortMapping(pbList), - libnetwork.CreateOptionExposedPorts(exposeList)) - - return createOptions, nil -} - // UpdateMonitor updates monitor configure for running container func (container *Container) UpdateMonitor(restartPolicy containertypes.RestartPolicy) { type policySetter interface { diff --git a/components/engine/daemon/container_operations.go b/components/engine/daemon/container_operations.go index 36790e02bf..df84f88f3f 100644 --- a/components/engine/daemon/container_operations.go +++ b/components/engine/daemon/container_operations.go @@ -31,7 +31,7 @@ var ( // ErrRootFSReadOnly is returned when a container // rootfs is marked readonly. ErrRootFSReadOnly = errors.New("container rootfs is marked read-only") - getPortMapInfo = container.GetSandboxPortMapInfo + getPortMapInfo = getSandboxPortMapInfo ) func (daemon *Daemon) getDNSSearchSettings(container *container.Container) []string { @@ -288,7 +288,7 @@ func (daemon *Daemon) updateNetworkSettings(container *container.Container, n li } func (daemon *Daemon) updateEndpointNetworkSettings(container *container.Container, n libnetwork.Network, ep libnetwork.Endpoint) error { - if err := container.BuildEndpointInfo(n, ep); err != nil { + if err := buildEndpointInfo(container.NetworkSettings, n, ep); err != nil { return err } @@ -572,7 +572,7 @@ func (daemon *Daemon) allocateNetwork(container *container.Container) error { if err != nil { return err } - container.UpdateSandboxNetworkSettings(sb) + updateSandboxNetworkSettings(container, sb) defer func() { if err != nil { sb.Delete() @@ -740,7 +740,7 @@ func (daemon *Daemon) connectToNetwork(container *container.Container, idOrName controller := daemon.netController sb := daemon.getNetworkSandbox(container) - createOptions, err := container.BuildCreateEndpointOptions(n, endpointConfig, sb, daemon.configStore.DNS) + createOptions, err := buildCreateEndpointOptions(container, n, endpointConfig, sb, daemon.configStore.DNS) if err != nil { return err } @@ -779,10 +779,10 @@ func (daemon *Daemon) connectToNetwork(container *container.Container, idOrName return err } - container.UpdateSandboxNetworkSettings(sb) + updateSandboxNetworkSettings(container, sb) } - joinOptions, err := container.BuildJoinOptions(n) + joinOptions, err := buildJoinOptions(container.NetworkSettings, n) if err != nil { return err } @@ -798,7 +798,7 @@ func (daemon *Daemon) connectToNetwork(container *container.Container, idOrName } } - if err := container.UpdateJoinInfo(n, ep); err != nil { + if err := updateJoinInfo(container.NetworkSettings, n, ep); err != nil { return fmt.Errorf("Updating join info failed: %v", err) } @@ -809,6 +809,37 @@ func (daemon *Daemon) connectToNetwork(container *container.Container, idOrName return nil } +func updateJoinInfo(networkSettings *network.Settings, n libnetwork.Network, ep libnetwork.Endpoint) error { // nolint: interfacer + if ep == nil { + return errors.New("invalid enppoint whhile building portmap info") + } + + if networkSettings == nil { + return errors.New("invalid network settings while building port map info") + } + + if len(networkSettings.Ports) == 0 { + pm, err := getEndpointPortMapInfo(ep) + if err != nil { + return err + } + networkSettings.Ports = pm + } + + epInfo := ep.Info() + if epInfo == nil { + // It is not an error to get an empty endpoint info + return nil + } + if epInfo.Gateway() != nil { + networkSettings.Networks[n.Name()].Gateway = epInfo.Gateway().String() + } + if epInfo.GatewayIPv6().To16() != nil { + networkSettings.Networks[n.Name()].IPv6Gateway = epInfo.GatewayIPv6().String() + } + return nil +} + // ForceEndpointDelete deletes an endpoint from a network forcefully func (daemon *Daemon) ForceEndpointDelete(name string, networkName string) error { n, err := daemon.FindNetwork(networkName) @@ -1110,3 +1141,10 @@ func getNetworkID(name string, endpointSettings *networktypes.EndpointSettings) } return name } + +// updateSandboxNetworkSettings updates the sandbox ID and Key. +func updateSandboxNetworkSettings(c *container.Container, sb libnetwork.Sandbox) error { + c.NetworkSettings.SandboxID = sb.ID() + c.NetworkSettings.SandboxKey = sb.Key() + return nil +} diff --git a/components/engine/daemon/container_operations_windows.go b/components/engine/daemon/container_operations_windows.go index a4541b0634..562528a8ef 100644 --- a/components/engine/daemon/container_operations_windows.go +++ b/components/engine/daemon/container_operations_windows.go @@ -174,7 +174,7 @@ func (daemon *Daemon) initializeNetworkingPaths(container *container.Container, continue } - ep, err := nc.GetEndpointInNetwork(sn) + ep, err := getEndpointInNetwork(nc.Name, sn) if err != nil { continue } diff --git a/components/engine/daemon/network.go b/components/engine/daemon/network.go index 414caf1c58..4263409be8 100644 --- a/components/engine/daemon/network.go +++ b/components/engine/daemon/network.go @@ -6,19 +6,27 @@ import ( "net" "runtime" "sort" + "strconv" "strings" "sync" "github.com/docker/docker/api/types" + containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" + "github.com/docker/docker/container" clustertypes "github.com/docker/docker/daemon/cluster/provider" + internalnetwork "github.com/docker/docker/daemon/network" "github.com/docker/docker/errdefs" + "github.com/docker/docker/opts" "github.com/docker/docker/pkg/plugingetter" "github.com/docker/docker/runconfig" + "github.com/docker/go-connections/nat" "github.com/docker/libnetwork" lncluster "github.com/docker/libnetwork/cluster" "github.com/docker/libnetwork/driverapi" "github.com/docker/libnetwork/ipamapi" + "github.com/docker/libnetwork/netlabel" + "github.com/docker/libnetwork/options" networktypes "github.com/docker/libnetwork/types" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -48,7 +56,7 @@ func (daemon *Daemon) NetworkControllerEnabled() bool { func (daemon *Daemon) FindNetwork(term string) (libnetwork.Network, error) { listByFullName := []libnetwork.Network{} listByPartialID := []libnetwork.Network{} - for _, nw := range daemon.GetNetworks() { + for _, nw := range daemon.getAllNetworks() { if nw.ID() == term { return nw, nil } @@ -575,7 +583,7 @@ func (daemon *Daemon) GetNetworks() []libnetwork.Network { // clearAttachableNetworks removes the attachable networks // after disconnecting any connected container func (daemon *Daemon) clearAttachableNetworks() { - for _, n := range daemon.GetNetworks() { + for _, n := range daemon.getAllNetworks() { if !n.Info().Attachable() { continue } @@ -599,3 +607,312 @@ func (daemon *Daemon) clearAttachableNetworks() { } } } + +// buildCreateEndpointOptions builds endpoint options from a given network. +func buildCreateEndpointOptions(c *container.Container, n libnetwork.Network, epConfig *network.EndpointSettings, sb libnetwork.Sandbox, daemonDNS []string) ([]libnetwork.EndpointOption, error) { + var ( + bindings = make(nat.PortMap) + pbList []networktypes.PortBinding + exposeList []networktypes.TransportPort + createOptions []libnetwork.EndpointOption + ) + + defaultNetName := runconfig.DefaultDaemonNetworkMode().NetworkName() + + if (!c.EnableServiceDiscoveryOnDefaultNetwork() && n.Name() == defaultNetName) || + c.NetworkSettings.IsAnonymousEndpoint { + createOptions = append(createOptions, libnetwork.CreateOptionAnonymous()) + } + + if epConfig != nil { + ipam := epConfig.IPAMConfig + + if ipam != nil { + var ( + ipList []net.IP + ip, ip6, linkip net.IP + ) + + for _, ips := range ipam.LinkLocalIPs { + if linkip = net.ParseIP(ips); linkip == nil && ips != "" { + return nil, errors.Errorf("Invalid link-local IP address: %s", ipam.LinkLocalIPs) + } + ipList = append(ipList, linkip) + + } + + if ip = net.ParseIP(ipam.IPv4Address); ip == nil && ipam.IPv4Address != "" { + return nil, errors.Errorf("Invalid IPv4 address: %s)", ipam.IPv4Address) + } + + if ip6 = net.ParseIP(ipam.IPv6Address); ip6 == nil && ipam.IPv6Address != "" { + return nil, errors.Errorf("Invalid IPv6 address: %s)", ipam.IPv6Address) + } + + createOptions = append(createOptions, + libnetwork.CreateOptionIpam(ip, ip6, ipList, nil)) + + } + + for _, alias := range epConfig.Aliases { + createOptions = append(createOptions, libnetwork.CreateOptionMyAlias(alias)) + } + for k, v := range epConfig.DriverOpts { + createOptions = append(createOptions, libnetwork.EndpointOptionGeneric(options.Generic{k: v})) + } + } + + if c.NetworkSettings.Service != nil { + svcCfg := c.NetworkSettings.Service + + var vip string + if svcCfg.VirtualAddresses[n.ID()] != nil { + vip = svcCfg.VirtualAddresses[n.ID()].IPv4 + } + + var portConfigs []*libnetwork.PortConfig + for _, portConfig := range svcCfg.ExposedPorts { + portConfigs = append(portConfigs, &libnetwork.PortConfig{ + Name: portConfig.Name, + Protocol: libnetwork.PortConfig_Protocol(portConfig.Protocol), + TargetPort: portConfig.TargetPort, + PublishedPort: portConfig.PublishedPort, + }) + } + + createOptions = append(createOptions, libnetwork.CreateOptionService(svcCfg.Name, svcCfg.ID, net.ParseIP(vip), portConfigs, svcCfg.Aliases[n.ID()])) + } + + if !containertypes.NetworkMode(n.Name()).IsUserDefined() { + createOptions = append(createOptions, libnetwork.CreateOptionDisableResolution()) + } + + // configs that are applicable only for the endpoint in the network + // to which container was connected to on docker run. + // Ideally all these network-specific endpoint configurations must be moved under + // container.NetworkSettings.Networks[n.Name()] + if n.Name() == c.HostConfig.NetworkMode.NetworkName() || + (n.Name() == defaultNetName && c.HostConfig.NetworkMode.IsDefault()) { + if c.Config.MacAddress != "" { + mac, err := net.ParseMAC(c.Config.MacAddress) + if err != nil { + return nil, err + } + + genericOption := options.Generic{ + netlabel.MacAddress: mac, + } + + createOptions = append(createOptions, libnetwork.EndpointOptionGeneric(genericOption)) + } + + } + + // Port-mapping rules belong to the container & applicable only to non-internal networks + portmaps := getSandboxPortMapInfo(sb) + if n.Info().Internal() || len(portmaps) > 0 { + return createOptions, nil + } + + if c.HostConfig.PortBindings != nil { + for p, b := range c.HostConfig.PortBindings { + bindings[p] = []nat.PortBinding{} + for _, bb := range b { + bindings[p] = append(bindings[p], nat.PortBinding{ + HostIP: bb.HostIP, + HostPort: bb.HostPort, + }) + } + } + } + + portSpecs := c.Config.ExposedPorts + ports := make([]nat.Port, len(portSpecs)) + var i int + for p := range portSpecs { + ports[i] = p + i++ + } + nat.SortPortMap(ports, bindings) + for _, port := range ports { + expose := networktypes.TransportPort{} + expose.Proto = networktypes.ParseProtocol(port.Proto()) + expose.Port = uint16(port.Int()) + exposeList = append(exposeList, expose) + + pb := networktypes.PortBinding{Port: expose.Port, Proto: expose.Proto} + binding := bindings[port] + for i := 0; i < len(binding); i++ { + pbCopy := pb.GetCopy() + newP, err := nat.NewPort(nat.SplitProtoPort(binding[i].HostPort)) + var portStart, portEnd int + if err == nil { + portStart, portEnd, err = newP.Range() + } + if err != nil { + return nil, errors.Wrapf(err, "Error parsing HostPort value (%s)", binding[i].HostPort) + } + pbCopy.HostPort = uint16(portStart) + pbCopy.HostPortEnd = uint16(portEnd) + pbCopy.HostIP = net.ParseIP(binding[i].HostIP) + pbList = append(pbList, pbCopy) + } + + if c.HostConfig.PublishAllPorts && len(binding) == 0 { + pbList = append(pbList, pb) + } + } + + var dns []string + + if len(c.HostConfig.DNS) > 0 { + dns = c.HostConfig.DNS + } else if len(daemonDNS) > 0 { + dns = daemonDNS + } + + if len(dns) > 0 { + createOptions = append(createOptions, + libnetwork.CreateOptionDNS(dns)) + } + + createOptions = append(createOptions, + libnetwork.CreateOptionPortMapping(pbList), + libnetwork.CreateOptionExposedPorts(exposeList)) + + return createOptions, nil +} + +// getEndpointInNetwork returns the container's endpoint to the provided network. +func getEndpointInNetwork(name string, n libnetwork.Network) (libnetwork.Endpoint, error) { + endpointName := strings.TrimPrefix(name, "/") + return n.EndpointByName(endpointName) +} + +// getSandboxPortMapInfo retrieves the current port-mapping programmed for the given sandbox +func getSandboxPortMapInfo(sb libnetwork.Sandbox) nat.PortMap { + pm := nat.PortMap{} + if sb == nil { + return pm + } + + for _, ep := range sb.Endpoints() { + pm, _ = getEndpointPortMapInfo(ep) + if len(pm) > 0 { + break + } + } + return pm +} + +func getEndpointPortMapInfo(ep libnetwork.Endpoint) (nat.PortMap, error) { + pm := nat.PortMap{} + driverInfo, err := ep.DriverInfo() + if err != nil { + return pm, err + } + + if driverInfo == nil { + // It is not an error for epInfo to be nil + return pm, nil + } + + if expData, ok := driverInfo[netlabel.ExposedPorts]; ok { + if exposedPorts, ok := expData.([]networktypes.TransportPort); ok { + for _, tp := range exposedPorts { + natPort, err := nat.NewPort(tp.Proto.String(), strconv.Itoa(int(tp.Port))) + if err != nil { + return pm, fmt.Errorf("Error parsing Port value(%v):%v", tp.Port, err) + } + pm[natPort] = nil + } + } + } + + mapData, ok := driverInfo[netlabel.PortMap] + if !ok { + return pm, nil + } + + if portMapping, ok := mapData.([]networktypes.PortBinding); ok { + for _, pp := range portMapping { + natPort, err := nat.NewPort(pp.Proto.String(), strconv.Itoa(int(pp.Port))) + if err != nil { + return pm, err + } + natBndg := nat.PortBinding{HostIP: pp.HostIP.String(), HostPort: strconv.Itoa(int(pp.HostPort))} + pm[natPort] = append(pm[natPort], natBndg) + } + } + + return pm, nil +} + +// buildEndpointInfo sets endpoint-related fields on container.NetworkSettings based on the provided network and endpoint. +func buildEndpointInfo(networkSettings *internalnetwork.Settings, n libnetwork.Network, ep libnetwork.Endpoint) error { + if ep == nil { + return errors.New("endpoint cannot be nil") + } + + if networkSettings == nil { + return errors.New("network cannot be nil") + } + + epInfo := ep.Info() + if epInfo == nil { + // It is not an error to get an empty endpoint info + return nil + } + + if _, ok := networkSettings.Networks[n.Name()]; !ok { + networkSettings.Networks[n.Name()] = &internalnetwork.EndpointSettings{ + EndpointSettings: &network.EndpointSettings{}, + } + } + networkSettings.Networks[n.Name()].NetworkID = n.ID() + networkSettings.Networks[n.Name()].EndpointID = ep.ID() + + iface := epInfo.Iface() + if iface == nil { + return nil + } + + if iface.MacAddress() != nil { + networkSettings.Networks[n.Name()].MacAddress = iface.MacAddress().String() + } + + if iface.Address() != nil { + ones, _ := iface.Address().Mask.Size() + networkSettings.Networks[n.Name()].IPAddress = iface.Address().IP.String() + networkSettings.Networks[n.Name()].IPPrefixLen = ones + } + + if iface.AddressIPv6() != nil && iface.AddressIPv6().IP.To16() != nil { + onesv6, _ := iface.AddressIPv6().Mask.Size() + networkSettings.Networks[n.Name()].GlobalIPv6Address = iface.AddressIPv6().IP.String() + networkSettings.Networks[n.Name()].GlobalIPv6PrefixLen = onesv6 + } + + return nil +} + +// buildJoinOptions builds endpoint Join options from a given network. +func buildJoinOptions(networkSettings *internalnetwork.Settings, n interface { + Name() string +}) ([]libnetwork.EndpointOption, error) { + var joinOptions []libnetwork.EndpointOption + if epConfig, ok := networkSettings.Networks[n.Name()]; ok { + for _, str := range epConfig.Links { + name, alias, err := opts.ParseLink(str) + if err != nil { + return nil, err + } + joinOptions = append(joinOptions, libnetwork.CreateOptionAlias(name, alias)) + } + for k, v := range epConfig.DriverOpts { + joinOptions = append(joinOptions, libnetwork.EndpointOptionGeneric(options.Generic{k: v})) + } + } + + return joinOptions, nil +} diff --git a/components/engine/daemon/oci_windows.go b/components/engine/daemon/oci_windows.go index edea2bcedc..f00ab3363d 100644 --- a/components/engine/daemon/oci_windows.go +++ b/components/engine/daemon/oci_windows.go @@ -156,7 +156,7 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) { continue } - ep, err := c.GetEndpointInNetwork(sn) + ep, err := getEndpointInNetwork(c.Name, sn) if err != nil { continue } From 645a5ebf1b6f14a5e410dc3077c23bb9dc930a5f Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 10 May 2018 16:47:12 -0400 Subject: [PATCH 03/10] Use mount package to unmount container stuff Really these mounts shouldn't be in the container pacakge. Signed-off-by: Brian Goff Upstream-commit: d6558ad6a4552fc745e893f93ae190710d1a36bc Component: engine --- .../engine/container/container_linux.go | 9 -------- .../engine/container/container_notlinux.go | 23 ------------------- components/engine/container/container_unix.go | 4 ++-- 3 files changed, 2 insertions(+), 34 deletions(-) delete mode 100644 components/engine/container/container_linux.go delete mode 100644 components/engine/container/container_notlinux.go diff --git a/components/engine/container/container_linux.go b/components/engine/container/container_linux.go deleted file mode 100644 index ea2ebabaaa..0000000000 --- a/components/engine/container/container_linux.go +++ /dev/null @@ -1,9 +0,0 @@ -package container // import "github.com/docker/docker/container" - -import ( - "golang.org/x/sys/unix" -) - -func detachMounted(path string) error { - return unix.Unmount(path, unix.MNT_DETACH) -} diff --git a/components/engine/container/container_notlinux.go b/components/engine/container/container_notlinux.go deleted file mode 100644 index dc9751b8c7..0000000000 --- a/components/engine/container/container_notlinux.go +++ /dev/null @@ -1,23 +0,0 @@ -// +build freebsd - -package container // import "github.com/docker/docker/container" - -import ( - "golang.org/x/sys/unix" -) - -func detachMounted(path string) error { - // FreeBSD do not support the lazy unmount or MNT_DETACH feature. - // Therefore there are separate definitions for this. - return unix.Unmount(path, 0) -} - -// SecretMounts returns the mounts for the secret path -func (container *Container) SecretMounts() []Mount { - return nil -} - -// UnmountSecrets unmounts the fs for secrets -func (container *Container) UnmountSecrets() error { - return nil -} diff --git a/components/engine/container/container_unix.go b/components/engine/container/container_unix.go index 9397cdf60b..c77ea07a18 100644 --- a/components/engine/container/container_unix.go +++ b/components/engine/container/container_unix.go @@ -1,4 +1,4 @@ -// +build linux freebsd +// +build !windows package container // import "github.com/docker/docker/container" @@ -380,7 +380,7 @@ func (container *Container) DetachAndUnmount(volumeEventLog func(name, action st } for _, mountPath := range mountPaths { - if err := detachMounted(mountPath); err != nil { + if err := mount.Unmount(mountPath); err != nil { logrus.Warnf("%s unmountVolumes: Failed to do lazy umount fo volume '%s': %v", container.ID, mountPath, err) } } From 5ecbe968134ec9a1f171e5f03dbe701b6b44616f Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Sun, 13 May 2018 12:53:05 -0400 Subject: [PATCH 04/10] image spec formatting fix Embedded new lines aren't interpreted correctly in markdown renderers (they are treated as preformatted text instead). I removed the embedded newlines in the docker image spec. Signed-off-by: Alex Goodman Upstream-commit: 4fb59c20a4fb54f944fe170d0ff1d00eb4a24d6f Component: engine --- components/engine/image/spec/v1.1.md | 18 ++------------ components/engine/image/spec/v1.2.md | 31 +++++------------------ components/engine/image/spec/v1.md | 37 ++++++++++------------------ 3 files changed, 21 insertions(+), 65 deletions(-) diff --git a/components/engine/image/spec/v1.1.md b/components/engine/image/spec/v1.1.md index ce761f112a..de74d91a19 100644 --- a/components/engine/image/spec/v1.1.md +++ b/components/engine/image/spec/v1.1.md @@ -223,9 +223,7 @@ whitespace. It has been added to this example for clarity. container using the image. This field can be null, in which case any execution parameters should be specified at creation of the container. -

Container RunConfig Field Descriptions

-
User string @@ -234,9 +232,7 @@ whitespace. It has been added to this example for clarity.

The username or UID which the process in the container should run as. This acts as a default value to use when the value is not specified when creating a container.

-

All of the following are valid:

-
  • user
  • uid
  • @@ -245,7 +241,6 @@ whitespace. It has been added to this example for clarity.
  • uid:group
  • user:gid
-

If group/gid is not specified, the default group and supplementary groups of the given user/uid in /etc/passwd @@ -284,13 +279,11 @@ whitespace. It has been added to this example for clarity. map[string]struct{} and is represented in JSON as an object mapping its keys to an empty object. Here is an example: -

{
     "8080": {},
     "53/udp": {},
     "2356/tcp": {}
 }
- Its keys can be in the format of:
  • @@ -304,10 +297,8 @@ whitespace. It has been added to this example for clarity.
with the default protocol being "tcp" if not - specified. - - These values act as defaults and are merged with any specified - when creating a container. + specified. These values act as defaults and are merged with any + specified when creating a container.
Env array of strings @@ -367,7 +358,6 @@ whitespace. It has been added to this example for clarity. The rootfs key references the layer content addresses used by the image. This makes the image config hash depend on the filesystem hash. rootfs has two subkeys: -
  • type is usually set to layers. @@ -376,10 +366,7 @@ whitespace. It has been added to this example for clarity. diff_ids is an array of layer content hashes (DiffIDs), in order from bottom-most to top-most.
- - Here is an example rootfs section: -
"rootfs": {
   "diff_ids": [
     "sha256:c6f988f4874bb0add23a778f753c65efe992244e148a1d2ec2a8b664fb66bbd1",
@@ -396,7 +383,6 @@ whitespace. It has been added to this example for clarity.
         history is an array of objects describing the history of
         each layer. The array is ordered from bottom-most layer to top-most
         layer. The object has the following fields.
-
         
  • created: Creation time, expressed as a ISO-8601 formatted diff --git a/components/engine/image/spec/v1.2.md b/components/engine/image/spec/v1.2.md index 789680c7a7..2ea3feec92 100644 --- a/components/engine/image/spec/v1.2.md +++ b/components/engine/image/spec/v1.2.md @@ -223,9 +223,7 @@ whitespace. It has been added to this example for clarity. container using the image. This field can be null, in which case any execution parameters should be specified at creation of the container. -

    Container RunConfig Field Descriptions

    -
    User string @@ -234,9 +232,7 @@ whitespace. It has been added to this example for clarity.

    The username or UID which the process in the container should run as. This acts as a default value to use when the value is not specified when creating a container.

    -

    All of the following are valid:

    -
    • user
    • uid
    • @@ -245,7 +241,6 @@ whitespace. It has been added to this example for clarity.
    • uid:group
    • user:gid
    -

    If group/gid is not specified, the default group and supplementary groups of the given user/uid in /etc/passwd @@ -284,13 +279,11 @@ whitespace. It has been added to this example for clarity. map[string]struct{} and is represented in JSON as an object mapping its keys to an empty object. Here is an example: -

    {
         "8080": {},
         "53/udp": {},
         "2356/tcp": {}
     }
    - Its keys can be in the format of:
    • @@ -304,10 +297,8 @@ whitespace. It has been added to this example for clarity.
    with the default protocol being "tcp" if not - specified. - - These values act as defaults and are merged with any specified - when creating a container. + specified. These values act as defaults and are merged with + any specified when creating a container.
    Env array of strings @@ -364,7 +355,6 @@ whitespace. It has been added to this example for clarity.
  • ["CMD", arg1, arg2, ...] : exec arguments directly
  • ["CMD-SHELL", command] : run command with system's default shell
- The test command should exit with a status of 0 if the container is healthy, or with 1 if it is unhealthy. @@ -387,12 +377,10 @@ whitespace. It has been added to this example for clarity. The number of consecutive failures needed to consider a container as unhealthy.
- In each case, the field can be omitted to indicate that the - value should be inherited from the base layer. - - These values act as defaults and are merged with any specified - when creating a container. + value should be inherited from the base layer. These values act + as defaults and are merged with any specified when creating a + container.
Volumes struct @@ -426,7 +414,6 @@ whitespace. It has been added to this example for clarity. The rootfs key references the layer content addresses used by the image. This makes the image config hash depend on the filesystem hash. rootfs has two subkeys: -
  • type is usually set to layers. @@ -435,10 +422,7 @@ whitespace. It has been added to this example for clarity. diff_ids is an array of layer content hashes (DiffIDs), in order from bottom-most to top-most.
- - Here is an example rootfs section: -
"rootfs": {
   "diff_ids": [
     "sha256:c6f988f4874bb0add23a778f753c65efe992244e148a1d2ec2a8b664fb66bbd1",
@@ -455,7 +439,6 @@ whitespace. It has been added to this example for clarity.
         history is an array of objects describing the history of
         each layer. The array is ordered from bottom-most layer to top-most
         layer. The object has the following fields.
-
         
  • created: Creation time, expressed as a ISO-8601 formatted @@ -478,9 +461,7 @@ whitespace. It has been added to this example for clarity. filesystem).
- -Here is an example history section: - + Here is an example history section:
"history": [
   {
     "created": "2015-10-31T22:22:54.690851953Z",
diff --git a/components/engine/image/spec/v1.md b/components/engine/image/spec/v1.md
index fce3a06e36..c1415947f1 100644
--- a/components/engine/image/spec/v1.md
+++ b/components/engine/image/spec/v1.md
@@ -17,12 +17,10 @@ This specification uses the following terms:
     
Images are composed of layers. Image layer is a general term which may be used to refer to one or both of the following: -
  1. The metadata for the layer, described in the JSON format.
  2. The filesystem changes described by a layer.
- To refer to the former you may use the term Layer JSON or Layer Metadata. To refer to the latter you may use the term Image Filesystem Changeset or Image Diff. @@ -244,9 +242,7 @@ Here is an example image JSON file: container using the image. This field can be null, in which case any execution parameters should be specified at creation of the container. -

Container RunConfig Field Descriptions

-
User string @@ -255,9 +251,7 @@ Here is an example image JSON file:

The username or UID which the process in the container should run as. This acts as a default value to use when the value is not specified when creating a container.

-

All of the following are valid:

-
  • user
  • uid
  • @@ -266,7 +260,6 @@ Here is an example image JSON file:
  • uid:group
  • user:gid
-

If group/gid is not specified, the default group and supplementary groups of the given user/uid in /etc/passwd @@ -305,13 +298,11 @@ Here is an example image JSON file: map[string]struct{} and is represented in JSON as an object mapping its keys to an empty object. Here is an example: -

{
     "8080": {},
     "53/udp": {},
     "2356/tcp": {}
 }
- Its keys can be in the format of:
  • @@ -325,9 +316,7 @@ Here is an example image JSON file:
with the default protocol being "tcp" if not - specified. - - These values act as defaults and are merged with any specified + specified. These values act as defaults and are merged with any specified when creating a container.
@@ -502,21 +491,21 @@ For example, here's what the full archive of `library/busybox` is (displayed in ``` . ├── 5785b62b697b99a5af6cd5d0aabc804d5748abbb6d3d07da5d1d3795f2dcc83e -│   ├── VERSION -│   ├── json -│   └── layer.tar +│ ├── VERSION +│ ├── json +│ └── layer.tar ├── a7b8b41220991bfc754d7ad445ad27b7f272ab8b4a2c175b9512b97471d02a8a -│   ├── VERSION -│   ├── json -│   └── layer.tar +│ ├── VERSION +│ ├── json +│ └── layer.tar ├── a936027c5ca8bf8f517923169a233e391cbb38469a75de8383b5228dc2d26ceb -│   ├── VERSION -│   ├── json -│   └── layer.tar +│ ├── VERSION +│ ├── json +│ └── layer.tar ├── f60c56784b832dd990022afc120b8136ab3da9528094752ae13fe63a2d28dc8c -│   ├── VERSION -│   ├── json -│   └── layer.tar +│ ├── VERSION +│ ├── json +│ └── layer.tar └── repositories ``` From 74db940ed75a1400227309b9559e9924b715ce2c Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Fri, 11 May 2018 20:04:34 +0000 Subject: [PATCH 05/10] Avoid unique name usage in TestDockerNetworkConnectAlias In TestDockerNetworkConnectAlias the network and container names used are unique which are not preferred. This fix address the issue by appending t.Name() so that names are randomized. Signed-off-by: Yong Tang Upstream-commit: e7b5f4500dc77ec55e5ef202065fbc92218ce394 Component: engine --- .../engine/integration/service/network_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/components/engine/integration/service/network_test.go b/components/engine/integration/service/network_test.go index 7db171a125..09f663b323 100644 --- a/components/engine/integration/service/network_test.go +++ b/components/engine/integration/service/network_test.go @@ -20,14 +20,14 @@ func TestDockerNetworkConnectAlias(t *testing.T) { defer client.Close() ctx := context.Background() - name := "test-alias" + name := t.Name() + "test-alias" _, err := client.NetworkCreate(ctx, name, types.NetworkCreate{ Driver: "overlay", Attachable: true, }) assert.NilError(t, err) - container.Create(t, ctx, client, container.WithName("ng1"), func(c *container.TestContainerConfig) { + cID1 := container.Create(t, ctx, client, func(c *container.TestContainerConfig) { c.NetworkingConfig = &network.NetworkingConfig{ map[string]*network.EndpointSettings{ name: {}, @@ -35,22 +35,22 @@ func TestDockerNetworkConnectAlias(t *testing.T) { } }) - err = client.NetworkConnect(ctx, name, "ng1", &network.EndpointSettings{ + err = client.NetworkConnect(ctx, name, cID1, &network.EndpointSettings{ Aliases: []string{ "aaa", }, }) assert.NilError(t, err) - err = client.ContainerStart(ctx, "ng1", types.ContainerStartOptions{}) + err = client.ContainerStart(ctx, cID1, types.ContainerStartOptions{}) assert.NilError(t, err) - ng1, err := client.ContainerInspect(ctx, "ng1") + ng1, err := client.ContainerInspect(ctx, cID1) assert.NilError(t, err) assert.Check(t, is.Equal(len(ng1.NetworkSettings.Networks[name].Aliases), 2)) assert.Check(t, is.Equal(ng1.NetworkSettings.Networks[name].Aliases[0], "aaa")) - container.Create(t, ctx, client, container.WithName("ng2"), func(c *container.TestContainerConfig) { + cID2 := container.Create(t, ctx, client, func(c *container.TestContainerConfig) { c.NetworkingConfig = &network.NetworkingConfig{ map[string]*network.EndpointSettings{ name: {}, @@ -58,17 +58,17 @@ func TestDockerNetworkConnectAlias(t *testing.T) { } }) - err = client.NetworkConnect(ctx, name, "ng2", &network.EndpointSettings{ + err = client.NetworkConnect(ctx, name, cID2, &network.EndpointSettings{ Aliases: []string{ "bbb", }, }) assert.NilError(t, err) - err = client.ContainerStart(ctx, "ng2", types.ContainerStartOptions{}) + err = client.ContainerStart(ctx, cID2, types.ContainerStartOptions{}) assert.NilError(t, err) - ng2, err := client.ContainerInspect(ctx, "ng2") + ng2, err := client.ContainerInspect(ctx, cID2) assert.NilError(t, err) assert.Check(t, is.Equal(len(ng2.NetworkSettings.Networks[name].Aliases), 2)) assert.Check(t, is.Equal(ng2.NetworkSettings.Networks[name].Aliases[0], "bbb")) From 1cb0dc30a7008e1e3d65b45e3120f587b88fe032 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 14 May 2018 11:03:48 -0400 Subject: [PATCH 06/10] Fix swagger volume type generation This was broken by bf6a790f00ab96bb8857e3c73502909ee5e36217 Signed-off-by: Brian Goff Upstream-commit: b16b125bb4326e2eec9948fd54ca8c5d83eba36a Component: engine --- .../engine/api/server/router/volume/volume_routes.go | 4 ++-- .../types/volume/{volumes_create.go => volume_create.go} | 8 ++++---- .../api/types/volume/{volumes_list.go => volume_list.go} | 8 ++++---- components/engine/client/interface.go | 4 ++-- components/engine/client/volume_create.go | 2 +- components/engine/client/volume_create_test.go | 4 ++-- components/engine/client/volume_list.go | 4 ++-- components/engine/client/volume_list_test.go | 2 +- .../engine/daemon/cluster/executor/container/container.go | 4 ++-- components/engine/hack/generate-swagger-api.sh | 4 ++-- .../engine/hack/integration-cli-on-swarm/host/volume.go | 2 +- components/engine/hack/validate/gometalinter.json | 1 + .../integration/plugin/authz/authz_plugin_v2_test.go | 6 +++--- components/engine/integration/volume/volume_test.go | 4 ++-- 14 files changed, 29 insertions(+), 28 deletions(-) rename components/engine/api/types/volume/{volumes_create.go => volume_create.go} (81%) rename components/engine/api/types/volume/{volumes_list.go => volume_list.go} (74%) diff --git a/components/engine/api/server/router/volume/volume_routes.go b/components/engine/api/server/router/volume/volume_routes.go index 01d710e727..602786ff8b 100644 --- a/components/engine/api/server/router/volume/volume_routes.go +++ b/components/engine/api/server/router/volume/volume_routes.go @@ -22,7 +22,7 @@ func (v *volumeRouter) getVolumesList(ctx context.Context, w http.ResponseWriter if err != nil { return err } - return httputils.WriteJSON(w, http.StatusOK, &volumetypes.VolumesListOKBody{Volumes: volumes, Warnings: warnings}) + return httputils.WriteJSON(w, http.StatusOK, &volumetypes.VolumeListOKBody{Volumes: volumes, Warnings: warnings}) } func (v *volumeRouter) getVolumeByName(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { @@ -46,7 +46,7 @@ func (v *volumeRouter) postVolumesCreate(ctx context.Context, w http.ResponseWri return err } - var req volumetypes.VolumesCreateBody + var req volumetypes.VolumeCreateBody if err := json.NewDecoder(r.Body).Decode(&req); err != nil { if err == io.EOF { return errdefs.InvalidParameter(errors.New("got EOF while reading request body")) diff --git a/components/engine/api/types/volume/volumes_create.go b/components/engine/api/types/volume/volume_create.go similarity index 81% rename from components/engine/api/types/volume/volumes_create.go rename to components/engine/api/types/volume/volume_create.go index b2dd3a419d..539e9b97d9 100644 --- a/components/engine/api/types/volume/volumes_create.go +++ b/components/engine/api/types/volume/volume_create.go @@ -1,4 +1,4 @@ -package volume // import "github.com/docker/docker/api/types/volume" +package volume // ---------------------------------------------------------------------------- // DO NOT EDIT THIS FILE @@ -7,9 +7,9 @@ package volume // import "github.com/docker/docker/api/types/volume" // See hack/generate-swagger-api.sh // ---------------------------------------------------------------------------- -// VolumesCreateBody volumes create body -// swagger:model VolumesCreateBody -type VolumesCreateBody struct { +// VolumeCreateBody +// swagger:model VolumeCreateBody +type VolumeCreateBody struct { // Name of the volume driver to use. // Required: true diff --git a/components/engine/api/types/volume/volumes_list.go b/components/engine/api/types/volume/volume_list.go similarity index 74% rename from components/engine/api/types/volume/volumes_list.go rename to components/engine/api/types/volume/volume_list.go index e071ca08fc..1bb279dbb3 100644 --- a/components/engine/api/types/volume/volumes_list.go +++ b/components/engine/api/types/volume/volume_list.go @@ -1,4 +1,4 @@ -package volume // import "github.com/docker/docker/api/types/volume" +package volume // ---------------------------------------------------------------------------- // DO NOT EDIT THIS FILE @@ -9,9 +9,9 @@ package volume // import "github.com/docker/docker/api/types/volume" import "github.com/docker/docker/api/types" -// VolumesListOKBody volumes list o k body -// swagger:model VolumesListOKBody -type VolumesListOKBody struct { +// VolumeListOKBody +// swagger:model VolumeListOKBody +type VolumeListOKBody struct { // List of volumes // Required: true diff --git a/components/engine/client/interface.go b/components/engine/client/interface.go index 314c940c3b..8ab25591c0 100644 --- a/components/engine/client/interface.go +++ b/components/engine/client/interface.go @@ -168,10 +168,10 @@ type SystemAPIClient interface { // VolumeAPIClient defines API client methods for the volumes type VolumeAPIClient interface { - VolumeCreate(ctx context.Context, options volumetypes.VolumesCreateBody) (types.Volume, error) + VolumeCreate(ctx context.Context, options volumetypes.VolumeCreateBody) (types.Volume, error) VolumeInspect(ctx context.Context, volumeID string) (types.Volume, error) VolumeInspectWithRaw(ctx context.Context, volumeID string) (types.Volume, []byte, error) - VolumeList(ctx context.Context, filter filters.Args) (volumetypes.VolumesListOKBody, error) + VolumeList(ctx context.Context, filter filters.Args) (volumetypes.VolumeListOKBody, error) VolumeRemove(ctx context.Context, volumeID string, force bool) error VolumesPrune(ctx context.Context, pruneFilter filters.Args) (types.VolumesPruneReport, error) } diff --git a/components/engine/client/volume_create.go b/components/engine/client/volume_create.go index 639c325286..f1f6fcdc4a 100644 --- a/components/engine/client/volume_create.go +++ b/components/engine/client/volume_create.go @@ -9,7 +9,7 @@ import ( ) // VolumeCreate creates a volume in the docker host. -func (cli *Client) VolumeCreate(ctx context.Context, options volumetypes.VolumesCreateBody) (types.Volume, error) { +func (cli *Client) VolumeCreate(ctx context.Context, options volumetypes.VolumeCreateBody) (types.Volume, error) { var volume types.Volume resp, err := cli.post(ctx, "/volumes/create", nil, options, nil) if err != nil { diff --git a/components/engine/client/volume_create_test.go b/components/engine/client/volume_create_test.go index 30b9a44db7..cfab191845 100644 --- a/components/engine/client/volume_create_test.go +++ b/components/engine/client/volume_create_test.go @@ -19,7 +19,7 @@ func TestVolumeCreateError(t *testing.T) { client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")), } - _, err := client.VolumeCreate(context.Background(), volumetypes.VolumesCreateBody{}) + _, err := client.VolumeCreate(context.Background(), volumetypes.VolumeCreateBody{}) if err == nil || err.Error() != "Error response from daemon: Server error" { t.Fatalf("expected a Server Error, got %v", err) } @@ -53,7 +53,7 @@ func TestVolumeCreate(t *testing.T) { }), } - volume, err := client.VolumeCreate(context.Background(), volumetypes.VolumesCreateBody{ + volume, err := client.VolumeCreate(context.Background(), volumetypes.VolumeCreateBody{ Name: "myvolume", Driver: "mydriver", DriverOpts: map[string]string{ diff --git a/components/engine/client/volume_list.go b/components/engine/client/volume_list.go index 4cf74831f1..284554d67c 100644 --- a/components/engine/client/volume_list.go +++ b/components/engine/client/volume_list.go @@ -10,8 +10,8 @@ import ( ) // VolumeList returns the volumes configured in the docker host. -func (cli *Client) VolumeList(ctx context.Context, filter filters.Args) (volumetypes.VolumesListOKBody, error) { - var volumes volumetypes.VolumesListOKBody +func (cli *Client) VolumeList(ctx context.Context, filter filters.Args) (volumetypes.VolumeListOKBody, error) { + var volumes volumetypes.VolumeListOKBody query := url.Values{} if filter.Len() > 0 { diff --git a/components/engine/client/volume_list_test.go b/components/engine/client/volume_list_test.go index c5e4bbb724..2a83823f7e 100644 --- a/components/engine/client/volume_list_test.go +++ b/components/engine/client/volume_list_test.go @@ -69,7 +69,7 @@ func TestVolumeList(t *testing.T) { if actualFilters != listCase.expectedFilters { return nil, fmt.Errorf("filters not set in URL query properly. Expected '%s', got %s", listCase.expectedFilters, actualFilters) } - content, err := json.Marshal(volumetypes.VolumesListOKBody{ + content, err := json.Marshal(volumetypes.VolumeListOKBody{ Volumes: []*types.Volume{ { Name: "volume", diff --git a/components/engine/daemon/cluster/executor/container/container.go b/components/engine/daemon/cluster/executor/container/container.go index 5d052dfe09..69d673bd30 100644 --- a/components/engine/daemon/cluster/executor/container/container.go +++ b/components/engine/daemon/cluster/executor/container/container.go @@ -398,7 +398,7 @@ func (c *containerConfig) hostConfig() *enginecontainer.HostConfig { } // This handles the case of volumes that are defined inside a service Mount -func (c *containerConfig) volumeCreateRequest(mount *api.Mount) *volumetypes.VolumesCreateBody { +func (c *containerConfig) volumeCreateRequest(mount *api.Mount) *volumetypes.VolumeCreateBody { var ( driverName string driverOpts map[string]string @@ -412,7 +412,7 @@ func (c *containerConfig) volumeCreateRequest(mount *api.Mount) *volumetypes.Vol } if mount.VolumeOptions != nil { - return &volumetypes.VolumesCreateBody{ + return &volumetypes.VolumeCreateBody{ Name: mount.Source, Driver: driverName, DriverOpts: driverOpts, diff --git a/components/engine/hack/generate-swagger-api.sh b/components/engine/hack/generate-swagger-api.sh index 9bbd8de5d7..a01a57387a 100755 --- a/components/engine/hack/generate-swagger-api.sh +++ b/components/engine/hack/generate-swagger-api.sh @@ -23,5 +23,5 @@ swagger generate operation -f api/swagger.yaml \ -n ContainerUpdate \ -n ContainerWait \ -n ImageHistory \ - -n VolumesCreate \ - -n VolumesList + -n VolumeCreate \ + -n VolumeList diff --git a/components/engine/hack/integration-cli-on-swarm/host/volume.go b/components/engine/hack/integration-cli-on-swarm/host/volume.go index c2f96984a0..a6ddc6fae7 100644 --- a/components/engine/hack/integration-cli-on-swarm/host/volume.go +++ b/components/engine/hack/integration-cli-on-swarm/host/volume.go @@ -41,7 +41,7 @@ func createTar(data map[string][]byte) (io.Reader, error) { // which is attached to the container. func createVolumeWithData(cli *client.Client, volumeName string, data map[string][]byte, image string) error { _, err := cli.VolumeCreate(context.Background(), - volume.VolumesCreateBody{ + volume.VolumeCreateBody{ Driver: "local", Name: volumeName, }) diff --git a/components/engine/hack/validate/gometalinter.json b/components/engine/hack/validate/gometalinter.json index 8e8da10a39..81eb1017cb 100644 --- a/components/engine/hack/validate/gometalinter.json +++ b/components/engine/hack/validate/gometalinter.json @@ -6,6 +6,7 @@ ".*\\.pb\\.go", "dockerversion/version_autogen.go", "api/types/container/container_.*", + "api/types/volume/volume_.*", "integration-cli/" ], "Skip": ["integration-cli/"], diff --git a/components/engine/integration/plugin/authz/authz_plugin_v2_test.go b/components/engine/integration/plugin/authz/authz_plugin_v2_test.go index fa3d37dc8c..93e36d76b9 100644 --- a/components/engine/integration/plugin/authz/authz_plugin_v2_test.go +++ b/components/engine/integration/plugin/authz/authz_plugin_v2_test.go @@ -77,7 +77,7 @@ func TestAuthZPluginV2Disable(t *testing.T) { d.Restart(t, "--authorization-plugin="+authzPluginNameWithTag) d.LoadBusybox(t) - _, err = client.VolumeCreate(context.Background(), volumetypes.VolumesCreateBody{Driver: "local"}) + _, err = client.VolumeCreate(context.Background(), volumetypes.VolumeCreateBody{Driver: "local"}) assert.Assert(t, err != nil) assert.Assert(t, strings.Contains(err.Error(), fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag))) @@ -86,7 +86,7 @@ func TestAuthZPluginV2Disable(t *testing.T) { assert.NilError(t, err) // now test to see if the docker api works. - _, err = client.VolumeCreate(context.Background(), volumetypes.VolumesCreateBody{Driver: "local"}) + _, err = client.VolumeCreate(context.Background(), volumetypes.VolumeCreateBody{Driver: "local"}) assert.NilError(t, err) } @@ -104,7 +104,7 @@ func TestAuthZPluginV2RejectVolumeRequests(t *testing.T) { // restart the daemon with the plugin d.Restart(t, "--authorization-plugin="+authzPluginNameWithTag) - _, err = client.VolumeCreate(context.Background(), volumetypes.VolumesCreateBody{Driver: "local"}) + _, err = client.VolumeCreate(context.Background(), volumetypes.VolumeCreateBody{Driver: "local"}) assert.Assert(t, err != nil) assert.Assert(t, strings.Contains(err.Error(), fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag))) diff --git a/components/engine/integration/volume/volume_test.go b/components/engine/integration/volume/volume_test.go index d4dc648771..36822bb447 100644 --- a/components/engine/integration/volume/volume_test.go +++ b/components/engine/integration/volume/volume_test.go @@ -26,7 +26,7 @@ func TestVolumesCreateAndList(t *testing.T) { ctx := context.Background() name := t.Name() - vol, err := client.VolumeCreate(ctx, volumetypes.VolumesCreateBody{ + vol, err := client.VolumeCreate(ctx, volumetypes.VolumeCreateBody{ Name: name, }) assert.NilError(t, err) @@ -83,7 +83,7 @@ func TestVolumesInspect(t *testing.T) { now := time.Now().Truncate(time.Minute) name := t.Name() - _, err := client.VolumeCreate(ctx, volumetypes.VolumesCreateBody{ + _, err := client.VolumeCreate(ctx, volumetypes.VolumeCreateBody{ Name: name, }) assert.NilError(t, err) From 5e354ed5337b0187ae72013a05d3567d6a2dc548 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Fri, 4 May 2018 12:41:42 -0400 Subject: [PATCH 07/10] Fix race conditions in logs API Closing the log driver was in a defer meanwhile logs are collected asyncronously, so the log driver was being closed before reads were actually finished. Signed-off-by: Brian Goff Upstream-commit: 2c252a48c252749d41079cf8c450d00a5c18296e Component: engine --- components/engine/daemon/logs.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/components/engine/daemon/logs.go b/components/engine/daemon/logs.go index 8ef8d00d23..37ca4caf63 100644 --- a/components/engine/daemon/logs.go +++ b/components/engine/daemon/logs.go @@ -22,7 +22,7 @@ import ( // // if it returns nil, the config channel will be active and return log // messages until it runs out or the context is canceled. -func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, config *types.ContainerLogsOptions) (<-chan *backend.LogMessage, bool, error) { +func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, config *types.ContainerLogsOptions) (messages <-chan *backend.LogMessage, isTTY bool, retErr error) { lg := logrus.WithFields(logrus.Fields{ "module": "daemon", "method": "(*Daemon).ContainerLogs", @@ -51,8 +51,10 @@ func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, c } if cLogCreated { defer func() { - if err = cLog.Close(); err != nil { - logrus.Errorf("Error closing logger: %v", err) + if retErr != nil { + if err = cLog.Close(); err != nil { + logrus.Errorf("Error closing logger: %v", err) + } } }() } @@ -101,6 +103,13 @@ func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, c // this goroutine functions as a shim between the logger and the caller. messageChan := make(chan *backend.LogMessage, 1) go func() { + if cLogCreated { + defer func() { + if err = cLog.Close(); err != nil { + logrus.Errorf("Error closing logger: %v", err) + } + }() + } // set up some defers defer logs.Close() From b600301f4e212d7aafaccda5363598574d529c44 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 7 May 2018 11:25:41 -0400 Subject: [PATCH 08/10] Fix some issues in logfile reader and rotation - Check errors.Cause(err) when comparing errors - Fix bug where oldest log file is not actually removed. This in particular causes issues when compression is enabled. On rotate it just overwrites the data in the log file corrupting it. - Use O_TRUNC to open new gzip files to ensure we don't corrupt log files as happens without the above fix. Signed-off-by: Brian Goff Upstream-commit: e87e9e6ad6ba501cc42a2ef47ac18c88a68f258f Component: engine --- .../daemon/logger/loggerutils/logfile.go | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/components/engine/daemon/logger/loggerutils/logfile.go b/components/engine/daemon/logger/loggerutils/logfile.go index b4148ce645..6e3cda8648 100644 --- a/components/engine/daemon/logger/loggerutils/logfile.go +++ b/components/engine/daemon/logger/loggerutils/logfile.go @@ -97,7 +97,7 @@ type LogFile struct { type makeDecoderFunc func(rdr io.Reader) func() (*logger.Message, error) -//NewLogFile creates new LogFile +// NewLogFile creates new LogFile func NewLogFile(logPath string, capacity int64, maxFiles int, compress bool, marshaller logger.MarshalFunc, decodeFunc makeDecoderFunc, perms os.FileMode) (*LogFile, error) { log, err := os.OpenFile(logPath, os.O_WRONLY|os.O_APPEND|os.O_CREATE, perms) if err != nil { @@ -201,6 +201,13 @@ func rotate(name string, maxFiles int, compress bool) error { if compress { extension = ".gz" } + + lastFile := fmt.Sprintf("%s.%d%s", name, maxFiles-1, extension) + err := os.Remove(lastFile) + if err != nil && !os.IsNotExist(err) { + return errors.Wrap(err, "error removing oldest log file") + } + for i := maxFiles - 1; i > 1; i-- { toPath := name + "." + strconv.Itoa(i) + extension fromPath := name + "." + strconv.Itoa(i-1) + extension @@ -230,7 +237,7 @@ func compressFile(fileName string, lastTimestamp time.Time) { } }() - outFile, err := os.OpenFile(fileName+".gz", os.O_CREATE|os.O_RDWR, 0640) + outFile, err := os.OpenFile(fileName+".gz", os.O_CREATE|os.O_TRUNC|os.O_RDWR, 0640) if err != nil { logrus.Errorf("Failed to open or create gzip log file: %v", err) return @@ -251,7 +258,7 @@ func compressFile(fileName string, lastTimestamp time.Time) { compressWriter.Header.Extra, err = json.Marshal(&extra) if err != nil { // Here log the error only and don't return since this is just an optimization. - logrus.Warningf("Failed to marshal JSON: %v", err) + logrus.Warningf("Failed to marshal gzip header as JSON: %v", err) } _, err = pools.Copy(compressWriter, file) @@ -281,6 +288,9 @@ func (w *LogFile) Close() error { } // ReadLogs decodes entries from log files and sends them the passed in watcher +// +// Note: Using the follow option can become inconsistent in cases with very frequent rotations and max log files is 1. +// TODO: Consider a different implementation which can effectively follow logs under frequent rotations. func (w *LogFile) ReadLogs(config logger.ReadConfig, watcher *logger.LogWatcher) { w.mu.RLock() currentFile, err := os.Open(w.f.Name()) @@ -364,7 +374,7 @@ func (w *LogFile) openRotatedFiles(config logger.ReadConfig) (files []*os.File, f, err := os.Open(fmt.Sprintf("%s.%d", w.f.Name(), i-1)) if err != nil { if !os.IsNotExist(err) { - return nil, err + return nil, errors.Wrap(err, "error opening rotated log file") } fileName := fmt.Sprintf("%s.%d.gz", w.f.Name(), i-1) @@ -377,8 +387,8 @@ func (w *LogFile) openRotatedFiles(config logger.ReadConfig) (files []*os.File, }) if err != nil { - if !os.IsNotExist(err) { - return nil, err + if !os.IsNotExist(errors.Cause(err)) { + return nil, errors.Wrap(err, "error getting reference to decompressed log file") } continue } @@ -399,13 +409,13 @@ func (w *LogFile) openRotatedFiles(config logger.ReadConfig) (files []*os.File, func decompressfile(fileName, destFileName string, since time.Time) (*os.File, error) { cf, err := os.Open(fileName) if err != nil { - return nil, err + return nil, errors.Wrap(err, "error opening file for decompression") } defer cf.Close() rc, err := gzip.NewReader(cf) if err != nil { - return nil, err + return nil, errors.Wrap(err, "error making gzip reader for compressed log file") } defer rc.Close() @@ -418,17 +428,17 @@ func decompressfile(fileName, destFileName string, since time.Time) (*os.File, e rs, err := os.OpenFile(destFileName, os.O_CREATE|os.O_RDWR, 0640) if err != nil { - return nil, err + return nil, errors.Wrap(err, "error creating file for copying decompressed log stream") } _, err = pools.Copy(rs, rc) if err != nil { rs.Close() rErr := os.Remove(rs.Name()) - if rErr != nil && os.IsNotExist(rErr) { + if rErr != nil && !os.IsNotExist(rErr) { logrus.Errorf("Failed to remove the logfile %q: %v", rs.Name(), rErr) } - return nil, err + return nil, errors.Wrap(err, "error while copying decompressed log stream to file") } return rs, nil @@ -461,7 +471,7 @@ func tailFile(f io.ReadSeeker, watcher *logger.LogWatcher, createDecoder makeDec for { msg, err := decodeLogLine() if err != nil { - if err != io.EOF { + if errors.Cause(err) != io.EOF { watcher.Err <- err } return @@ -569,7 +579,7 @@ func followLogs(f *os.File, logWatcher *logger.LogWatcher, notifyRotate chan int } handleDecodeErr := func(err error) error { - if err != io.EOF { + if errors.Cause(err) != io.EOF { return err } From 9488201604fa30822a60f50c7213219acf9720c5 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 9 May 2018 17:20:11 -0400 Subject: [PATCH 09/10] Fix logging plugin crash unrecoverable In cases where a logging plugin has crashed when the daemon tries to copy the container stdio to the logging plugin it returns a broken pipe error and any log entries that occurr while the plugin is down are lost. Fix this by opening read+write in the daemon so logs are not lost while the plugin is down. Signed-off-by: Brian Goff Upstream-commit: e7479e3ab8128f9e84cc640f0bed4e77b268a6e9 Component: engine --- .../engine/daemon/logger/plugin_unix.go | 5 +- .../integration/internal/container/ops.go | 18 ++++ .../plugin/logging/cmd/close_on_start/main.go | 48 +++++++++++ .../logging/cmd/close_on_start/main_test.go | 1 + .../plugin/logging/logging_test.go | 82 +++++++++++++++++++ 5 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 components/engine/integration/plugin/logging/cmd/close_on_start/main.go create mode 100644 components/engine/integration/plugin/logging/cmd/close_on_start/main_test.go create mode 100644 components/engine/integration/plugin/logging/logging_test.go diff --git a/components/engine/daemon/logger/plugin_unix.go b/components/engine/daemon/logger/plugin_unix.go index 6192f52eb8..e9a16af9b1 100644 --- a/components/engine/daemon/logger/plugin_unix.go +++ b/components/engine/daemon/logger/plugin_unix.go @@ -12,7 +12,10 @@ import ( ) func openPluginStream(a *pluginAdapter) (io.WriteCloser, error) { - f, err := fifo.OpenFifo(context.Background(), a.fifoPath, unix.O_WRONLY|unix.O_CREAT|unix.O_NONBLOCK, 0700) + // Make sure to also open with read (in addition to write) to avoid borken pipe errors on plugin failure. + // It is up to the plugin to keep track of pipes that it should re-attach to, however. + // If the plugin doesn't open for reads, then the container will block once the pipe is full. + f, err := fifo.OpenFifo(context.Background(), a.fifoPath, unix.O_RDWR|unix.O_CREAT|unix.O_NONBLOCK, 0700) if err != nil { return nil, errors.Wrapf(err, "error creating i/o pipe for log plugin: %s", a.Name()) } diff --git a/components/engine/integration/internal/container/ops.go b/components/engine/integration/internal/container/ops.go index b7d1a7bda9..df5598b62f 100644 --- a/components/engine/integration/internal/container/ops.go +++ b/components/engine/integration/internal/container/ops.go @@ -116,3 +116,21 @@ func WithIPv6(network, ip string) func(*TestContainerConfig) { c.NetworkingConfig.EndpointsConfig[network].IPAMConfig.IPv6Address = ip } } + +// WithLogDriver sets the log driver to use for the container +func WithLogDriver(driver string) func(*TestContainerConfig) { + return func(c *TestContainerConfig) { + if c.HostConfig == nil { + c.HostConfig = &containertypes.HostConfig{} + } + c.HostConfig.LogConfig.Type = driver + } +} + +// WithAutoRemove sets the container to be removed on exit +func WithAutoRemove(c *TestContainerConfig) { + if c.HostConfig == nil { + c.HostConfig = &containertypes.HostConfig{} + } + c.HostConfig.AutoRemove = true +} diff --git a/components/engine/integration/plugin/logging/cmd/close_on_start/main.go b/components/engine/integration/plugin/logging/cmd/close_on_start/main.go new file mode 100644 index 0000000000..6891d6a995 --- /dev/null +++ b/components/engine/integration/plugin/logging/cmd/close_on_start/main.go @@ -0,0 +1,48 @@ +package main + +import ( + "encoding/json" + "fmt" + "net" + "net/http" + "os" +) + +type start struct { + File string +} + +func main() { + l, err := net.Listen("unix", "/run/docker/plugins/plugin.sock") + if err != nil { + panic(err) + } + + mux := http.NewServeMux() + mux.HandleFunc("/LogDriver.StartLogging", func(w http.ResponseWriter, req *http.Request) { + startReq := &start{} + if err := json.NewDecoder(req.Body).Decode(startReq); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + f, err := os.OpenFile(startReq.File, os.O_RDONLY, 0600) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + // Close the file immediately, this allows us to test what happens in the daemon when the plugin has closed the + // file or, for example, the plugin has crashed. + f.Close() + + w.WriteHeader(http.StatusOK) + fmt.Fprintln(w, `{}`) + }) + server := http.Server{ + Addr: l.Addr().String(), + Handler: mux, + } + + server.Serve(l) +} diff --git a/components/engine/integration/plugin/logging/cmd/close_on_start/main_test.go b/components/engine/integration/plugin/logging/cmd/close_on_start/main_test.go new file mode 100644 index 0000000000..06ab7d0f9a --- /dev/null +++ b/components/engine/integration/plugin/logging/cmd/close_on_start/main_test.go @@ -0,0 +1 @@ +package main diff --git a/components/engine/integration/plugin/logging/logging_test.go b/components/engine/integration/plugin/logging/logging_test.go new file mode 100644 index 0000000000..7c118caa29 --- /dev/null +++ b/components/engine/integration/plugin/logging/logging_test.go @@ -0,0 +1,82 @@ +package logging + +import ( + "bufio" + "context" + "os" + "strings" + "testing" + "time" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/volume" + "github.com/docker/docker/integration/internal/container" + "github.com/docker/docker/internal/test/daemon" + "github.com/gotestyourself/gotestyourself/assert" +) + +func TestContinueAfterPluginCrash(t *testing.T) { + t.Parallel() + + d := daemon.New(t) + d.StartWithBusybox(t, "--iptables=false", "--init") + defer d.Stop(t) + + client := d.NewClientT(t) + createPlugin(t, client, "test", "close_on_start", asLogDriver) + + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + assert.Assert(t, client.PluginEnable(ctx, "test", types.PluginEnableOptions{Timeout: 30})) + cancel() + defer client.PluginRemove(context.Background(), "test", types.PluginRemoveOptions{Force: true}) + + v, err := client.VolumeCreate(context.Background(), volume.VolumesCreateBody{}) + assert.Assert(t, err) + defer client.VolumeRemove(context.Background(), v.Name, true) + + ctx, cancel = context.WithTimeout(context.Background(), 60*time.Second) + + id := container.Run(t, ctx, client, + container.WithAutoRemove, + container.WithLogDriver("test"), + container.WithCmd( + "/bin/sh", "-c", "while true; do sleep 1; echo hello; done", + ), + ) + cancel() + defer client.ContainerRemove(context.Background(), id, types.ContainerRemoveOptions{Force: true}) + + // Attach to the container to make sure it's written a few times to stdout + attach, err := client.ContainerAttach(context.Background(), id, types.ContainerAttachOptions{Stream: true, Stdout: true}) + assert.Assert(t, err) + + chErr := make(chan error) + go func() { + defer close(chErr) + rdr := bufio.NewReader(attach.Reader) + for i := 0; i < 5; i++ { + _, _, err := rdr.ReadLine() + if err != nil { + chErr <- err + return + } + } + }() + + select { + case err := <-chErr: + assert.Assert(t, err) + case <-time.After(60 * time.Second): + t.Fatal("timeout waiting for container i/o") + } + + // check daemon logs for "broken pipe" + // TODO(@cpuguy83): This is horribly hacky but is the only way to really test this case right now. + // It would be nice if there was a way to know that a broken pipe has occurred without looking through the logs. + log, err := os.Open(d.LogFileName()) + assert.Assert(t, err) + scanner := bufio.NewScanner(log) + for scanner.Scan() { + assert.Assert(t, !strings.Contains(scanner.Text(), "broken pipe")) + } +} From c4764c184a9da390a730a5dd1df54993d088d940 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Tue, 15 May 2018 15:01:03 +0200 Subject: [PATCH 10/10] Fix logging test type Signed-off-by: Vincent Demeester Upstream-commit: 25494e4c74f31551ef06e6e2df4e1119cd83250e Component: engine --- components/engine/integration/plugin/logging/logging_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/components/engine/integration/plugin/logging/logging_test.go b/components/engine/integration/plugin/logging/logging_test.go index 7c118caa29..ad2336fc64 100644 --- a/components/engine/integration/plugin/logging/logging_test.go +++ b/components/engine/integration/plugin/logging/logging_test.go @@ -9,7 +9,6 @@ import ( "time" "github.com/docker/docker/api/types" - "github.com/docker/docker/api/types/volume" "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/internal/test/daemon" "github.com/gotestyourself/gotestyourself/assert" @@ -30,10 +29,6 @@ func TestContinueAfterPluginCrash(t *testing.T) { cancel() defer client.PluginRemove(context.Background(), "test", types.PluginRemoveOptions{Force: true}) - v, err := client.VolumeCreate(context.Background(), volume.VolumesCreateBody{}) - assert.Assert(t, err) - defer client.VolumeRemove(context.Background(), v.Name, true) - ctx, cancel = context.WithTimeout(context.Background(), 60*time.Second) id := container.Run(t, ctx, client,