Skip to content

Services: use ServiceStatus on API v1.41 and up#2157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
thaJeztah merged 4 commits into from
Oct 29, 2019
Merged

Services: use ServiceStatus on API v1.41 and up #2157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
962015b
internal/builders: add GlobalService, ServiceStatus, NodeList()
thaJeztah Oct 28, 2019
228e0f5
TestStackServicesErrors: use sub-tests, and return tasks
thaJeztah Oct 28, 2019
7405ac5
Services: use ServiceStatus on API v1.41 and up
thaJeztah Oct 23, 2019
20424e2
Add test for ServiceStatus
thaJeztah Oct 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions cli/command/service/client_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,7 @@ import (
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/swarm"
"github.com/docker/docker/client"

// Import builders to get the builder function as package function
. "github.com/docker/cli/internal/test/builders"
)
Expand All@@ -18,9 +19,13 @@ type fakeClient struct {
taskListFunc func(context.Context, types.TaskListOptions) ([]swarm.Task, error)
infoFunc func(ctx context.Context) (types.Info, error)
networkInspectFunc func(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, error)
nodeListFunc func(ctx context.Context, options types.NodeListOptions) ([]swarm.Node, error)
}

func (f *fakeClient) NodeList(ctx context.Context, options types.NodeListOptions) ([]swarm.Node, error) {
if f.nodeListFunc != nil {
return f.nodeListFunc(ctx, options)
}
return nil, nil
}

Expand DownExpand Up@@ -69,9 +74,6 @@ func (f *fakeClient) NetworkInspect(ctx context.Context, networkID string, optio
return types.NetworkResource{}, nil
}

func newService(id string, name string) swarm.Service {
return swarm.Service{
ID: id,
Spec: swarm.ServiceSpec{Annotations: swarm.Annotations{Name: name}},
}
func newService(id string, name string, opts ...func(*swarm.Service)) swarm.Service {
return *Service(append(opts, ServiceID(id), ServiceName(name))...)
}
46 changes: 33 additions & 13 deletions cli/command/service/formatter.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@ import (
"github.com/docker/docker/pkg/stringid"
units "github.com/docker/go-units"
"github.com/pkg/errors"
"vbom.ml/util/sortorder"
)

const serviceInspectPrettyTemplate formatter.Format = `
Expand DownExpand Up@@ -520,17 +521,14 @@ func NewListFormat(source string, quiet bool) formatter.Format {
return formatter.Format(source)
}

// ListInfo stores the information about mode and replicas to be used by template
type ListInfo struct {
Mode string
Replicas string
}

// ListFormatWrite writes the context
func ListFormatWrite(ctx formatter.Context, services []swarm.Service, info map[string]ListInfo) error {
func ListFormatWrite(ctx formatter.Context, services []swarm.Service) error {
render := func(format func(subContext formatter.SubContext) error) error {
sort.Slice(services, func(i, j int) bool {
return sortorder.NaturalLess(services[i].Spec.Name, services[j].Spec.Name)
})
for _, service := range services {
serviceCtx := &serviceContext{service: service, mode: info[service.ID].Mode, replicas: info[service.ID].Replicas}
serviceCtx := &serviceContext{service: service}
if err := format(serviceCtx); err != nil {
return err
}
Expand All@@ -551,9 +549,7 @@ func ListFormatWrite(ctx formatter.Context, services []swarm.Service, info map[s

type serviceContext struct {
formatter.HeaderContext
service swarm.Service
mode string
replicas string
service swarm.Service
}

func (c *serviceContext) MarshalJSON() ([]byte, error) {
Expand All@@ -569,11 +565,35 @@ func (c *serviceContext) Name() string {
}

func (c *serviceContext) Mode() string {
return c.mode
switch {
case c.service.Spec.Mode.Global != nil:
return "global"
case c.service.Spec.Mode.Replicated != nil:
return "replicated"
default:
return ""
}
}

func (c *serviceContext) Replicas() string {
return c.replicas
s := &c.service

var running, desired uint64
if s.ServiceStatus != nil {
running = c.service.ServiceStatus.RunningTasks
desired = c.service.ServiceStatus.DesiredTasks
}
if r := c.maxReplicas(); r > 0 {
return fmt.Sprintf("%d/%d (max %d per node)", running, desired, r)
}
return fmt.Sprintf("%d/%d", running, desired)
}

func (c *serviceContext) maxReplicas() uint64 {

@thaJeztahthaJeztahOct 26, 2019

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I want to export this one, so that it can be used in --format, but keeping that separate to reduce the changes a bit

if c.Mode() != "replicated" || c.service.Spec.TaskTemplate.Placement == nil {
return 0
}
return c.service.Spec.TaskTemplate.Placement.MaxReplicas
}

func (c *serviceContext) Image() string {
Expand Down
172 changes: 119 additions & 53 deletions cli/command/service/formatter_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -33,29 +33,37 @@ func TestServiceContextWrite(t *testing.T) {
// Table format
{
formatter.Context{Format: NewListFormat("table", false)},
`ID NAME MODE REPLICAS IMAGE PORTS
id_baz baz global 2/4 *:80->8080/tcp
id_bar bar replicated 2/4 *:80->8080/tcp
`ID NAME MODE REPLICAS IMAGE PORTS
02_bar bar replicated 2/4 *:80->8090/udp

@silvin-lubeckisilvin-lubeckiOct 28, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be sorted by service name? 🤔

@silvin-lubeckisilvin-lubeckiOct 28, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok just saw your comment below 👍

01_baz baz global 1/3 *:80->8080/tcp

@thaJeztahthaJeztahOct 26, 2019

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

changed the replica-count to be unique for each service, to verify we're showing the correct status

04_qux2 qux2 replicated 3/3 (max 2 per node)
03_qux10 qux10 replicated 2/3 (max 1 per node)

@thaJeztahthaJeztahOct 26, 2019

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Added test-cases to test the max X per node output

`,
},
{
formatter.Context{Format: NewListFormat("table", true)},
`id_baz
id_bar
`02_bar
01_baz
04_qux2
03_qux10
`,
},
{
formatter.Context{Format: NewListFormat("table {{.Name}}", false)},
`NAME
baz
bar
formatter.Context{Format: NewListFormat("table {{.Name}}\t{{.Mode}}", false)},
`NAME MODE
bar replicated
baz global
qux2 replicated
qux10 replicated

@thaJeztahthaJeztahOct 26, 2019

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Sorting moved to the formatter (see the PR description), which is why these tests now have their output in a different order. I added services named qux2 and qux10 to verify the formatter uses naturalsort (otherwise qux2 would come after qux10)

`,
},
{
formatter.Context{Format: NewListFormat("table {{.Name}}", true)},
`NAME
baz
bar
baz
qux2
qux10
`,
},
// Raw Format
Expand All@@ -65,25 +73,32 @@ bar
},
{
formatter.Context{Format: NewListFormat("raw", true)},
`id: id_baz
id: id_bar
`id: 02_bar
id: 01_baz
id: 04_qux2
id: 03_qux10
`,
},
// Custom Format
{
formatter.Context{Format: NewListFormat("{{.Name}}", false)},
`baz
bar
`bar
baz
qux2
qux10
`,
},
}

