Skip to content

Commit 697397f

Browse files
authored
feat(log): add context fields to lifecycle events (#1007)
1 parent cd0ec88 commit 697397f

File tree

5 files changed

+191
-140
lines changed

5 files changed

+191
-140
lines changed

pkg/container/client.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,7 @@ func (client dockerClient) RemoveImageByID(id t.ImageID) error {
361361

362362
func (client dockerClient) ExecuteCommand(containerID t.ContainerID, command string, timeout int) (SkipUpdate bool, err error) {
363363
bg := context.Background()
364+
clog := log.WithField("containerID", containerID)
364365

365366
// Create the exec
366367
execConfig := types.ExecConfig{
@@ -379,7 +380,7 @@ func (client dockerClient) ExecuteCommand(containerID t.ContainerID, command str
379380
Detach: false,
380381
})
381382
if attachErr != nil {
382-
log.Errorf("Failed to extract command exec logs: %v", attachErr)
383+
clog.Errorf("Failed to extract command exec logs: %v", attachErr)
383384
}
384385

385386
// Run the exec
@@ -395,7 +396,7 @@ func (client dockerClient) ExecuteCommand(containerID t.ContainerID, command str
395396
var writer bytes.Buffer
396397
written, err := writer.ReadFrom(response.Reader)
397398
if err != nil {
398-
log.Error(err)
399+
clog.Error(err)
399400
} else if written > 0 {
400401
output = strings.TrimSpace(writer.String())
401402
}
@@ -428,9 +429,10 @@ func (client dockerClient) waitForExecOrTimeout(bg context.Context, ID string, e
428429

429430
//goland:noinspection GoNilness
430431
log.WithFields(log.Fields{
431-
"exit-code": execInspect.ExitCode,
432-
"exec-id": execInspect.ExecID,
433-
"running": execInspect.Running,
432+
"exit-code": execInspect.ExitCode,
433+
"exec-id": execInspect.ExecID,
434+
"running": execInspect.Running,
435+
"container-id": execInspect.ContainerID,
434436
}).Debug("Awaiting timeout or completion")
435437

436438
if err != nil {

pkg/container/client_test.go

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
package container
2+
3+
import (
4+
"github.com/containrrr/watchtower/pkg/container/mocks"
5+
"github.com/containrrr/watchtower/pkg/filters"
6+
cli "github.com/docker/docker/client"
7+
. "github.com/onsi/ginkgo"
8+
. "github.com/onsi/gomega"
9+
"github.com/onsi/gomega/gbytes"
10+
"github.com/sirupsen/logrus"
11+
)
12+
13+
var _ = Describe("the client", func() {
14+
var docker *cli.Client
15+
var client Client
16+
BeforeSuite(func() {
17+
server := mocks.NewMockAPIServer()
18+
docker, _ = cli.NewClientWithOpts(
19+
cli.WithHost(server.URL),
20+
cli.WithHTTPClient(server.Client()))
21+
client = dockerClient{
22+
api: docker,
23+
pullImages: false,
24+
}
25+
})
26+
It("should return a client for the api", func() {
27+
Expect(client).NotTo(BeNil())
28+
})
29+
Describe("WarnOnHeadPullFailed", func() {
30+
containerUnknown := *mockContainerWithImageName("unknown.repo/prefix/imagename:latest")
31+
containerKnown := *mockContainerWithImageName("docker.io/prefix/imagename:latest")
32+
33+
When("warn on head failure is set to \"always\"", func() {
34+
c := newClientNoAPI(false, false, false, false, false, "always")
35+
It("should always return true", func() {
36+
Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeTrue())
37+
Expect(c.WarnOnHeadPullFailed(containerKnown)).To(BeTrue())
38+
})
39+
})
40+
When("warn on head failure is set to \"auto\"", func() {
41+
c := newClientNoAPI(false, false, false, false, false, "auto")
42+
It("should always return true", func() {
43+
Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeFalse())
44+
})
45+
It("should", func() {
46+
Expect(c.WarnOnHeadPullFailed(containerKnown)).To(BeTrue())
47+
})
48+
})
49+
When("warn on head failure is set to \"never\"", func() {
50+
c := newClientNoAPI(false, false, false, false, false, "never")
51+
It("should never return true", func() {
52+
Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeFalse())
53+
Expect(c.WarnOnHeadPullFailed(containerKnown)).To(BeFalse())
54+
})
55+
})
56+
})
57+
58+
When("listing containers without any filter", func() {
59+
It("should return all available containers", func() {
60+
containers, err := client.ListContainers(filters.NoFilter)
61+
Expect(err).NotTo(HaveOccurred())
62+
Expect(len(containers) == 2).To(BeTrue())
63+
})
64+
})
65+
When("listing containers with a filter matching nothing", func() {
66+
It("should return an empty array", func() {
67+
filter := filters.FilterByNames([]string{"lollercoaster"}, filters.NoFilter)
68+
containers, err := client.ListContainers(filter)
69+
Expect(err).NotTo(HaveOccurred())
70+
Expect(len(containers) == 0).To(BeTrue())
71+
})
72+
})
73+
When("listing containers with a watchtower filter", func() {
74+
It("should return only the watchtower container", func() {
75+
containers, err := client.ListContainers(filters.WatchtowerContainersFilter)
76+
Expect(err).NotTo(HaveOccurred())
77+
Expect(len(containers) == 1).To(BeTrue())
78+
Expect(containers[0].ImageName()).To(Equal("containrrr/watchtower:latest"))
79+
})
80+
})
81+
When(`listing containers with the "include stopped" option`, func() {
82+
It("should return both stopped and running containers", func() {
83+
client = dockerClient{
84+
api: docker,
85+
pullImages: false,
86+
includeStopped: true,
87+
}
88+
containers, err := client.ListContainers(filters.NoFilter)
89+
Expect(err).NotTo(HaveOccurred())
90+
Expect(len(containers) > 0).To(BeTrue())
91+
})
92+
})
93+
When(`listing containers with the "include restart" option`, func() {
94+
It("should return both stopped, restarting and running containers", func() {
95+
client = dockerClient{
96+
api: docker,
97+
pullImages: false,
98+
includeRestarting: true,
99+
}
100+
containers, err := client.ListContainers(filters.NoFilter)
101+
Expect(err).NotTo(HaveOccurred())
102+
RestartingContainerFound := false
103+
for _, ContainerRunning := range containers {
104+
if ContainerRunning.containerInfo.State.Restarting {
105+
RestartingContainerFound = true
106+
}
107+
}
108+
Expect(RestartingContainerFound).To(BeTrue())
109+
Expect(RestartingContainerFound).NotTo(BeFalse())
110+
})
111+
})
112+
When(`listing containers without restarting ones`, func() {
113+
It("should not return restarting containers", func() {
114+
client = dockerClient{
115+
api: docker,
116+
pullImages: false,
117+
includeRestarting: false,
118+
}
119+
containers, err := client.ListContainers(filters.NoFilter)
120+
Expect(err).NotTo(HaveOccurred())
121+
RestartingContainerFound := false
122+
for _, ContainerRunning := range containers {
123+
if ContainerRunning.containerInfo.State.Restarting {
124+
RestartingContainerFound = true
125+
}
126+
}
127+
Expect(RestartingContainerFound).To(BeFalse())
128+
Expect(RestartingContainerFound).NotTo(BeTrue())
129+
})
130+
})
131+
Describe(`ExecuteCommand`, func() {
132+
When(`logging`, func() {
133+
It("should include container id field", func() {
134+
// Capture logrus output in buffer
135+
logbuf := gbytes.NewBuffer()
136+
origOut := logrus.StandardLogger().Out
137+
defer logrus.SetOutput(origOut)
138+
logrus.SetOutput(logbuf)
139+
140+
_, err := client.ExecuteCommand("ex-cont-id", "exec-cmd", 1)
141+
Expect(err).NotTo(HaveOccurred())
142+
// Note: Since Execute requires opening up a raw TCP stream to the daemon for the output, this will fail
143+
// when using the mock API server. Regardless of the outcome, the log should include the container ID
144+
Eventually(logbuf).Should(gbytes.Say(`containerID="?ex-cont-id"?`))
145+
})
146+
})
147+
})
148+
})

pkg/container/container_test.go

Lines changed: 0 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -1,136 +1,14 @@
11
package container
22

33
import (
4-
"github.com/containrrr/watchtower/pkg/container/mocks"
5-
"github.com/containrrr/watchtower/pkg/filters"
64
"github.com/docker/docker/api/types"
75
"github.com/docker/docker/api/types/container"
8-
cli "github.com/docker/docker/client"
96
"github.com/docker/go-connections/nat"
107
. "github.com/onsi/ginkgo"
118
. "github.com/onsi/gomega"
129
)
1310

1411
var _ = Describe("the container", func() {
15-
Describe("the client", func() {
16-
var docker *cli.Client
17-
var client Client
18-
BeforeSuite(func() {
19-
server := mocks.NewMockAPIServer()
20-
docker, _ = cli.NewClientWithOpts(
21-
cli.WithHost(server.URL),
22-
cli.WithHTTPClient(server.Client()))
23-
client = dockerClient{
24-
api: docker,
25-
pullImages: false,
26-
}
27-
})
28-
It("should return a client for the api", func() {
29-
Expect(client).NotTo(BeNil())
30-
})
31-
Describe("WarnOnHeadPullFailed", func() {
32-
containerUnknown := *mockContainerWithImageName("unknown.repo/prefix/imagename:latest")
33-
containerKnown := *mockContainerWithImageName("docker.io/prefix/imagename:latest")
34-
35-
When("warn on head failure is set to \"always\"", func() {
36-
c := newClientNoAPI(false, false, false, false, false, "always")
37-
It("should always return true", func() {
38-
Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeTrue())
39-
Expect(c.WarnOnHeadPullFailed(containerKnown)).To(BeTrue())
40-
})
41-
})
42-
When("warn on head failure is set to \"auto\"", func() {
43-
c := newClientNoAPI(false, false, false, false, false, "auto")
44-
It("should always return true", func() {
45-
Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeFalse())
46-
})
47-
It("should", func() {
48-
Expect(c.WarnOnHeadPullFailed(containerKnown)).To(BeTrue())
49-
})
50-
})
51-
When("warn on head failure is set to \"never\"", func() {
52-
c := newClientNoAPI(false, false, false, false, false, "never")
53-
It("should never return true", func() {
54-
Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeFalse())
55-
Expect(c.WarnOnHeadPullFailed(containerKnown)).To(BeFalse())
56-
})
57-
})
58-
})
59-
60-
When("listing containers without any filter", func() {
61-
It("should return all available containers", func() {
62-
containers, err := client.ListContainers(filters.NoFilter)
63-
Expect(err).NotTo(HaveOccurred())
64-
Expect(len(containers) == 2).To(BeTrue())
65-
})
66-
})
67-
When("listing containers with a filter matching nothing", func() {
68-
It("should return an empty array", func() {
69-
filter := filters.FilterByNames([]string{"lollercoaster"}, filters.NoFilter)
70-
containers, err := client.ListContainers(filter)
71-
Expect(err).NotTo(HaveOccurred())
72-
Expect(len(containers) == 0).To(BeTrue())
73-
})
74-
})
75-
When("listing containers with a watchtower filter", func() {
76-
It("should return only the watchtower container", func() {
77-
containers, err := client.ListContainers(filters.WatchtowerContainersFilter)
78-
Expect(err).NotTo(HaveOccurred())
79-
Expect(len(containers) == 1).To(BeTrue())
80-
Expect(containers[0].ImageName()).To(Equal("containrrr/watchtower:latest"))
81-
})
82-
})
83-
When(`listing containers with the "include stopped" option`, func() {
84-
It("should return both stopped and running containers", func() {
85-
client = dockerClient{
86-
api: docker,
87-
pullImages: false,
88-
includeStopped: true,
89-
}
90-
containers, err := client.ListContainers(filters.NoFilter)
91-
Expect(err).NotTo(HaveOccurred())
92-
Expect(len(containers) > 0).To(BeTrue())
93-
})
94-
})
95-
When(`listing containers with the "include restart" option`, func() {
96-
It("should return both stopped, restarting and running containers", func() {
97-
client = dockerClient{
98-
api: docker,
99-
pullImages: false,
100-
includeRestarting: true,
101-
}
102-
containers, err := client.ListContainers(filters.NoFilter)
103-
Expect(err).NotTo(HaveOccurred())
104-
RestartingContainerFound := false
105-
for _, ContainerRunning := range containers {
106-
if ContainerRunning.containerInfo.State.Restarting {
107-
RestartingContainerFound = true
108-
}
109-
}
110-
Expect(RestartingContainerFound).To(BeTrue())
111-
Expect(RestartingContainerFound).NotTo(BeFalse())
112-
})
113-
})
114-
When(`listing containers without restarting ones`, func() {
115-
It("should not return restarting containers", func() {
116-
client = dockerClient{
117-
api: docker,
118-
pullImages: false,
119-
includeRestarting: false,
120-
}
121-
containers, err := client.ListContainers(filters.NoFilter)
122-
Expect(err).NotTo(HaveOccurred())
123-
RestartingContainerFound := false
124-
for _, ContainerRunning := range containers {
125-
if ContainerRunning.containerInfo.State.Restarting {
126-
RestartingContainerFound = true
127-
}
128-
}
129-
Expect(RestartingContainerFound).To(BeFalse())
130-
Expect(RestartingContainerFound).NotTo(BeTrue())
131-
})
132-
})
133-
})
13412
Describe("VerifyConfiguration", func() {
13513
When("verifying a container with no image info", func() {
13614
It("should return an error", func() {

pkg/container/mocks/ApiServer.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package mocks
33
import (
44
"encoding/json"
55
"fmt"
6+
"github.com/onsi/ginkgo"
67
"io/ioutil"
78
"net/http"
89
"net/http/httptest"
@@ -55,6 +56,22 @@ func NewMockAPIServer() *httptest.Server {
5556
response = getMockJSONFromDisk("./mocks/data/image01.json")
5657
} else if isRequestFor("sha256:4dbc5f9c07028a985e14d1393e849ea07f68804c4293050d5a641b138db72daa", r) {
5758
response = getMockJSONFromDisk("./mocks/data/image02.json")
59+
} else if isRequestFor("containers/ex-cont-id/exec", r) {
60+
response = `{"Id": "ex-exec-id"}`
61+
} else if isRequestFor("exec/ex-exec-id/start", r) {
62+
response = `{"Id": "ex-exec-id"}`
63+
} else if isRequestFor("exec/ex-exec-id/json", r) {
64+
response = `{
65+
"ExecID": "ex-exec-id",
66+
"ContainerID": "ex-cont-id",
67+
"Running": false,
68+
"ExitCode": 0,
69+
"Pid": 0
70+
}`
71+
} else {
72+
// Allow ginkgo to correctly capture the failed assertion, even though this is called from a go func
73+
defer ginkgo.GinkgoRecover()
74+
ginkgo.Fail(fmt.Sprintf("mock API server endpoint not supported: %q", r.URL.String()))
5875
}
5976
_, _ = fmt.Fprintln(w, response)
6077
},

0 commit comments

Comments
 (0)