Skip to content
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.

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

Promote LocalStorageCapacityIsolation feature to beta #60159

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/features/kube_features.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ const (
PersistentLocalVolumes utilfeature.Feature = "PersistentLocalVolumes"

// owner: @jinxu
// alpha: v1.7
// beta: v1.10
//
// New local storage types to support local storage capacity isolation
LocalStorageCapacityIsolation utilfeature.Feature = "LocalStorageCapacityIsolation"
Expand Down Expand Up @@ -281,7 +281,7 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS
RotateKubeletServerCertificate: {Default: false, PreRelease: utilfeature.Alpha},
RotateKubeletClientCertificate: {Default: true, PreRelease: utilfeature.Beta},
PersistentLocalVolumes: {Default: true, PreRelease: utilfeature.Beta},
LocalStorageCapacityIsolation: {Default: false, PreRelease: utilfeature.Alpha},
LocalStorageCapacityIsolation: {Default: true, PreRelease: utilfeature.Beta},
HugePages: {Default: true, PreRelease: utilfeature.Beta},
DebugContainers: {Default: false, PreRelease: utilfeature.Alpha},
PodShareProcessNamespace: {Default: false, PreRelease: utilfeature.Alpha},
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/cm/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ go_library(
"//pkg/scheduler/schedulercache:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library",
Expand Down Expand Up @@ -140,7 +141,6 @@ go_library(
"//vendor/github.com/opencontainers/runc/libcontainer/cgroups/fs:go_default_library",
"//vendor/github.com/opencontainers/runc/libcontainer/cgroups/systemd:go_default_library",
"//vendor/github.com/opencontainers/runc/libcontainer/configs:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//vendor/k8s.io/client-go/tools/record:go_default_library",
Expand Down
8 changes: 7 additions & 1 deletion pkg/kubelet/cm/container_manager_stub.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/golang/glog"
"k8s.io/api/core/v1"

"k8s.io/apimachinery/pkg/api/resource"
internalapi "k8s.io/kubernetes/pkg/kubelet/apis/cri"
"k8s.io/kubernetes/pkg/kubelet/cm/cpumanager"
"k8s.io/kubernetes/pkg/kubelet/config"
Expand Down Expand Up @@ -67,7 +68,12 @@ func (cm *containerManagerStub) GetNodeAllocatableReservation() v1.ResourceList
}

func (cm *containerManagerStub) GetCapacity() v1.ResourceList {
return nil
c := v1.ResourceList{
v1.ResourceEphemeralStorage: *resource.NewQuantity(
int64(0),
resource.BinarySI),
}
return c
}

func (cm *containerManagerStub) GetDevicePluginResourceCapacity() (v1.ResourceList, v1.ResourceList, []string) {
Expand Down
1 change: 1 addition & 0 deletions pkg/kubelet/images/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ go_library(
"//pkg/kubelet/apis/stats/v1alpha1:go_default_library",
"//pkg/kubelet/container:go_default_library",
"//pkg/kubelet/events:go_default_library",
"//pkg/kubelet/util/sliceutils:go_default_library",
"//pkg/util/parsers:go_default_library",
"//vendor/github.com/docker/distribution/reference:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
Expand Down
13 changes: 9 additions & 4 deletions pkg/kubelet/images/image_gc_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
statsapi "k8s.io/kubernetes/pkg/kubelet/apis/stats/v1alpha1"
"k8s.io/kubernetes/pkg/kubelet/container"
"k8s.io/kubernetes/pkg/kubelet/events"
"k8s.io/kubernetes/pkg/kubelet/util/sliceutils"
)

// StatsProvider is an interface for fetching stats used during image garbage
Expand Down Expand Up @@ -120,11 +121,15 @@ func (i *imageCache) set(images []container.Image) {
i.images = images
}

// get gets image list from image cache.
// get gets a sorted (by image size) image list from image cache.
// There is a potentical data race in this function. See PR #60448
// Because there is deepcopy function available currently, move sort
// function inside this function
func (i *imageCache) get() []container.Image {
i.RLock()
defer i.RUnlock()
return append([]container.Image{}, i.images...)
i.Lock()
defer i.Unlock()
sort.Sort(sliceutils.ByImageSize(i.images))
return i.images
}

// Information about the images we track.
Expand Down
9 changes: 5 additions & 4 deletions pkg/kubelet/kubelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -560,11 +560,11 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
return nil, err
}
klet.networkPlugin = plug

machineInfo, err := klet.GetCachedMachineInfo()
machineInfo, err := klet.cadvisor.MachineInfo()
if err != nil {
return nil, err
}
klet.machineInfo = machineInfo

imageBackOff := flowcontrol.NewBackOff(backOffPeriod, MaxContainerBackOff)

Expand Down Expand Up @@ -1338,8 +1338,6 @@ func (kl *Kubelet) initializeRuntimeDependentModules() {
// TODO(random-liu): Add backoff logic in the babysitter
glog.Fatalf("Failed to start cAdvisor %v", err)
}
// eviction manager must start after cadvisor because it needs to know if the container runtime has a dedicated imagefs
kl.evictionManager.Start(kl.StatsProvider, kl.GetActivePods, kl.podResourcesAreReclaimed, evictionMonitoringPeriod)

// trigger on-demand stats collection once so that we have capacity information for ephemeral storage.
// ignore any errors, since if stats collection is not successful, the container manager will fail to start below.
Expand All @@ -1355,6 +1353,9 @@ func (kl *Kubelet) initializeRuntimeDependentModules() {
// Fail kubelet and rely on the babysitter to retry starting kubelet.
glog.Fatalf("Failed to start ContainerManager %v", err)
}
// eviction manager must start after cadvisor because it needs to know if the container runtime has a dedicated imagefs
kl.evictionManager.Start(kl.StatsProvider, kl.GetActivePods, kl.podResourcesAreReclaimed, evictionMonitoringPeriod)

// container log manager must start after container runtime is up to retrieve information from container runtime
// and inform container to reopen log file after log rotation.
kl.containerLogManager.Start()
Expand Down
7 changes: 0 additions & 7 deletions pkg/kubelet/kubelet_getters.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,12 +281,5 @@ func (kl *Kubelet) GetVersionInfo() (*cadvisorapiv1.VersionInfo, error) {

// GetCachedMachineInfo assumes that the machine info can't change without a reboot
func (kl *Kubelet) GetCachedMachineInfo() (*cadvisorapiv1.MachineInfo, error) {
if kl.machineInfo == nil {
info, err := kl.cadvisor.MachineInfo()
if err != nil {
return nil, err
}
kl.machineInfo = info
}
return kl.machineInfo, nil
}
12 changes: 7 additions & 5 deletions pkg/kubelet/kubelet_node_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import (
"math"
"net"
goruntime "runtime"
"sort"
"strings"
"sync"
"time"

"github.com/golang/glog"
Expand All @@ -42,7 +42,6 @@ import (
"k8s.io/kubernetes/pkg/kubelet/cadvisor"
"k8s.io/kubernetes/pkg/kubelet/events"
"k8s.io/kubernetes/pkg/kubelet/util"
"k8s.io/kubernetes/pkg/kubelet/util/sliceutils"
"k8s.io/kubernetes/pkg/scheduler/algorithm"
nodeutil "k8s.io/kubernetes/pkg/util/node"
"k8s.io/kubernetes/pkg/version"
Expand Down Expand Up @@ -702,7 +701,6 @@ func (kl *Kubelet) setNodeStatusImages(node *v1.Node) {
return
}
// sort the images from max to min, and only set top N images into the node status.
sort.Sort(sliceutils.ByImageSize(containerImages))
if maxImagesInNodeStatus < len(containerImages) {
containerImages = containerImages[0:maxImagesInNodeStatus]
}
Expand Down Expand Up @@ -773,7 +771,6 @@ func (kl *Kubelet) setNodeReadyCondition(node *v1.Node) {
LastHeartbeatTime: currentTime,
}
}

// Append AppArmor status if it's enabled.
// TODO(tallclair): This is a temporary message until node feature reporting is added.
if newNodeReadyCondition.Status == v1.ConditionTrue &&
Expand Down Expand Up @@ -1019,10 +1016,15 @@ func (kl *Kubelet) setNodeOODCondition(node *v1.Node) {

// Maintains Node.Spec.Unschedulable value from previous run of tryUpdateNodeStatus()
// TODO: why is this a package var?
var oldNodeUnschedulable bool
var (
oldNodeUnschedulable bool
oldNodeUnschedulableLock sync.Mutex
)

// record if node schedulable change.
func (kl *Kubelet) recordNodeSchedulableEvent(node *v1.Node) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think recordNodeScheduableEvent should be one of NodeStatusFuncs.
I'm fine with adding a lock here for now. But actually we should move recordNodeScheduableEvent out of NodeStatusFuncs and put it where it should be.

@dashpole WDYT?

oldNodeUnschedulableLock.Lock()
defer oldNodeUnschedulableLock.Unlock()
if oldNodeUnschedulable != node.Spec.Unschedulable {
if node.Spec.Unschedulable {
kl.recordNodeStatusEvent(v1.EventTypeNormal, events.NodeNotSchedulable)
Expand Down