Skip to content
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.

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

feat(log): use short image/container IDs in logs #888

Merged
merged 1 commit into from
Apr 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 14 additions & 11 deletions pkg/container/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,10 @@ func (client dockerClient) StopContainer(c Container, timeout time.Duration) err
signal = defaultStopSignal
}

shortID := ShortID(c.ID())

if c.IsRunning() {
log.Infof("Stopping %s (%s) with %s", c.Name(), c.ID(), signal)
log.Infof("Stopping %s (%s) with %s", c.Name(), shortID, signal)
if err := client.api.ContainerKill(bg, c.ID(), signal); err != nil {
return err
}
Expand All @@ -158,9 +160,9 @@ func (client dockerClient) StopContainer(c Container, timeout time.Duration) err
_ = client.waitForStopOrTimeout(c, timeout)

if c.containerInfo.HostConfig.AutoRemove {
log.Debugf("AutoRemove container %s, skipping ContainerRemove call.", c.ID())
log.Debugf("AutoRemove container %s, skipping ContainerRemove call.", shortID)
} else {
log.Debugf("Removing container %s", c.ID())
log.Debugf("Removing container %s", shortID)

if err := client.api.ContainerRemove(bg, c.ID(), types.ContainerRemoveOptions{Force: true, RemoveVolumes: client.removeVolumes}); err != nil {
return err
Expand All @@ -169,7 +171,7 @@ func (client dockerClient) StopContainer(c Container, timeout time.Duration) err

// Wait for container to be removed. In this case an error is a good thing
if err := client.waitForStopOrTimeout(c, timeout); err == nil {
return fmt.Errorf("container %s (%s) could not be removed", c.Name(), c.ID())
return fmt.Errorf("container %s (%s) could not be removed", c.Name(), shortID)
}

return nil
Expand Down Expand Up @@ -229,7 +231,7 @@ func (client dockerClient) StartContainer(c Container) (string, error) {
func (client dockerClient) doStartContainer(bg context.Context, c Container, creation container.ContainerCreateCreatedBody) error {
name := c.Name()

log.Debugf("Starting container %s (%s)", name, creation.ID)
log.Debugf("Starting container %s (%s)", name, ShortID(creation.ID))
err := client.api.ContainerStart(bg, creation.ID, types.ContainerStartOptions{})
if err != nil {
return err
Expand All @@ -239,7 +241,7 @@ func (client dockerClient) doStartContainer(bg context.Context, c Container, cre

func (client dockerClient) RenameContainer(c Container, newName string) error {
bg := context.Background()
log.Debugf("Renaming container %s (%s) to %s", c.Name(), c.ID(), newName)
log.Debugf("Renaming container %s (%s) to %s", c.Name(), ShortID(c.ID()), newName)
return client.api.ContainerRename(bg, c.ID(), newName)
}

Expand Down Expand Up @@ -269,7 +271,7 @@ func (client dockerClient) HasNewImage(ctx context.Context, container Container)
return false, nil
}

log.Infof("Found new %s image (%s)", imageName, newImageInfo.ID)
log.Infof("Found new %s image (%s)", imageName, ShortID(newImageInfo.ID))
return true, nil
}

Expand All @@ -284,13 +286,13 @@ func (client dockerClient) PullImage(ctx context.Context, container Container) e

log.WithFields(fields).Debugf("Trying to load authentication credentials.")
opts, err := registry.GetPullOptions(imageName)
if opts.RegistryAuth != "" {
log.Debug("Credentials loaded")
}
if err != nil {
log.Debugf("Error loading authentication credentials %s", err)
return err
}
if opts.RegistryAuth != "" {
log.Debug("Credentials loaded")
}
Comment on lines -287 to +295
Copy link
Member Author

Choose a reason for hiding this comment

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

Not actually part of the intended changes, but got a lint warning about the use of opts before checking if an error was returned. Shouldn't matter, but doing it in this order is at least more correct.


log.WithFields(fields).Debugf("Checking if pull is needed")

Expand Down Expand Up @@ -326,7 +328,7 @@ func (client dockerClient) PullImage(ctx context.Context, container Container) e
}

func (client dockerClient) RemoveImageByID(id string) error {
log.Infof("Removing image %s", id)
log.Infof("Removing image %s", ShortID(id))

_, err := client.api.ImageRemove(
context.Background(),
Expand Down Expand Up @@ -404,6 +406,7 @@ func (client dockerClient) waitForExecOrTimeout(bg context.Context, ID string, e
for {
execInspect, err := client.api.ContainerExecInspect(ctx, ID)

//goland:noinspection GoNilness
log.WithFields(log.Fields{
"exit-code": execInspect.ExitCode,
"exec-id": execInspect.ExecID,
Expand Down
23 changes: 23 additions & 0 deletions pkg/container/util.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package container

import "strings"

// ShortID returns the 12-character (hex) short version of an image ID hash, removing any "sha256:" prefix if present
func ShortID(imageID string) (short string) {
prefixSep := strings.IndexRune(imageID, ':')
offset := 0
length := 12
if prefixSep >= 0 {
if imageID[0:prefixSep] == "sha256" {
offset = prefixSep + 1
} else {
length += prefixSep + 1
}
}

if len(imageID) >= offset+length {
return imageID[offset : offset+length]
}

return imageID
}
46 changes: 46 additions & 0 deletions pkg/container/util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package container_test

import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

. "github.com/containrrr/watchtower/pkg/container"
)

var _ = Describe("container utils", func() {
Describe("ShortID", func() {
When("given a normal image ID", func() {
When("it contains a sha256 prefix", func() {
It("should return that ID in short version", func() {
actual := ShortID("sha256:0123456789abcd00000000001111111111222222222233333333334444444444")
Expect(actual).To(Equal("0123456789ab"))
})
})
When("it doesn't contain a prefix", func() {
It("should return that ID in short version", func() {
actual := ShortID("0123456789abcd00000000001111111111222222222233333333334444444444")
Expect(actual).To(Equal("0123456789ab"))
})
})
})
When("given a short image ID", func() {
When("it contains no prefix", func() {
It("should return the same string", func() {
Expect(ShortID("0123456789ab")).To(Equal("0123456789ab"))
})
})
When("it contains a the sha256 prefix", func() {
It("should return the ID without the prefix", func() {
Expect(ShortID("sha256:0123456789ab")).To(Equal("0123456789ab"))
})
})
})
When("given an ID with an unknown prefix", func() {
It("should return a short version of that ID including the prefix", func() {
Expect(ShortID("md5:0123456789ab")).To(Equal("md5:0123456789ab"))
Expect(ShortID("md5:0123456789abcdefg")).To(Equal("md5:0123456789ab"))
Expect(ShortID("md5:01")).To(Equal("md5:01"))
})
})
})
})