Refactor to use structure to pass parameters

pkg/kubelet/kubelet.go

@@ -867,8 +867,16 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
 		util.SetNodeOwnerFunc(klet.heartbeatClient, string(klet.nodeName)))
 
 	// setup node shutdown manager
-	shutdownManager, shutdownAdmitHandler := nodeshutdown.NewManager(klet.probeManager, kubeDeps.Recorder, nodeRef, klet.GetActivePods, killPodNow(klet.podWorkers, kubeDeps.Recorder), klet.syncNodeStatus, kubeCfg.ShutdownGracePeriod.Duration, kubeCfg.ShutdownGracePeriodCriticalPods.Duration)
+	shutdownManager, shutdownAdmitHandler := nodeshutdown.NewManager(&nodeshutdown.Config{
-
+		ProbeManager:                    klet.probeManager,
+		Recorder:                        kubeDeps.Recorder,
+		NodeRef:                         nodeRef,
+		GetPodsFunc:                     klet.GetActivePods,
+		KillPodFunc:                     killPodNow(klet.podWorkers, kubeDeps.Recorder),
+		SyncNodeStatusFunc:              klet.syncNodeStatus,
+		ShutdownGracePeriodRequested:    kubeCfg.ShutdownGracePeriod.Duration,
+		ShutdownGracePeriodCriticalPods: kubeCfg.ShutdownGracePeriodCriticalPods.Duration,
+	})
 	klet.shutdownManager = shutdownManager
 	klet.admitHandlers.AddPodAdmitHandler(shutdownAdmitHandler)
 
@@ -1214,7 +1222,7 @@ type Kubelet struct {
 	runtimeClassManager *runtimeclass.Manager
 
 	// Handles node shutdown events for the Node.
-	shutdownManager *nodeshutdown.Manager
+	shutdownManager nodeshutdown.Manager
 }
 
 // ListPodStats is delegated to StatsProvider, which implements stats.Provider interface

pkg/kubelet/kubelet_test.go

