From c0a75ea18d7e438b123c3a7967bac35a15eb7605 Mon Sep 17 00:00:00 2001 From: Drew Erny Date: Wed, 18 Oct 2017 14:41:59 -0700 Subject: [PATCH 1/2] Output network attachment task information Adds functionality to parse and return network attachment spec information. Network attachment tasks are phony tasks created in swarmkit to deal with unmanaged containers attached to swarmkit. Before this change, attempting `docker inspect` on the task id of a network attachment task would result in an empty task object. After this change, a full task object is returned Fixes #26548 the correct way. Signed-off-by: Drew Erny Signed-off-by: Sebastiaan van Stijn Upstream-commit: 5b69ff466e61fa168f24869710f2070c742a5565 Component: engine --- components/engine/api/swagger.yaml | 7 ++ components/engine/api/types/swarm/runtime.go | 8 ++ components/engine/api/types/swarm/task.go | 11 ++- .../engine/daemon/cluster/convert/service.go | 38 ++++++++-- .../daemon/cluster/convert/service_test.go | 74 +++++++++++++++++++ .../engine/daemon/cluster/convert/task.go | 3 - components/engine/daemon/cluster/services.go | 4 + 7 files changed, 132 insertions(+), 13 deletions(-) diff --git a/components/engine/api/swagger.yaml b/components/engine/api/swagger.yaml index 459ce9b186..c0b442416d 100644 --- a/components/engine/api/swagger.yaml +++ b/components/engine/api/swagger.yaml @@ -2688,6 +2688,13 @@ definitions: - "default" - "process" - "hyperv" + NetworkAttachmentSpec: + description: "Read-only spec type for non-swarm containers attached to swarm overlay networks" + type: "object" + properties: + ContainerID: + description: "ID of the container represented by this task" + type: "string" Resources: description: "Resource requirements which apply to each individual container created as part of the service." type: "object" diff --git a/components/engine/api/types/swarm/runtime.go b/components/engine/api/types/swarm/runtime.go index 81b5f4cfd9..0c77403ccf 100644 --- a/components/engine/api/types/swarm/runtime.go +++ b/components/engine/api/types/swarm/runtime.go @@ -11,9 +11,17 @@ const ( RuntimeContainer RuntimeType = "container" // RuntimePlugin is the plugin based runtime RuntimePlugin RuntimeType = "plugin" + // RuntimeNetworkAttachment is the network attachment runtime + RuntimeNetworkAttachment RuntimeType = "attachment" // RuntimeURLContainer is the proto url for the container type RuntimeURLContainer RuntimeURL = "types.docker.com/RuntimeContainer" // RuntimeURLPlugin is the proto url for the plugin type RuntimeURLPlugin RuntimeURL = "types.docker.com/RuntimePlugin" ) + +// NetworkAttachmentSpec represents the runtime spec type for network +// attachment tasks +type NetworkAttachmentSpec struct { + ContainerID string +} diff --git a/components/engine/api/types/swarm/task.go b/components/engine/api/types/swarm/task.go index 5c2bc492e4..b35605d12f 100644 --- a/components/engine/api/types/swarm/task.go +++ b/components/engine/api/types/swarm/task.go @@ -60,10 +60,13 @@ type Task struct { // TaskSpec represents the spec of a task. type TaskSpec struct { - // ContainerSpec and PluginSpec are mutually exclusive. - // PluginSpec will only be used when the `Runtime` field is set to `plugin` - ContainerSpec *ContainerSpec `json:",omitempty"` - PluginSpec *runtime.PluginSpec `json:",omitempty"` + // ContainerSpec, NetworkAttachmentSpec, and PluginSpec are mutually exclusive. + // PluginSpec is only used when the `Runtime` field is set to `plugin` + // NetworkAttachmentSpec is used if the `Runtime` field is set to + // `attachment`. + ContainerSpec *ContainerSpec `json:",omitempty"` + PluginSpec *runtime.PluginSpec `json:",omitempty"` + NetworkAttachmentSpec *NetworkAttachmentSpec `json:",omitempty"` Resources *ResourceRequirements `json:",omitempty"` RestartPolicy *RestartPolicy `json:",omitempty"` diff --git a/components/engine/daemon/cluster/convert/service.go b/components/engine/daemon/cluster/convert/service.go index 7aa34fa77f..5a1609aa01 100644 --- a/components/engine/daemon/cluster/convert/service.go +++ b/components/engine/daemon/cluster/convert/service.go @@ -17,6 +17,8 @@ import ( var ( // ErrUnsupportedRuntime returns an error if the runtime is not supported by the daemon ErrUnsupportedRuntime = errors.New("unsupported runtime") + // ErrMismatchedRuntime returns an error if the runtime does not match the provided spec + ErrMismatchedRuntime = errors.New("mismatched Runtime and *Spec fields") ) // ServiceFromGRPC converts a grpc Service to a Service. @@ -176,15 +178,18 @@ func ServiceSpecToGRPC(s types.ServiceSpec) (swarmapi.ServiceSpec, error) { return swarmapi.ServiceSpec{}, err } spec.Task.Runtime = &swarmapi.TaskSpec_Container{Container: containerSpec} + } else { + // If the ContainerSpec is nil, we can't set the task runtime + return swarmapi.ServiceSpec{}, ErrMismatchedRuntime } case types.RuntimePlugin: - if s.Mode.Replicated != nil { - return swarmapi.ServiceSpec{}, errors.New("plugins must not use replicated mode") - } - - s.Mode.Global = &types.GlobalService{} // must always be global - if s.TaskTemplate.PluginSpec != nil { + if s.Mode.Replicated != nil { + return swarmapi.ServiceSpec{}, errors.New("plugins must not use replicated mode") + } + + s.Mode.Global = &types.GlobalService{} // must always be global + pluginSpec, err := proto.Marshal(s.TaskTemplate.PluginSpec) if err != nil { return swarmapi.ServiceSpec{}, err @@ -198,7 +203,16 @@ func ServiceSpecToGRPC(s types.ServiceSpec) (swarmapi.ServiceSpec, error) { }, }, } + } else { + return swarmapi.ServiceSpec{}, ErrMismatchedRuntime } + case types.RuntimeNetworkAttachment: + // NOTE(dperny) I'm leaving this case here for completeness. The actual + // code is left out out deliberately, as we should refuse to parse a + // Network Attachment runtime; it will cause weird behavior all over + // the system if we do. Instead, fallthrough and return + // ErrUnsupportedRuntime if we get one. + fallthrough default: return swarmapi.ServiceSpec{}, ErrUnsupportedRuntime } @@ -573,6 +587,12 @@ func updateConfigToGRPC(updateConfig *types.UpdateConfig) (*swarmapi.UpdateConfi return converted, nil } +func networkAttachmentSpecFromGRPC(attachment swarmapi.NetworkAttachmentSpec) *types.NetworkAttachmentSpec { + return &types.NetworkAttachmentSpec{ + ContainerID: attachment.ContainerID, + } +} + func taskSpecFromGRPC(taskSpec swarmapi.TaskSpec) (types.TaskSpec, error) { taskNetworks := make([]types.NetworkAttachmentConfig, 0, len(taskSpec.Networks)) for _, n := range taskSpec.Networks { @@ -607,6 +627,12 @@ func taskSpecFromGRPC(taskSpec swarmapi.TaskSpec) (types.TaskSpec, error) { t.PluginSpec = &p } } + case *swarmapi.TaskSpec_Attachment: + a := taskSpec.GetAttachment() + if a != nil { + t.NetworkAttachmentSpec = networkAttachmentSpecFromGRPC(*a) + } + t.Runtime = types.RuntimeNetworkAttachment } return t, nil diff --git a/components/engine/daemon/cluster/convert/service_test.go b/components/engine/daemon/cluster/convert/service_test.go index 0794af99a6..826cf6fbef 100644 --- a/components/engine/daemon/cluster/convert/service_test.go +++ b/components/engine/daemon/cluster/convert/service_test.go @@ -232,3 +232,77 @@ func TestServiceConvertFromGRPCIsolation(t *testing.T) { }) } } + +func TestServiceConvertToGRPCNetworkAtachmentRuntime(t *testing.T) { + someid := "asfjkl" + s := swarmtypes.ServiceSpec{ + TaskTemplate: swarmtypes.TaskSpec{ + Runtime: swarmtypes.RuntimeNetworkAttachment, + NetworkAttachmentSpec: &swarmtypes.NetworkAttachmentSpec{ + ContainerID: someid, + }, + }, + } + + // discard the service, which will be empty + _, err := ServiceSpecToGRPC(s) + if err == nil { + t.Fatalf("expected error %v but got no error", ErrUnsupportedRuntime) + } + if err != ErrUnsupportedRuntime { + t.Fatalf("expected error %v but got error %v", ErrUnsupportedRuntime, err) + } +} + +func TestServiceConvertToGRPCMismatchedRuntime(t *testing.T) { + // NOTE(dperny): an earlier version of this test was for code that also + // converted network attachment tasks to GRPC. that conversion code was + // removed, so if this loop body seems a bit complicated, that's why. + for i, rt := range []swarmtypes.RuntimeType{ + swarmtypes.RuntimeContainer, + swarmtypes.RuntimePlugin, + } { + for j, spec := range []swarmtypes.TaskSpec{ + {ContainerSpec: &swarmtypes.ContainerSpec{}}, + {PluginSpec: &runtime.PluginSpec{}}, + } { + // skip the cases, where the indices match, which would not error + if i == j { + continue + } + // set the task spec, then change the runtime + s := swarmtypes.ServiceSpec{ + TaskTemplate: spec, + } + s.TaskTemplate.Runtime = rt + + if _, err := ServiceSpecToGRPC(s); err != ErrMismatchedRuntime { + t.Fatalf("expected %v got %v", ErrMismatchedRuntime, err) + } + } + } +} + +func TestTaskConvertFromGRPCNetworkAttachment(t *testing.T) { + containerID := "asdfjkl" + s := swarmapi.TaskSpec{ + Runtime: &swarmapi.TaskSpec_Attachment{ + Attachment: &swarmapi.NetworkAttachmentSpec{ + ContainerID: containerID, + }, + }, + } + ts, err := taskSpecFromGRPC(s) + if err != nil { + t.Fatal(err) + } + if ts.NetworkAttachmentSpec == nil { + t.Fatal("expected task spec to have network attachment spec") + } + if ts.NetworkAttachmentSpec.ContainerID != containerID { + t.Fatalf("expected network attachment spec container id to be %q, was %q", containerID, ts.NetworkAttachmentSpec.ContainerID) + } + if ts.Runtime != swarmtypes.RuntimeNetworkAttachment { + t.Fatalf("expected Runtime to be %v", swarmtypes.RuntimeNetworkAttachment) + } +} diff --git a/components/engine/daemon/cluster/convert/task.go b/components/engine/daemon/cluster/convert/task.go index dbe6d7414d..72e2805e1e 100644 --- a/components/engine/daemon/cluster/convert/task.go +++ b/components/engine/daemon/cluster/convert/task.go @@ -10,9 +10,6 @@ import ( // TaskFromGRPC converts a grpc Task to a Task. func TaskFromGRPC(t swarmapi.Task) (types.Task, error) { - if t.Spec.GetAttachment() != nil { - return types.Task{}, nil - } containerStatus := t.Status.GetContainer() taskSpec, err := taskSpecFromGRPC(t.Spec) if err != nil { diff --git a/components/engine/daemon/cluster/services.go b/components/engine/daemon/cluster/services.go index ed438e93a0..c14037645c 100644 --- a/components/engine/daemon/cluster/services.go +++ b/components/engine/daemon/cluster/services.go @@ -135,6 +135,8 @@ func (c *Cluster) CreateService(s types.ServiceSpec, encodedAuth string, queryRe resp = &apitypes.ServiceCreateResponse{} switch serviceSpec.Task.Runtime.(type) { + case *swarmapi.TaskSpec_Attachment: + return fmt.Errorf("invalid task spec: spec type %q not supported", types.RuntimeNetworkAttachment) // handle other runtimes here case *swarmapi.TaskSpec_Generic: switch serviceSpec.Task.GetGeneric().Kind { @@ -244,6 +246,8 @@ func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec typ resp = &apitypes.ServiceUpdateResponse{} switch serviceSpec.Task.Runtime.(type) { + case *swarmapi.TaskSpec_Attachment: + return fmt.Errorf("invalid task spec: spec type %q not supported", types.RuntimeNetworkAttachment) case *swarmapi.TaskSpec_Generic: switch serviceSpec.Task.GetGeneric().Kind { case string(types.RuntimePlugin): From 53104fbd7ccdff2f5549a4087bac5335f8408ee6 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 22 May 2018 23:37:33 +0200 Subject: [PATCH 2/2] Update swagger and API history Signed-off-by: Sebastiaan van Stijn Upstream-commit: 3682703ad4a75ea489aa84cbaddcfa4697a4e57e Component: engine --- components/engine/api/swagger.yaml | 31 +++++++++++++++++-- components/engine/docs/api/version-history.md | 6 ++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/components/engine/api/swagger.yaml b/components/engine/api/swagger.yaml index c0b442416d..3c0c272e5c 100644 --- a/components/engine/api/swagger.yaml +++ b/components/engine/api/swagger.yaml @@ -2449,7 +2449,15 @@ definitions: properties: PluginSpec: type: "object" - description: "Invalid when specified with `ContainerSpec`. *(Experimental release only.)*" + description: | + Plugin spec for the service. *(Experimental release only.)* + +


