From d68b68fc433fe70c4a2b4f6a96866d3f90d6a2b4 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 7 Apr 2021 13:42:31 +0200 Subject: [PATCH 1/3] restartmanager: RestartManager.Cancel(): remove unused error return This function would never return an error, and no code was handling errors. Signed-off-by: Sebastiaan van Stijn --- restartmanager/restartmanager.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/restartmanager/restartmanager.go b/restartmanager/restartmanager.go index 12094def60..363fcc2af0 100644 --- a/restartmanager/restartmanager.go +++ b/restartmanager/restartmanager.go @@ -21,7 +21,7 @@ var ErrRestartCanceled = errors.New("restart canceled") // RestartManager defines object that controls container restarting rules. type RestartManager interface { - Cancel() error + Cancel() ShouldRestart(exitCode uint32, hasBeenManuallyStopped bool, executionDuration time.Duration) (bool, chan error, error) } @@ -125,12 +125,11 @@ func (rm *restartManager) ShouldRestart(exitCode uint32, hasBeenManuallyStopped return true, ch, nil } -func (rm *restartManager) Cancel() error { +func (rm *restartManager) Cancel() { rm.Do(func() { rm.Lock() rm.canceled = true close(rm.cancel) rm.Unlock() }) - return nil } From efb97da0dacb630bc91e12be0f26cbc21528990c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 7 Apr 2021 13:40:15 +0200 Subject: [PATCH 2/3] restartmanager: add SetPolicy() to the RestartManager interface Signed-off-by: Sebastiaan van Stijn --- container/container.go | 8 +------- restartmanager/restartmanager.go | 1 + 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/container/container.go b/container/container.go index 53c452159d..dada1dc980 100644 --- a/container/container.go +++ b/container/container.go @@ -557,13 +557,7 @@ func (container *Container) InitDNSHostConfig() { // UpdateMonitor updates monitor configure for running container func (container *Container) UpdateMonitor(restartPolicy containertypes.RestartPolicy) { - type policySetter interface { - SetPolicy(containertypes.RestartPolicy) - } - - if rm, ok := container.RestartManager().(policySetter); ok { - rm.SetPolicy(restartPolicy) - } + container.RestartManager().SetPolicy(restartPolicy) } // FullHostname returns hostname and optional domain appended to it. diff --git a/restartmanager/restartmanager.go b/restartmanager/restartmanager.go index 363fcc2af0..f7a547d2d1 100644 --- a/restartmanager/restartmanager.go +++ b/restartmanager/restartmanager.go @@ -23,6 +23,7 @@ var ErrRestartCanceled = errors.New("restart canceled") type RestartManager interface { Cancel() ShouldRestart(exitCode uint32, hasBeenManuallyStopped bool, executionDuration time.Duration) (bool, chan error, error) + SetPolicy(policy container.RestartPolicy) } type restartManager struct { From c5d4b6b311645d23ebd5e0bd173708d5dd4a6c85 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 10 Feb 2022 17:27:38 +0100 Subject: [PATCH 3/3] restartmanager: remove RestartManager interface It only had a single implementation, so we may as well remove the added complexity of defining it as an interface. Signed-off-by: Sebastiaan van Stijn --- container/container.go | 4 ++-- restartmanager/restartmanager.go | 23 ++++++++++------------- restartmanager/restartmanager_test.go | 4 ++-- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/container/container.go b/container/container.go index dada1dc980..37a6a74c1c 100644 --- a/container/container.go +++ b/container/container.go @@ -93,7 +93,7 @@ type Container struct { // logDriver for closing LogDriver logger.Logger `json:"-"` LogCopier *logger.Copier `json:"-"` - restartManager restartmanager.RestartManager + restartManager *restartmanager.RestartManager attachContext *attachContext // Fields here are specific to Unix platforms @@ -570,7 +570,7 @@ func (container *Container) FullHostname() string { } // RestartManager returns the current restartmanager instance connected to container. -func (container *Container) RestartManager() restartmanager.RestartManager { +func (container *Container) RestartManager() *restartmanager.RestartManager { if container.restartManager == nil { container.restartManager = restartmanager.New(container.HostConfig.RestartPolicy, container.RestartCount) } diff --git a/restartmanager/restartmanager.go b/restartmanager/restartmanager.go index f7a547d2d1..43738454fc 100644 --- a/restartmanager/restartmanager.go +++ b/restartmanager/restartmanager.go @@ -20,13 +20,7 @@ const ( var ErrRestartCanceled = errors.New("restart canceled") // RestartManager defines object that controls container restarting rules. -type RestartManager interface { - Cancel() - ShouldRestart(exitCode uint32, hasBeenManuallyStopped bool, executionDuration time.Duration) (bool, chan error, error) - SetPolicy(policy container.RestartPolicy) -} - -type restartManager struct { +type RestartManager struct { sync.Mutex sync.Once policy container.RestartPolicy @@ -37,18 +31,20 @@ type restartManager struct { canceled bool } -// New returns a new restartManager based on a policy. -func New(policy container.RestartPolicy, restartCount int) RestartManager { - return &restartManager{policy: policy, restartCount: restartCount, cancel: make(chan struct{})} +// New returns a new RestartManager based on a policy. +func New(policy container.RestartPolicy, restartCount int) *RestartManager { + return &RestartManager{policy: policy, restartCount: restartCount, cancel: make(chan struct{})} } -func (rm *restartManager) SetPolicy(policy container.RestartPolicy) { +// SetPolicy sets the restart-policy for the RestartManager. +func (rm *RestartManager) SetPolicy(policy container.RestartPolicy) { rm.Lock() rm.policy = policy rm.Unlock() } -func (rm *restartManager) ShouldRestart(exitCode uint32, hasBeenManuallyStopped bool, executionDuration time.Duration) (bool, chan error, error) { +// ShouldRestart returns whether the container should be restarted. +func (rm *RestartManager) ShouldRestart(exitCode uint32, hasBeenManuallyStopped bool, executionDuration time.Duration) (bool, chan error, error) { if rm.policy.IsNone() { return false, nil, nil } @@ -126,7 +122,8 @@ func (rm *restartManager) ShouldRestart(exitCode uint32, hasBeenManuallyStopped return true, ch, nil } -func (rm *restartManager) Cancel() { +// Cancel tells the RestartManager to no longer restart the container. +func (rm *RestartManager) Cancel() { rm.Do(func() { rm.Lock() rm.canceled = true diff --git a/restartmanager/restartmanager_test.go b/restartmanager/restartmanager_test.go index 82558946bc..3dbd37c536 100644 --- a/restartmanager/restartmanager_test.go +++ b/restartmanager/restartmanager_test.go @@ -8,7 +8,7 @@ import ( ) func TestRestartManagerTimeout(t *testing.T) { - rm := New(container.RestartPolicy{Name: "always"}, 0).(*restartManager) + rm := New(container.RestartPolicy{Name: "always"}, 0) var duration = 1 * time.Second should, _, err := rm.ShouldRestart(0, false, duration) if err != nil { @@ -23,7 +23,7 @@ func TestRestartManagerTimeout(t *testing.T) { } func TestRestartManagerTimeoutReset(t *testing.T) { - rm := New(container.RestartPolicy{Name: "always"}, 0).(*restartManager) + rm := New(container.RestartPolicy{Name: "always"}, 0) rm.timeout = 5 * time.Second var duration = 10 * time.Second _, _, err := rm.ShouldRestart(0, false, duration)