Skip to content

Commit

Permalink
fix: always use container interface (#1516)
Browse files Browse the repository at this point in the history
  • Loading branch information
@piksel
piksel committed Apr 12, 2023
1 parent 25fdb40 commit dd1ec09
Show file tree
Hide file tree
Showing 12 changed files with 147 additions and 111 deletions.
11 changes: 6 additions & 5 deletions internal/actions/actions_suite_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package actions_test

import (
"github.com/sirupsen/logrus"
"testing"
"time"

"github.com/sirupsen/logrus"

"github.com/containrrr/watchtower/internal/actions"
"github.com/containrrr/watchtower/pkg/container"
"github.com/containrrr/watchtower/pkg/types"

. "github.com/containrrr/watchtower/internal/actions/mocks"
. "github.com/onsi/ginkgo"
Expand Down Expand Up @@ -37,7 +38,7 @@ var _ = Describe("the actions package", func() {
It("should not do anything", func() {
client := CreateMockClient(
&TestData{
Containers: []container.Container{
Containers: []types.Container{
CreateMockContainer(
"test-container",
"test-container",
Expand All @@ -59,7 +60,7 @@ var _ = Describe("the actions package", func() {
client = CreateMockClient(
&TestData{
NameOfContainerToKeep: "test-container-02",
Containers: []container.Container{
Containers: []types.Container{
CreateMockContainer(
"test-container-01",
"test-container-01",
Expand Down Expand Up @@ -89,7 +90,7 @@ var _ = Describe("the actions package", func() {
BeforeEach(func() {
client = CreateMockClient(
&TestData{
Containers: []container.Container{
Containers: []types.Container{
CreateMockContainer(
"test-container-01",
"test-container-01",
Expand Down
7 changes: 3 additions & 4 deletions internal/actions/check.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,15 @@ package actions

import (
"fmt"
"github.com/containrrr/watchtower/pkg/types"
"sort"
"time"

"github.com/containrrr/watchtower/pkg/container"
"github.com/containrrr/watchtower/pkg/filters"
"github.com/containrrr/watchtower/pkg/sorter"
"github.com/containrrr/watchtower/pkg/types"

log "github.com/sirupsen/logrus"

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

// CheckForSanity makes sure everything is sane before starting
Expand Down Expand Up @@ -55,7 +54,7 @@ func CheckForMultipleWatchtowerInstances(client container.Client, cleanup bool,
return cleanupExcessWatchtowers(containers, client, cleanup)
}

func cleanupExcessWatchtowers(containers []container.Container, client container.Client, cleanup bool) error {
func cleanupExcessWatchtowers(containers []types.Container, client container.Client, cleanup bool) error {
var stopErrors int

sort.Sort(sorter.ByCreated(containers))
Expand Down
18 changes: 8 additions & 10 deletions internal/actions/mocks/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import (
"fmt"
"time"

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

t "github.com/containrrr/watchtower/pkg/types"
)

Expand All @@ -21,7 +19,7 @@ type MockClient struct {
type TestData struct {
TriedToRemoveImageCount int
NameOfContainerToKeep string
Containers []container.Container
Containers []t.Container
Staleness map[string]bool
}

Expand All @@ -40,25 +38,25 @@ func CreateMockClient(data *TestData, pullImages bool, removeVolumes bool) MockC
}

// ListContainers is a mock method returning the provided container testdata
func (client MockClient) ListContainers(_ t.Filter) ([]container.Container, error) {
func (client MockClient) ListContainers(_ t.Filter) ([]t.Container, error) {
return client.TestData.Containers, nil
}

// StopContainer is a mock method
func (client MockClient) StopContainer(c container.Container, _ time.Duration) error {
func (client MockClient) StopContainer(c t.Container, _ time.Duration) error {
if c.Name() == client.TestData.NameOfContainerToKeep {
return errors.New("tried to stop the instance we want to keep")
}
return nil
}

// StartContainer is a mock method
func (client MockClient) StartContainer(_ container.Container) (t.ContainerID, error) {
func (client MockClient) StartContainer(_ t.Container) (t.ContainerID, error) {
return "", nil
}

// RenameContainer is a mock method
func (client MockClient) RenameContainer(_ container.Container, _ string) error {
func (client MockClient) RenameContainer(_ t.Container, _ string) error {
return nil
}

Expand All @@ -69,7 +67,7 @@ func (client MockClient) RemoveImageByID(_ t.ImageID) error {
}

// GetContainer is a mock method
func (client MockClient) GetContainer(_ t.ContainerID) (container.Container, error) {
func (client MockClient) GetContainer(_ t.ContainerID) (t.Container, error) {
return client.TestData.Containers[0], nil
}

Expand All @@ -88,7 +86,7 @@ func (client MockClient) ExecuteCommand(_ t.ContainerID, command string, _ int)
}

// IsContainerStale is true if not explicitly stated in TestData for the mock client
func (client MockClient) IsContainerStale(cont container.Container) (bool, t.ImageID, error) {
func (client MockClient) IsContainerStale(cont t.Container) (bool, t.ImageID, error) {
stale, found := client.TestData.Staleness[cont.Name()]
if !found {
stale = true
Expand All @@ -97,6 +95,6 @@ func (client MockClient) IsContainerStale(cont container.Container) (bool, t.Ima
}

// WarnOnHeadPullFailed is always true for the mock client
func (client MockClient) WarnOnHeadPullFailed(_ container.Container) bool {
func (client MockClient) WarnOnHeadPullFailed(_ t.Container) bool {
return true
}
22 changes: 11 additions & 11 deletions internal/actions/mocks/container.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
)

// CreateMockContainer creates a container substitute valid for testing
func CreateMockContainer(id string, name string, image string, created time.Time) container.Container {
func CreateMockContainer(id string, name string, image string, created time.Time) wt.Container {
content := types.ContainerJSON{
ContainerJSONBase: &types.ContainerJSONBase{
ID: id,
Expand All @@ -31,7 +31,7 @@ func CreateMockContainer(id string, name string, image string, created time.Time
ExposedPorts: map[nat.Port]struct{}{},
},
}
return *container.NewContainer(
return container.NewContainer(
&content,
CreateMockImageInfo(image),
)
Expand All @@ -48,12 +48,12 @@ func CreateMockImageInfo(image string) *types.ImageInspect {
}

// CreateMockContainerWithImageInfo should only be used for testing
func CreateMockContainerWithImageInfo(id string, name string, image string, created time.Time, imageInfo types.ImageInspect) container.Container {
func CreateMockContainerWithImageInfo(id string, name string, image string, created time.Time, imageInfo types.ImageInspect) wt.Container {
return CreateMockContainerWithImageInfoP(id, name, image, created, &imageInfo)
}

// CreateMockContainerWithImageInfoP should only be used for testing
func CreateMockContainerWithImageInfoP(id string, name string, image string, created time.Time, imageInfo *types.ImageInspect) container.Container {
func CreateMockContainerWithImageInfoP(id string, name string, image string, created time.Time, imageInfo *types.ImageInspect) wt.Container {
content := types.ContainerJSON{
ContainerJSONBase: &types.ContainerJSONBase{
ID: id,
Expand All @@ -66,21 +66,21 @@ func CreateMockContainerWithImageInfoP(id string, name string, image string, cre
Labels: make(map[string]string),
},
}
return *container.NewContainer(
return container.NewContainer(
&content,
imageInfo,
)
}

// CreateMockContainerWithDigest should only be used for testing
func CreateMockContainerWithDigest(id string, name string, image string, created time.Time, digest string) container.Container {
func CreateMockContainerWithDigest(id string, name string, image string, created time.Time, digest string) wt.Container {
c := CreateMockContainer(id, name, image, created)
c.ImageInfo().RepoDigests = []string{digest}
return c
}

// CreateMockContainerWithConfig creates a container substitute valid for testing
func CreateMockContainerWithConfig(id string, name string, image string, running bool, restarting bool, created time.Time, config *dockerContainer.Config) container.Container {
func CreateMockContainerWithConfig(id string, name string, image string, running bool, restarting bool, created time.Time, config *dockerContainer.Config) wt.Container {
content := types.ContainerJSON{
ContainerJSONBase: &types.ContainerJSONBase{
ID: id,
Expand All @@ -97,14 +97,14 @@ func CreateMockContainerWithConfig(id string, name string, image string, running
},
Config: config,
}
return *container.NewContainer(
return container.NewContainer(
&content,
CreateMockImageInfo(image),
)
}

// CreateContainerForProgress creates a container substitute for tracking session/update progress
func CreateContainerForProgress(index int, idPrefix int, nameFormat string) (container.Container, wt.ImageID) {
func CreateContainerForProgress(index int, idPrefix int, nameFormat string) (wt.Container, wt.ImageID) {
indexStr := strconv.Itoa(idPrefix + index)
mockID := indexStr + strings.Repeat("0", 61-len(indexStr))
contID := "c79" + mockID
Expand All @@ -120,7 +120,7 @@ func CreateContainerForProgress(index int, idPrefix int, nameFormat string) (con
}

// CreateMockContainerWithLinks should only be used for testing
func CreateMockContainerWithLinks(id string, name string, image string, created time.Time, links []string, imageInfo *types.ImageInspect) container.Container {
func CreateMockContainerWithLinks(id string, name string, image string, created time.Time, links []string, imageInfo *types.ImageInspect) wt.Container {
content := types.ContainerJSON{
ContainerJSONBase: &types.ContainerJSONBase{
ID: id,
Expand All @@ -136,7 +136,7 @@ func CreateMockContainerWithLinks(id string, name string, image string, created
Labels: make(map[string]string),
},
}
return *container.NewContainer(
return container.NewContainer(
&content,
imageInfo,
)
Expand Down
26 changes: 13 additions & 13 deletions internal/actions/update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func Update(client container.Client, params types.UpdateParams) (types.Report, e
} else {
progress.AddScanned(targetContainer, newestImage)
}
containers[i].Stale = stale
containers[i].SetStale(stale)

if stale {
staleCount++
Expand All @@ -71,7 +71,7 @@ func Update(client container.Client, params types.UpdateParams) (types.Report, e

UpdateImplicitRestart(containers)

var containersToUpdate []container.Container
var containersToUpdate []types.Container
if !params.MonitorOnly {
for _, c := range containers {
if !c.IsMonitorOnly() {
Expand All @@ -96,7 +96,7 @@ func Update(client container.Client, params types.UpdateParams) (types.Report, e
return progress.Report(), nil
}

func performRollingRestart(containers []container.Container, client container.Client, params types.UpdateParams) map[types.ContainerID]error {
func performRollingRestart(containers []types.Container, client container.Client, params types.UpdateParams) map[types.ContainerID]error {
cleanupImageIDs := make(map[types.ImageID]bool, len(containers))
failed := make(map[types.ContainerID]error, len(containers))

Expand All @@ -108,7 +108,7 @@ func performRollingRestart(containers []container.Container, client container.Cl
} else {
if err := restartStaleContainer(containers[i], client, params); err != nil {
failed[containers[i].ID()] = err
} else if containers[i].Stale {
} else if containers[i].IsStale() {
// Only add (previously) stale containers' images to cleanup
cleanupImageIDs[containers[i].ImageID()] = true
}
Expand All @@ -122,7 +122,7 @@ func performRollingRestart(containers []container.Container, client container.Cl
return failed
}

func stopContainersInReversedOrder(containers []container.Container, client container.Client, params types.UpdateParams) (failed map[types.ContainerID]error, stopped map[types.ImageID]bool) {
func stopContainersInReversedOrder(containers []types.Container, client container.Client, params types.UpdateParams) (failed map[types.ContainerID]error, stopped map[types.ImageID]bool) {
failed = make(map[types.ContainerID]error, len(containers))
stopped = make(map[types.ImageID]bool, len(containers))
for i := len(containers) - 1; i >= 0; i-- {
Expand All @@ -137,7 +137,7 @@ func stopContainersInReversedOrder(containers []container.Container, client cont
return
}

func stopStaleContainer(container container.Container, client container.Client, params types.UpdateParams) error {
func stopStaleContainer(container types.Container, client container.Client, params types.UpdateParams) error {
if container.IsWatchtower() {
log.Debugf("This is the watchtower container %s", container.Name())
return nil
Expand All @@ -148,7 +148,7 @@ func stopStaleContainer(container container.Container, client container.Client,
}

// Perform an additional check here to prevent us from stopping a linked container we cannot restart
if container.LinkedToRestarting {
if container.IsLinkedToRestarting() {
if err := container.VerifyConfiguration(); err != nil {
return err
}
Expand All @@ -174,7 +174,7 @@ func stopStaleContainer(container container.Container, client container.Client,
return nil
}

func restartContainersInSortedOrder(containers []container.Container, client container.Client, params types.UpdateParams, stoppedImages map[types.ImageID]bool) map[types.ContainerID]error {
func restartContainersInSortedOrder(containers []types.Container, client container.Client, params types.UpdateParams, stoppedImages map[types.ImageID]bool) map[types.ContainerID]error {
cleanupImageIDs := make(map[types.ImageID]bool, len(containers))
failed := make(map[types.ContainerID]error, len(containers))

Expand All @@ -185,7 +185,7 @@ func restartContainersInSortedOrder(containers []container.Container, client con
if stoppedImages[c.SafeImageID()] {
if err := restartStaleContainer(c, client, params); err != nil {
failed[c.ID()] = err
} else if c.Stale {
} else if c.IsStale() {
// Only add (previously) stale containers' images to cleanup
cleanupImageIDs[c.ImageID()] = true
}
Expand All @@ -210,7 +210,7 @@ func cleanupImages(client container.Client, imageIDs map[types.ImageID]bool) {
}
}

func restartStaleContainer(container container.Container, client container.Client, params types.UpdateParams) error {
func restartStaleContainer(container types.Container, client container.Client, params types.UpdateParams) error {
// Since we can't shutdown a watchtower container immediately, we need to
// start the new one while the old one is still running. This prevents us
// from re-using the same container name so we first rename the current
Expand All @@ -235,7 +235,7 @@ func restartStaleContainer(container container.Container, client container.Clien

// UpdateImplicitRestart iterates through the passed containers, setting the
// `LinkedToRestarting` flag if any of it's linked containers are marked for restart
func UpdateImplicitRestart(containers []container.Container) {
func UpdateImplicitRestart(containers []types.Container) {

for ci, c := range containers {
if c.ToRestart() {
Expand All @@ -249,15 +249,15 @@ func UpdateImplicitRestart(containers []container.Container) {
"linked": c.Name(),
}).Debug("container is linked to restarting")
// NOTE: To mutate the array, the `c` variable cannot be used as it's a copy
containers[ci].LinkedToRestarting = true
containers[ci].SetLinkedToRestarting(true)
}

}
}

// linkedContainerMarkedForRestart returns the name of the first link that matches a
// container marked for restart
func linkedContainerMarkedForRestart(links []string, containers []container.Container) string {
func linkedContainerMarkedForRestart(links []string, containers []types.Container) string {
for _, linkName := range links {
for _, candidate := range containers {
if candidate.Name() == linkName && candidate.ToRestart() {
Expand Down
Loading

0 comments on commit dd1ec09

Please sign in to comment.