+ + > **Note**: ContainerSpec, NetworkAttachmentSpec, and PluginSpec are + > mutually exclusive. PluginSpec is only used when the Runtime field + > is set to `plugin`. NetworkAttachmentSpec is used when the Runtime + > field is set to `attachment`. properties: Name: description: "The name or 'alias' to use for the plugin." @@ -2476,7 +2484,15 @@ definitions: type: "string" ContainerSpec: type: "object" - description: "Invalid when specified with `PluginSpec`." + description: | + Container spec for the service. + +


+ + > **Note**: ContainerSpec, NetworkAttachmentSpec, and PluginSpec are + > mutually exclusive. PluginSpec is only used when the Runtime field + > is set to `plugin`. NetworkAttachmentSpec is used when the Runtime + > field is set to `attachment`. properties: Image: description: "The image name to use for the container" @@ -2689,7 +2705,16 @@ definitions: - "process" - "hyperv" NetworkAttachmentSpec: - description: "Read-only spec type for non-swarm containers attached to swarm overlay networks" + description: | + Read-only spec type for non-swarm containers attached to swarm overlay + networks. + +


+ + > **Note**: ContainerSpec, NetworkAttachmentSpec, and PluginSpec are + > mutually exclusive. PluginSpec is only used when the Runtime field + > is set to `plugin`. NetworkAttachmentSpec is used when the Runtime + > field is set to `attachment`. type: "object" properties: ContainerID: diff --git a/components/engine/docs/api/version-history.md b/components/engine/docs/api/version-history.md index 61245e14a7..8c56a1c19f 100644 --- a/components/engine/docs/api/version-history.md +++ b/components/engine/docs/api/version-history.md @@ -13,6 +13,12 @@ keywords: "API, Docker, rcli, REST, documentation" will be rejected. --> +## V1.38 API changes + +* `GET /tasks` and `GET /tasks/{id}` now return a `NetworkAttachmentSpec` field, + containing the `ContainerID` for non-service containers connected to "attachable" + swarm-scoped networks. + ## v1.37 API changes [Docker Engine API v1.37](https://docs.docker.com/engine/api/v1.37/) documentation