@@ -314,7 +314,16 @@ func newTestKubeletWithImageList(
 	kubelet.admitHandlers.AddPodAdmitHandler(evictionAdmitHandler)
 
 	// setup shutdown manager
-	shutdownManager, shutdownAdmitHandler := nodeshutdown.NewManager(kubelet.probeManager, fakeRecorder, nodeRef, kubelet.podManager.GetPods, killPodNow(kubelet.podWorkers, fakeRecorder), func() {}, 0 /* shutdownGracePeriodRequested*/, 0 /*shutdownGracePeriodCriticalPods */)
+	shutdownManager, shutdownAdmitHandler := nodeshutdown.NewManager(&nodeshutdown.Config{
+		ProbeManager:                    kubelet.probeManager,
+		Recorder:                        fakeRecorder,
+		NodeRef:                         nodeRef,
+		GetPodsFunc:                     kubelet.podManager.GetPods,
+		KillPodFunc:                     killPodNow(kubelet.podWorkers, fakeRecorder),
+		SyncNodeStatusFunc:              func() {},
+		ShutdownGracePeriodRequested:    0,
+		ShutdownGracePeriodCriticalPods: 0,
+	})
 	kubelet.shutdownManager = shutdownManager
 	kubelet.admitHandlers.AddPodAdmitHandler(shutdownAdmitHandler)
 

pkg/kubelet/nodeshutdown/nodeshutdown_manager.go

@@ -0,0 +1,66 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package nodeshutdown
+
+import (
+	"time"
+
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/client-go/tools/record"
+	"k8s.io/kubernetes/pkg/kubelet/eviction"
+	"k8s.io/kubernetes/pkg/kubelet/lifecycle"
+	"k8s.io/kubernetes/pkg/kubelet/prober"
+	"k8s.io/utils/clock"
+)
+
+// Manager interface provides methods for Kubelet to manage node shutdown.
+type Manager interface {
+	Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAdmitResult
+	Start() error
+	ShutdownStatus() error
+}
+
+// Config represents Manager configuration
+type Config struct {
+	ProbeManager                    prober.Manager
+	Recorder                        record.EventRecorder
+	NodeRef                         *v1.ObjectReference
+	GetPodsFunc                     eviction.ActivePodsFunc
+	KillPodFunc                     eviction.KillPodFunc
+	SyncNodeStatusFunc              func()
+	ShutdownGracePeriodRequested    time.Duration
+	ShutdownGracePeriodCriticalPods time.Duration
+	Clock                           clock.Clock
+}
+
+// managerStub is a fake node shutdown managerImpl .
+type managerStub struct{}
+
+// Admit returns a fake Pod admission which always returns true
+func (managerStub) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAdmitResult {
+	return lifecycle.PodAdmitResult{Admit: true}
+}
+
+// Start is a no-op always returning nil for non linux platforms.
+func (managerStub) Start() error {
+	return nil
+}
+
+// ShutdownStatus is a no-op always returning nil for non linux platforms.
+func (managerStub) ShutdownStatus() error {
+	return nil
+}

pkg/kubelet/nodeshutdown/nodeshutdown_manager_linux.go

@@ -60,8 +60,8 @@ type dbusInhibiter interface {
 	OverrideInhibitDelay(inhibitDelayMax time.Duration) error
 }
 
-// Manager has functions that can be used to interact with the Node Shutdown Manager.
+// managerImpl has functions that can be used to interact with the Node Shutdown Manager.
-type Manager struct {
+type managerImpl struct {
 	recorder     record.EventRecorder
 	nodeRef      *v1.ObjectReference
 	probeManager prober.Manager
@@ -83,23 +83,31 @@ type Manager struct {
 }
 
 // NewManager returns a new node shutdown manager.
-func NewManager(probeManager prober.Manager, recorder record.EventRecorder, nodeRef *v1.ObjectReference, getPodsFunc eviction.ActivePodsFunc, killPodFunc eviction.KillPodFunc, syncNodeStatus func(), shutdownGracePeriodRequested, shutdownGracePeriodCriticalPods time.Duration) (*Manager, lifecycle.PodAdmitHandler) {
+func NewManager(conf *Config) (Manager, lifecycle.PodAdmitHandler) {
-	manager := &Manager{
+	if !utilfeature.DefaultFeatureGate.Enabled(features.GracefulNodeShutdown) ||
-		recorder:                        recorder,
+		(conf.ShutdownGracePeriodRequested == 0 && conf.ShutdownGracePeriodCriticalPods == 0) {
-		nodeRef:                         nodeRef,
+		m := managerStub{}
-		getPods:                         getPodsFunc,
+		return m, m
-		killPodFunc:                     killPodFunc,
+	}
-		syncNodeStatus:                  syncNodeStatus,
+	if conf.Clock == nil {
-		shutdownGracePeriodRequested:    shutdownGracePeriodRequested,
+		conf.Clock = clock.RealClock{}
-		shutdownGracePeriodCriticalPods: shutdownGracePeriodCriticalPods,
+	}
-		clock:                           clock.RealClock{},
+	manager := &managerImpl{
-		probeManager:                    probeManager,
+		probeManager:                    conf.ProbeManager,
+		recorder:                        conf.Recorder,
+		nodeRef:                         conf.NodeRef,
+		getPods:                         conf.GetPodsFunc,
+		killPodFunc:                     conf.KillPodFunc,
+		syncNodeStatus:                  conf.SyncNodeStatusFunc,
+		shutdownGracePeriodRequested:    conf.ShutdownGracePeriodRequested,
+		shutdownGracePeriodCriticalPods: conf.ShutdownGracePeriodCriticalPods,
+		clock:                           conf.Clock,
 	}
 	return manager, manager
 }
 
 // Admit rejects all pods if node is shutting
-func (m *Manager) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAdmitResult {
+func (m *managerImpl) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAdmitResult {
 	nodeShuttingDown := m.ShutdownStatus() != nil
 
 	if nodeShuttingDown {
@@ -113,10 +121,7 @@ func (m *Manager) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAdmitR
 }
 
 // Start starts the node shutdown manager and will start watching the node for shutdown events.
-func (m *Manager) Start() error {
+func (m *managerImpl) Start() error {
-	if !m.isFeatureEnabled() {
-		return nil
-	}
 	stop, err := m.start()
 	if err != nil {
 		return err
@@ -138,7 +143,7 @@ func (m *Manager) Start() error {
 	return nil
 }
 
-func (m *Manager) start() (chan struct{}, error) {
+func (m *managerImpl) start() (chan struct{}, error) {
 	systemBus, err := systemDbus()
 	if err != nil {
 		return nil, err
@@ -234,7 +239,7 @@ func (m *Manager) start() (chan struct{}, error) {
 	return stop, nil
 }
 
-func (m *Manager) aquireInhibitLock() error {
+func (m *managerImpl) aquireInhibitLock() error {
 	lock, err := m.dbusCon.InhibitShutdown()
 	if err != nil {
 		return err
@@ -246,17 +251,8 @@ func (m *Manager) aquireInhibitLock() error {
 	return nil
 }
 
-// Returns if the feature is enabled
-func (m *Manager) isFeatureEnabled() bool {
-	return utilfeature.DefaultFeatureGate.Enabled(features.GracefulNodeShutdown) && m.shutdownGracePeriodRequested > 0
-}
-
 // ShutdownStatus will return an error if the node is currently shutting down.
-func (m *Manager) ShutdownStatus() error {
+func (m *managerImpl) ShutdownStatus() error {
-	if !m.isFeatureEnabled() {
-		return nil
-	}
-
 	m.nodeShuttingDownMutex.Lock()
 	defer m.nodeShuttingDownMutex.Unlock()
 
@@ -266,7 +262,7 @@ func (m *Manager) ShutdownStatus() error {
 	return nil
 }
 
-func (m *Manager) processShutdownEvent() error {
+func (m *managerImpl) processShutdownEvent() error {
 	klog.V(1).InfoS("Shutdown manager processing shutdown event")
 	activePods := m.getPods()
 

pkg/kubelet/nodeshutdown/nodeshutdown_manager_linux_test.go

@@ -240,9 +240,17 @@ func TestManager(t *testing.T) {
 			proberManager := probetest.FakeManager{}
 			fakeRecorder := &record.FakeRecorder{}
 			nodeRef := &v1.ObjectReference{Kind: "Node", Name: "test", UID: types.UID("test"), Namespace: ""}
-
+			manager, _ := NewManager(&Config{
-			manager, _ := NewManager(proberManager, fakeRecorder, nodeRef, activePodsFunc, killPodsFunc, func() {}, tc.shutdownGracePeriodRequested, tc.shutdownGracePeriodCriticalPods)
+				ProbeManager:                    proberManager,
-			manager.clock = testingclock.NewFakeClock(time.Now())
+				Recorder:                        fakeRecorder,
+				NodeRef:                         nodeRef,
+				GetPodsFunc:                     activePodsFunc,
+				KillPodFunc:                     killPodsFunc,
+				SyncNodeStatusFunc:              func() {},
+				ShutdownGracePeriodRequested:    tc.shutdownGracePeriodRequested,
+				ShutdownGracePeriodCriticalPods: tc.shutdownGracePeriodCriticalPods,
+				Clock:                           testingclock.NewFakeClock(time.Now()),
+			})
 
 			err := manager.Start()
 			lock.Unlock()
@@ -321,10 +329,18 @@ func TestFeatureEnabled(t *testing.T) {
 			fakeRecorder := &record.FakeRecorder{}
 			nodeRef := &v1.ObjectReference{Kind: "Node", Name: "test", UID: types.UID("test"), Namespace: ""}
 
-			manager, _ := NewManager(proberManager, fakeRecorder, nodeRef, activePodsFunc, killPodsFunc, func() {}, tc.shutdownGracePeriodRequested, 0 /*shutdownGracePeriodCriticalPods*/)
+			manager, _ := NewManager(&Config{
-			manager.clock = testingclock.NewFakeClock(time.Now())
+				ProbeManager:                    proberManager,
+				Recorder:                        fakeRecorder,
+				NodeRef:                         nodeRef,
+				GetPodsFunc:                     activePodsFunc,
+				KillPodFunc:                     killPodsFunc,
+				SyncNodeStatusFunc:              func() {},
+				ShutdownGracePeriodRequested:    tc.shutdownGracePeriodRequested,
+				ShutdownGracePeriodCriticalPods: 0,
+			})
 
-			assert.Equal(t, tc.expectEnabled, manager.isFeatureEnabled())
+			assert.Equal(t, tc.expectEnabled, manager != managerStub{})
 		})
 	}
 }
@@ -367,7 +383,17 @@ func TestRestart(t *testing.T) {
 	proberManager := probetest.FakeManager{}
 	fakeRecorder := &record.FakeRecorder{}
 	nodeRef := &v1.ObjectReference{Kind: "Node", Name: "test", UID: types.UID("test"), Namespace: ""}
-	manager, _ := NewManager(proberManager, fakeRecorder, nodeRef, activePodsFunc, killPodsFunc, syncNodeStatus, shutdownGracePeriodRequested, shutdownGracePeriodCriticalPods)
+	manager, _ := NewManager(&Config{
+		ProbeManager:                    proberManager,
+		Recorder:                        fakeRecorder,
+		NodeRef:                         nodeRef,
+		GetPodsFunc:                     activePodsFunc,
+		KillPodFunc:                     killPodsFunc,
+		SyncNodeStatusFunc:              syncNodeStatus,
+		ShutdownGracePeriodRequested:    shutdownGracePeriodRequested,
+		ShutdownGracePeriodCriticalPods: shutdownGracePeriodCriticalPods,
+	})
+
 	err := manager.Start()
 	lock.Unlock()
 

pkg/kubelet/nodeshutdown/nodeshutdown_manager_others.go

@@ -20,35 +20,11 @@ limitations under the License.
 package nodeshutdown
 
 import (
-	"time"
-
-	v1 "k8s.io/api/core/v1"
-	"k8s.io/client-go/tools/record"
-	"k8s.io/kubernetes/pkg/kubelet/eviction"
 	"k8s.io/kubernetes/pkg/kubelet/lifecycle"
-	"k8s.io/kubernetes/pkg/kubelet/prober"
 )
 
-// Manager is a fake node shutdown manager for non linux platforms.
-type Manager struct{}
-
 // NewManager returns a fake node shutdown manager for non linux platforms.
-func NewManager(proberManager prober.Manager, recorder record.EventRecorder, nodeRef *v1.ObjectReference, getPodsFunc eviction.ActivePodsFunc, killPodFunc eviction.KillPodFunc, syncNodeStatus func(), shutdownGracePeriodRequested, shutdownGracePeriodCriticalPods time.Duration) (*Manager, lifecycle.PodAdmitHandler) {
+func NewManager(conf *Config) (Manager, lifecycle.PodAdmitHandler) {
-	m := &Manager{}
+	m := managerStub{}
 	return m, m
 }
-
-// Admit returns a fake Pod admission which always returns true
-func (m *Manager) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAdmitResult {
-	return lifecycle.PodAdmitResult{Admit: true}
-}
-
-// Start is a no-op always returning nil for non linux platforms.
-func (m *Manager) Start() error {
-	return nil
-}
-
-// ShutdownStatus is a no-op always returning nil for non linux platforms.
-func (m *Manager) ShutdownStatus() error {
-	return nil
-}