Skip to content

fix: always use container interface #1516

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
piksel merged 3 commits into from
Apr 12, 2023
Merged

fix: always use container interface #1516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1192d26
fix: always use container interface
piksel Dec 31, 2022
f28ffc6
Merge branch 'main' into fix/container-interface
piksel Apr 12, 2023
801b8f1
add missing method in interface
piksel Apr 12, 2023
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
11 changes: 6 additions & 5 deletions internal/actions/actions_suite_test.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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