for _, testcase := range cases {
services := []swarm.Service{
{
ID: "id_baz",
ID: "01_baz",

@thaJeztahthaJeztahOct 26, 2019

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

prefixing the ID's with numbers, so that we verify the list is sorted by the service name not the service ID

Spec: swarm.ServiceSpec{
Annotations: swarm.Annotations{Name: "baz"},
Mode: swarm.ServiceMode{
Global: &swarm.GlobalService{},
},
},
Endpoint: swarm.Endpoint{
Ports: []swarm.PortConfig{
Expand All@@ -95,37 +110,70 @@ bar
},
},
},
ServiceStatus: &swarm.ServiceStatus{
RunningTasks: 1,
DesiredTasks: 3,
},
},
{
ID: "id_bar",
ID: "02_bar",
Spec: swarm.ServiceSpec{
Annotations: swarm.Annotations{Name: "bar"},
Mode: swarm.ServiceMode{
Replicated: &swarm.ReplicatedService{},
},
},
Endpoint: swarm.Endpoint{
Ports: []swarm.PortConfig{
{
PublishMode: "ingress",
PublishedPort: 80,
TargetPort: 8080,
Protocol: "tcp",
TargetPort: 8090,

@thaJeztahthaJeztahOct 26, 2019•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Not an important change, but multiple services cannot map to the same host-port, so this is just making the test-case more realistic/correct.

Protocol: "udp",
},
},
},
ServiceStatus: &swarm.ServiceStatus{
RunningTasks: 2,
DesiredTasks: 4,
},
},
}
info := map[string]ListInfo{
"id_baz": {
Mode: "global",
Replicas: "2/4",
{
ID: "03_qux10",
Spec: swarm.ServiceSpec{
Annotations: swarm.Annotations{Name: "qux10"},
Mode: swarm.ServiceMode{
Replicated: &swarm.ReplicatedService{},
},
TaskTemplate: swarm.TaskSpec{
Placement: &swarm.Placement{MaxReplicas: 1},
},
},
ServiceStatus: &swarm.ServiceStatus{
RunningTasks: 2,
DesiredTasks: 3,
},
},
"id_bar": {
Mode: "replicated",
Replicas: "2/4",
{
ID: "04_qux2",
Spec: swarm.ServiceSpec{
Annotations: swarm.Annotations{Name: "qux2"},
Mode: swarm.ServiceMode{
Replicated: &swarm.ReplicatedService{},
},
TaskTemplate: swarm.TaskSpec{
Placement: &swarm.Placement{MaxReplicas: 2},
},
},
ServiceStatus: &swarm.ServiceStatus{
RunningTasks: 3,
DesiredTasks: 3,
},
},
}
out := bytes.NewBufferString("")
testcase.context.Output = out
err := ListFormatWrite(testcase.context, services, info)
err := ListFormatWrite(testcase.context, services)
if err != nil {
assert.Error(t, err, testcase.expected)
} else {
Expand All@@ -137,9 +185,12 @@ bar
func TestServiceContextWriteJSON(t *testing.T) {
services := []swarm.Service{
{
ID: "id_baz",
ID: "01_baz",
Spec: swarm.ServiceSpec{
Annotations: swarm.Annotations{Name: "baz"},
Mode: swarm.ServiceMode{
Global: &swarm.GlobalService{},
},
},
Endpoint: swarm.Endpoint{
Ports: []swarm.PortConfig{
Expand All@@ -151,11 +202,18 @@ func TestServiceContextWriteJSON(t *testing.T) {
},
},
},
ServiceStatus: &swarm.ServiceStatus{
RunningTasks: 1,
DesiredTasks: 3,
},
},
{
ID: "id_bar",
ID: "02_bar",
Spec: swarm.ServiceSpec{
Annotations: swarm.Annotations{Name: "bar"},
Mode: swarm.ServiceMode{
Replicated: &swarm.ReplicatedService{},
},
},
Endpoint: swarm.Endpoint{
Ports: []swarm.PortConfig{
Expand All@@ -167,25 +225,19 @@ func TestServiceContextWriteJSON(t *testing.T) {
},
},
},
},
}
info := map[string]ListInfo{
"id_baz": {
Mode: "global",
Replicas: "2/4",
},
"id_bar": {
Mode: "replicated",
Replicas: "2/4",
ServiceStatus: &swarm.ServiceStatus{
RunningTasks: 2,
DesiredTasks: 4,
},
},
}
expectedJSONs := []map[string]interface{}{
{"ID": "id_baz", "Name": "baz", "Mode": "global", "Replicas": "2/4", "Image": "", "Ports": "*:80->8080/tcp"},
{"ID": "id_bar", "Name": "bar", "Mode": "replicated", "Replicas": "2/4", "Image": "", "Ports": "*:80->8080/tcp"},
{"ID": "02_bar", "Name": "bar", "Mode": "replicated", "Replicas": "2/4", "Image": "", "Ports": "*:80->8080/tcp"},
{"ID": "01_baz", "Name": "baz", "Mode": "global", "Replicas": "1/3", "Image": "", "Ports": "*:80->8080/tcp"},
}

out := bytes.NewBufferString("")
err := ListFormatWrite(formatter.Context{Format: "{{json .}}", Output: out}, services, info)
err := ListFormatWrite(formatter.Context{Format: "{{json .}}", Output: out}, services)
if err != nil {
t.Fatal(err)
}
Expand All@@ -199,21 +251,35 @@ func TestServiceContextWriteJSON(t *testing.T) {
}
func TestServiceContextWriteJSONField(t *testing.T) {
services := []swarm.Service{
{ID: "id_baz", Spec: swarm.ServiceSpec{Annotations: swarm.Annotations{Name: "baz"}}},
{ID: "id_bar", Spec: swarm.ServiceSpec{Annotations: swarm.Annotations{Name: "bar"}}},
}
info := map[string]ListInfo{
"id_baz": {
Mode: "global",
Replicas: "2/4",
{
ID: "01_baz",
Spec: swarm.ServiceSpec{
Annotations: swarm.Annotations{Name: "baz"},
Mode: swarm.ServiceMode{
Global: &swarm.GlobalService{},
},
},
ServiceStatus: &swarm.ServiceStatus{
RunningTasks: 2,
DesiredTasks: 4,
},
},
"id_bar": {
Mode: "replicated",
Replicas: "2/4",
{
ID: "24_bar",
Spec: swarm.ServiceSpec{
Annotations: swarm.Annotations{Name: "bar"},
Mode: swarm.ServiceMode{
Replicated: &swarm.ReplicatedService{},
},
},
ServiceStatus: &swarm.ServiceStatus{
RunningTasks: 2,
DesiredTasks: 4,
},
},
}
out := bytes.NewBufferString("")
err := ListFormatWrite(formatter.Context{Format: "{{json .Name}}", Output: out}, services, info)
err := ListFormatWrite(formatter.Context{Format: "{{json .Name}}", Output: out}, services)
if err != nil {
t.Fatal(err)
}
Expand Down
Loading