Skip to content

K
k3s
Unverified Commit 53ee0c86 authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub
Browse files
Options

Merge pull request #65660 from mtaufen/incremental-refactor-kubelet-node-status

Automatic merge from submit-queue (batch tested with PRs 66152, 66406, 66218, 66278, 65660). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Refactor kubelet node status setters, add test coverage This internal refactor moves the node status setters to a new package, explicitly injects dependencies to facilitate unit testing, and adds individual unit tests for the setters. I gave each setter a distinct commit to facilitate review. Non-goals: - I intentionally excluded the class of setters that return a "modified" boolean, as I want to think more carefully about how to cleanly handle the behavior, and this PR is already rather large. - I would like to clean up the status update control loops as well, but that belongs in a separate PR. ```release-note NONE ```
parents 4ca54820 0f8976bf
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 147 additions and 14 deletions
pkg/kubelet/BUILD
View file @ 53ee0c86
...@@ -10,7 +10,6 @@ go_library( ...@@ -10,7 +10,6 @@ go_library(
name = "go_default_library", name = "go_default_library",
srcs = [ srcs = [
"active_deadline.go", "active_deadline.go",
"cloud_request_manager.go",
"doc.go", "doc.go",
"kubelet.go", "kubelet.go",
"kubelet_getters.go", "kubelet_getters.go",
...@@ -48,6 +47,7 @@ go_library( ...@@ -48,6 +47,7 @@ go_library(
"//pkg/kubelet/cadvisor:go_default_library", "//pkg/kubelet/cadvisor:go_default_library",
"//pkg/kubelet/certificate:go_default_library", "//pkg/kubelet/certificate:go_default_library",
"//pkg/kubelet/checkpointmanager:go_default_library", "//pkg/kubelet/checkpointmanager:go_default_library",
"//pkg/kubelet/cloudresource:go_default_library",
"//pkg/kubelet/cm:go_default_library", "//pkg/kubelet/cm:go_default_library",
"//pkg/kubelet/config:go_default_library", "//pkg/kubelet/config:go_default_library",
"//pkg/kubelet/configmap:go_default_library", "//pkg/kubelet/configmap:go_default_library",
...@@ -66,6 +66,7 @@ go_library( ...@@ -66,6 +66,7 @@ go_library(
"//pkg/kubelet/metrics/collectors:go_default_library", "//pkg/kubelet/metrics/collectors:go_default_library",
"//pkg/kubelet/mountpod:go_default_library", "//pkg/kubelet/mountpod:go_default_library",
"//pkg/kubelet/network/dns:go_default_library", "//pkg/kubelet/network/dns:go_default_library",
"//pkg/kubelet/nodestatus:go_default_library",
"//pkg/kubelet/pleg:go_default_library", "//pkg/kubelet/pleg:go_default_library",
"//pkg/kubelet/pod:go_default_library", "//pkg/kubelet/pod:go_default_library",
"//pkg/kubelet/preemption:go_default_library", "//pkg/kubelet/preemption:go_default_library",
...@@ -102,7 +103,6 @@ go_library( ...@@ -102,7 +103,6 @@ go_library(
"//pkg/util/node:go_default_library", "//pkg/util/node:go_default_library",
"//pkg/util/oom:go_default_library", "//pkg/util/oom:go_default_library",
"//pkg/util/removeall:go_default_library", "//pkg/util/removeall:go_default_library",
"//pkg/version:go_default_library",
"//pkg/volume:go_default_library", "//pkg/volume:go_default_library",
"//pkg/volume/csi:go_default_library", "//pkg/volume/csi:go_default_library",
"//pkg/volume/util:go_default_library", "//pkg/volume/util:go_default_library",
...@@ -119,7 +119,6 @@ go_library( ...@@ -119,7 +119,6 @@ go_library(
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/clock:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/clock:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/net:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation:go_default_library",
...@@ -147,7 +146,6 @@ go_test( ...@@ -147,7 +146,6 @@ go_test(
name = "go_default_test", name = "go_default_test",
srcs = [ srcs = [
"active_deadline_test.go", "active_deadline_test.go",
"cloud_request_manager_test.go",
"kubelet_getters_test.go", "kubelet_getters_test.go",
"kubelet_network_test.go", "kubelet_network_test.go",
"kubelet_node_status_test.go", "kubelet_node_status_test.go",
...@@ -166,7 +164,6 @@ go_test( ...@@ -166,7 +164,6 @@ go_test(
deps = [ deps = [
"//pkg/apis/core/install:go_default_library", "//pkg/apis/core/install:go_default_library",
"//pkg/capabilities:go_default_library", "//pkg/capabilities:go_default_library",
"//pkg/cloudprovider/providers/fake:go_default_library",
"//pkg/kubelet/apis:go_default_library", "//pkg/kubelet/apis:go_default_library",
"//pkg/kubelet/apis/cri/runtime/v1alpha2:go_default_library", "//pkg/kubelet/apis/cri/runtime/v1alpha2:go_default_library",
"//pkg/kubelet/cadvisor/testing:go_default_library", "//pkg/kubelet/cadvisor/testing:go_default_library",
...@@ -180,6 +177,7 @@ go_test( ...@@ -180,6 +177,7 @@ go_test(
"//pkg/kubelet/lifecycle:go_default_library", "//pkg/kubelet/lifecycle:go_default_library",
"//pkg/kubelet/logs:go_default_library", "//pkg/kubelet/logs:go_default_library",
"//pkg/kubelet/network/dns:go_default_library", "//pkg/kubelet/network/dns:go_default_library",
"//pkg/kubelet/nodestatus:go_default_library",
"//pkg/kubelet/pleg:go_default_library", "//pkg/kubelet/pleg:go_default_library",
"//pkg/kubelet/pod:go_default_library", "//pkg/kubelet/pod:go_default_library",
"//pkg/kubelet/pod/testing:go_default_library", "//pkg/kubelet/pod/testing:go_default_library",
...@@ -255,6 +253,7 @@ filegroup( ...@@ -255,6 +253,7 @@ filegroup(
"//pkg/kubelet/checkpoint:all-srcs", "//pkg/kubelet/checkpoint:all-srcs",
"//pkg/kubelet/checkpointmanager:all-srcs", "//pkg/kubelet/checkpointmanager:all-srcs",
"//pkg/kubelet/client:all-srcs", "//pkg/kubelet/client:all-srcs",
"//pkg/kubelet/cloudresource:all-srcs",
"//pkg/kubelet/cm:all-srcs", "//pkg/kubelet/cm:all-srcs",
"//pkg/kubelet/config:all-srcs", "//pkg/kubelet/config:all-srcs",
"//pkg/kubelet/configmap:all-srcs", "//pkg/kubelet/configmap:all-srcs",
...@@ -273,6 +272,7 @@ filegroup( ...@@ -273,6 +272,7 @@ filegroup(
"//pkg/kubelet/metrics:all-srcs", "//pkg/kubelet/metrics:all-srcs",
"//pkg/kubelet/mountpod:all-srcs", "//pkg/kubelet/mountpod:all-srcs",
"//pkg/kubelet/network:all-srcs", "//pkg/kubelet/network:all-srcs",
"//pkg/kubelet/nodestatus:all-srcs",
"//pkg/kubelet/pleg:all-srcs", "//pkg/kubelet/pleg:all-srcs",
"//pkg/kubelet/pod:all-srcs", "//pkg/kubelet/pod:all-srcs",
"//pkg/kubelet/preemption:all-srcs", "//pkg/kubelet/preemption:all-srcs",
......
pkg/kubelet/cloudresource/BUILD 0 → 100644
View file @ 53ee0c86
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
go_library(
name = "go_default_library",
srcs = ["cloud_request_manager.go"],
importpath = "k8s.io/kubernetes/pkg/kubelet/cloudresource",
visibility = ["//visibility:public"],
deps = [
"//pkg/cloudprovider:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
],
)
go_test(
name = "go_default_test",
srcs = ["cloud_request_manager_test.go"],
embed = [":go_default_library"],
deps = [
"//pkg/cloudprovider/providers/fake:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
],
)
filegroup(
name = "package-srcs",
srcs = glob(["**"]),
tags = ["automanaged"],
visibility = ["//visibility:private"],
)
filegroup(
name = "all-srcs",
srcs = [":package-srcs"],
tags = ["automanaged"],
visibility = ["//visibility:public"],
)
pkg/kubelet/cloud_request_manager.go → pkg/kubelet/cloudresource/cloud_request_manager.go
View file @ 53ee0c86
...@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and ...@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License. limitations under the License.
*/ */
package kubelet package cloudresource
import ( import (
"context" "context"
...@@ -32,6 +32,14 @@ import ( ...@@ -32,6 +32,14 @@ import (
var nodeAddressesRetryPeriod = 5 * time.Second var nodeAddressesRetryPeriod = 5 * time.Second
// SyncManager is an interface for making requests to a cloud provider
type SyncManager interface {
Run(stopCh <-chan struct{})
NodeAddresses() ([]v1.NodeAddress, error)
}
var _ SyncManager = &cloudResourceSyncManager{}
type cloudResourceSyncManager struct { type cloudResourceSyncManager struct {
// Cloud provider interface. // Cloud provider interface.
cloud cloudprovider.Interface cloud cloudprovider.Interface
...@@ -45,9 +53,9 @@ type cloudResourceSyncManager struct { ...@@ -45,9 +53,9 @@ type cloudResourceSyncManager struct {
nodeName types.NodeName nodeName types.NodeName
} }
// NewCloudResourceSyncManager creates a manager responsible for collecting resources // NewSyncManager creates a manager responsible for collecting resources
// from a cloud provider through requests that are sensitive to timeouts and hanging // from a cloud provider through requests that are sensitive to timeouts and hanging
func NewCloudResourceSyncManager(cloud cloudprovider.Interface, nodeName types.NodeName, syncPeriod time.Duration) *cloudResourceSyncManager { func NewSyncManager(cloud cloudprovider.Interface, nodeName types.NodeName, syncPeriod time.Duration) SyncManager {
return &cloudResourceSyncManager{ return &cloudResourceSyncManager{
cloud: cloud, cloud: cloud,
syncPeriod: syncPeriod, syncPeriod: syncPeriod,
......
pkg/kubelet/cloud_request_manager_test.go → pkg/kubelet/cloudresource/cloud_request_manager_test.go
View file @ 53ee0c86
...@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and ...@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License. limitations under the License.
*/ */
package kubelet package cloudresource
import ( import (
"fmt" "fmt"
...@@ -64,7 +64,7 @@ func TestNodeAddressesRequest(t *testing.T) { ...@@ -64,7 +64,7 @@ func TestNodeAddressesRequest(t *testing.T) {
stopCh := make(chan struct{}) stopCh := make(chan struct{})
defer close(stopCh) defer close(stopCh)
manager := NewCloudResourceSyncManager(cloud, "defaultNode", syncPeriod) manager := NewSyncManager(cloud, "defaultNode", syncPeriod).(*cloudResourceSyncManager)
go manager.Run(stopCh) go manager.Run(stopCh)
nodeAddresses, err := collectNodeAddresses(manager) nodeAddresses, err := collectNodeAddresses(manager)
......
pkg/kubelet/kubelet.go
View file @ 53ee0c86
...@@ -61,6 +61,7 @@ import ( ...@@ -61,6 +61,7 @@ import (
"k8s.io/kubernetes/pkg/kubelet/cadvisor" "k8s.io/kubernetes/pkg/kubelet/cadvisor"
kubeletcertificate "k8s.io/kubernetes/pkg/kubelet/certificate" kubeletcertificate "k8s.io/kubernetes/pkg/kubelet/certificate"
"k8s.io/kubernetes/pkg/kubelet/checkpointmanager" "k8s.io/kubernetes/pkg/kubelet/checkpointmanager"
"k8s.io/kubernetes/pkg/kubelet/cloudresource"
"k8s.io/kubernetes/pkg/kubelet/cm" "k8s.io/kubernetes/pkg/kubelet/cm"
"k8s.io/kubernetes/pkg/kubelet/config" "k8s.io/kubernetes/pkg/kubelet/config"
"k8s.io/kubernetes/pkg/kubelet/configmap" "k8s.io/kubernetes/pkg/kubelet/configmap"
...@@ -521,7 +522,7 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration, ...@@ -521,7 +522,7 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
} }
if klet.cloud != nil { if klet.cloud != nil {
klet.cloudResourceSyncManager = NewCloudResourceSyncManager(klet.cloud, nodeName, klet.nodeStatusUpdateFrequency) klet.cloudResourceSyncManager = cloudresource.NewSyncManager(klet.cloud, nodeName, klet.nodeStatusUpdateFrequency)
} }
var secretManager secret.Manager var secretManager secret.Manager
...@@ -789,7 +790,6 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration, ...@@ -789,7 +790,6 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
klet.backOff = flowcontrol.NewBackOff(backOffPeriod, MaxContainerBackOff) klet.backOff = flowcontrol.NewBackOff(backOffPeriod, MaxContainerBackOff)
klet.podKillingCh = make(chan *kubecontainer.PodPair, podKillingChannelCapacity) klet.podKillingCh = make(chan *kubecontainer.PodPair, podKillingChannelCapacity)
klet.setNodeStatusFuncs = klet.defaultNodeStatusFuncs()
// setup eviction manager // setup eviction manager
evictionManager, evictionAdmitHandler := eviction.NewManager(klet.resourceAnalyzer, evictionConfig, killPodNow(klet.podWorkers, kubeDeps.Recorder), klet.imageManager, klet.containerGC, kubeDeps.Recorder, nodeRef, klet.clock) evictionManager, evictionAdmitHandler := eviction.NewManager(klet.resourceAnalyzer, evictionConfig, killPodNow(klet.podWorkers, kubeDeps.Recorder), klet.imageManager, klet.containerGC, kubeDeps.Recorder, nodeRef, klet.clock)
...@@ -836,6 +836,11 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration, ...@@ -836,6 +836,11 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
// Finally, put the most recent version of the config on the Kubelet, so // Finally, put the most recent version of the config on the Kubelet, so
// people can see how it was configured. // people can see how it was configured.
klet.kubeletConfiguration = *kubeCfg klet.kubeletConfiguration = *kubeCfg
// Generating the status funcs should be the last thing we do,
// since this relies on the rest of the Kubelet having been constructed.
klet.setNodeStatusFuncs = klet.defaultNodeStatusFuncs()
return klet, nil return klet, nil
} }
...@@ -962,7 +967,7 @@ type Kubelet struct { ...@@ -962,7 +967,7 @@ type Kubelet struct {
// Cloud provider interface. // Cloud provider interface.
cloud cloudprovider.Interface cloud cloudprovider.Interface
// Handles requests to cloud provider with timeout // Handles requests to cloud provider with timeout
cloudResourceSyncManager *cloudResourceSyncManager cloudResourceSyncManager cloudresource.SyncManager
// Indicates that the node initialization happens in an external cloud controller // Indicates that the node initialization happens in an external cloud controller
externalCloudProvider bool externalCloudProvider bool
......
pkg/kubelet/kubelet_test.go
View file @ 53ee0c86
...@@ -292,7 +292,6 @@ func newTestKubeletWithImageList( ...@@ -292,7 +292,6 @@ func newTestKubeletWithImageList(
// Relist period does not affect the tests. // Relist period does not affect the tests.
kubelet.pleg = pleg.NewGenericPLEG(fakeRuntime, 100, time.Hour, nil, clock.RealClock{}) kubelet.pleg = pleg.NewGenericPLEG(fakeRuntime, 100, time.Hour, nil, clock.RealClock{})
kubelet.clock = fakeClock kubelet.clock = fakeClock
kubelet.setNodeStatusFuncs = kubelet.defaultNodeStatusFuncs()
nodeRef := &v1.ObjectReference{ nodeRef := &v1.ObjectReference{
Kind: "Node", Kind: "Node",
...@@ -338,6 +337,8 @@ func newTestKubeletWithImageList( ...@@ -338,6 +337,8 @@ func newTestKubeletWithImageList(
false, /* experimentalCheckNodeCapabilitiesBeforeMount*/ false, /* experimentalCheckNodeCapabilitiesBeforeMount*/
false /* keepTerminatedPodVolumes */) false /* keepTerminatedPodVolumes */)
kubelet.setNodeStatusFuncs = kubelet.defaultNodeStatusFuncs()
// enable active deadline handler // enable active deadline handler
activeDeadlineHandler, err := newActiveDeadlineHandler(kubelet.statusManager, kubelet.recorder, kubelet.clock) activeDeadlineHandler, err := newActiveDeadlineHandler(kubelet.statusManager, kubelet.recorder, kubelet.clock)
require.NoError(t, err, "Can't initialize active deadline handler") require.NoError(t, err, "Can't initialize active deadline handler")
......
pkg/kubelet/nodestatus/BUILD 0 → 100644
View file @ 53ee0c86
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
go_library(
name = "go_default_library",
srcs = ["setters.go"],
importpath = "k8s.io/kubernetes/pkg/kubelet/nodestatus",
visibility = ["//visibility:public"],
deps = [
"//pkg/apis/core/v1/helper:go_default_library",
"//pkg/cloudprovider:go_default_library",
"//pkg/features:go_default_library",
"//pkg/kubelet/apis:go_default_library",
"//pkg/kubelet/cadvisor:go_default_library",
"//pkg/kubelet/cm:go_default_library",
"//pkg/kubelet/container:go_default_library",
"//pkg/kubelet/events:go_default_library",
"//pkg/version:go_default_library",
"//pkg/volume:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/net:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
"//vendor/github.com/google/cadvisor/info/v1:go_default_library",
],
)
filegroup(
name = "package-srcs",
srcs = glob(["**"]),
tags = ["automanaged"],
visibility = ["//visibility:private"],
)
filegroup(
name = "all-srcs",
srcs = [":package-srcs"],
tags = ["automanaged"],
visibility = ["//visibility:public"],
)
go_test(
name = "go_default_test",
srcs = ["setters_test.go"],
embed = [":go_default_library"],
deps = [
"//pkg/cloudprovider/providers/fake:go_default_library",
"//pkg/kubelet/cm:go_default_library",
"//pkg/kubelet/container:go_default_library",
"//pkg/kubelet/container/testing:go_default_library",
"//pkg/kubelet/events:go_default_library",
"//pkg/kubelet/util/sliceutils:go_default_library",
"//pkg/version:go_default_library",
"//pkg/volume:go_default_library",
"//pkg/volume/testing:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/rand:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library",
"//vendor/github.com/google/cadvisor/info/v1:go_default_library",
"//vendor/github.com/stretchr/testify/assert:go_default_library",
"//vendor/github.com/stretchr/testify/require:go_default_library",
],
)
pkg/volume/testing/testing.go
View file @ 53ee0c86
...@@ -223,6 +223,9 @@ type FakeVolumePlugin struct { ...@@ -223,6 +223,9 @@ type FakeVolumePlugin struct {
LastProvisionerOptions VolumeOptions LastProvisionerOptions VolumeOptions
NewAttacherCallCount int NewAttacherCallCount int
NewDetacherCallCount int NewDetacherCallCount int
VolumeLimits map[string]int64
VolumeLimitsError error
LimitKey string
Mounters []*FakeVolume Mounters []*FakeVolume
Unmounters []*FakeVolume Unmounters []*FakeVolume
...@@ -238,6 +241,7 @@ var _ RecyclableVolumePlugin = &FakeVolumePlugin{} ...@@ -238,6 +241,7 @@ var _ RecyclableVolumePlugin = &FakeVolumePlugin{}
var _ DeletableVolumePlugin = &FakeVolumePlugin{} var _ DeletableVolumePlugin = &FakeVolumePlugin{}
var _ ProvisionableVolumePlugin = &FakeVolumePlugin{} var _ ProvisionableVolumePlugin = &FakeVolumePlugin{}
var _ AttachableVolumePlugin = &FakeVolumePlugin{} var _ AttachableVolumePlugin = &FakeVolumePlugin{}
var _ VolumePluginWithAttachLimits = &FakeVolumePlugin{}
func (plugin *FakeVolumePlugin) getFakeVolume(list *[]*FakeVolume) *FakeVolume { func (plugin *FakeVolumePlugin) getFakeVolume(list *[]*FakeVolume) *FakeVolume {
volume := &FakeVolume{} volume := &FakeVolume{}
...@@ -448,6 +452,14 @@ func (plugin *FakeVolumePlugin) RequiresFSResize() bool { ...@@ -448,6 +452,14 @@ func (plugin *FakeVolumePlugin) RequiresFSResize() bool {
return true return true
} }
func (plugin *FakeVolumePlugin) GetVolumeLimits() (map[string]int64, error) {
return plugin.VolumeLimits, plugin.VolumeLimitsError
}
func (plugin *FakeVolumePlugin) VolumeLimitKey(spec *Spec) string {
return plugin.LimitKey
}
type FakeFileVolumePlugin struct { type FakeFileVolumePlugin struct {
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment