From 45146cff4e2e2a9854bdbca800d5757c7bb8b037 Mon Sep 17 00:00:00 2001 From: yanxuean Date: Mon, 25 Sep 2017 15:19:20 +0800 Subject: [PATCH 1/3] Use arg cgroupRoot,not nodeConfig.CgroupRoot Using both arg cgroupRoot and nodeConfig.CgroupRoot is confused in function NewQOSContainerManager Signed-off-by: yanxuean --- pkg/kubelet/cm/qos_container_manager_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kubelet/cm/qos_container_manager_linux.go b/pkg/kubelet/cm/qos_container_manager_linux.go index 8cfea46ce6f..4436b10af10 100644 --- a/pkg/kubelet/cm/qos_container_manager_linux.go +++ b/pkg/kubelet/cm/qos_container_manager_linux.go @@ -63,7 +63,7 @@ type qosContainerManagerImpl struct { func NewQOSContainerManager(subsystems *CgroupSubsystems, cgroupRoot string, nodeConfig NodeConfig) (QOSContainerManager, error) { if !nodeConfig.CgroupsPerQOS { return &qosContainerManagerNoop{ - cgroupRoot: CgroupName(nodeConfig.CgroupRoot), + cgroupRoot: CgroupName(cgroupRoot), }, nil } From f011c044d4a1c302355f46e339d9e9070221fdd3 Mon Sep 17 00:00:00 2001 From: yanxuean Date: Mon, 25 Sep 2017 16:59:15 +0800 Subject: [PATCH 2/3] improve cgroupmanager in qosContainerManager improve arg "cgroupRoot" type in NewQOSContainerManager Signed-off-by: yanxuean --- pkg/kubelet/cm/container_manager_linux.go | 2 +- pkg/kubelet/cm/qos_container_manager_linux.go | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/kubelet/cm/container_manager_linux.go b/pkg/kubelet/cm/container_manager_linux.go index fea87a2da88..e38dd90f3cb 100644 --- a/pkg/kubelet/cm/container_manager_linux.go +++ b/pkg/kubelet/cm/container_manager_linux.go @@ -256,7 +256,7 @@ func NewContainerManager(mountUtil mount.Interface, cadvisorInterface cadvisor.I } glog.Infof("Creating Container Manager object based on Node Config: %+v", nodeConfig) - qosContainerManager, err := NewQOSContainerManager(subsystems, cgroupRoot, nodeConfig) + qosContainerManager, err := NewQOSContainerManager(subsystems, cgroupRoot, nodeConfig, cgroupManager) if err != nil { return nil, err } diff --git a/pkg/kubelet/cm/qos_container_manager_linux.go b/pkg/kubelet/cm/qos_container_manager_linux.go index 4436b10af10..3a3e897034b 100644 --- a/pkg/kubelet/cm/qos_container_manager_linux.go +++ b/pkg/kubelet/cm/qos_container_manager_linux.go @@ -56,11 +56,11 @@ type qosContainerManagerImpl struct { cgroupManager CgroupManager activePods ActivePodsFunc getNodeAllocatable func() v1.ResourceList - cgroupRoot string + cgroupRoot CgroupName qosReserved map[v1.ResourceName]int64 } -func NewQOSContainerManager(subsystems *CgroupSubsystems, cgroupRoot string, nodeConfig NodeConfig) (QOSContainerManager, error) { +func NewQOSContainerManager(subsystems *CgroupSubsystems, cgroupRoot string, nodeConfig NodeConfig, cgroupManager CgroupManager) (QOSContainerManager, error) { if !nodeConfig.CgroupsPerQOS { return &qosContainerManagerNoop{ cgroupRoot: CgroupName(cgroupRoot), @@ -69,8 +69,8 @@ func NewQOSContainerManager(subsystems *CgroupSubsystems, cgroupRoot string, nod return &qosContainerManagerImpl{ subsystems: subsystems, - cgroupManager: NewCgroupManager(subsystems, nodeConfig.CgroupDriver), - cgroupRoot: cgroupRoot, + cgroupManager: cgroupManager, + cgroupRoot: CgroupName(cgroupRoot), qosReserved: nodeConfig.ExperimentalQOSReserved, }, nil } @@ -81,7 +81,7 @@ func (m *qosContainerManagerImpl) GetQOSContainersInfo() QOSContainersInfo { func (m *qosContainerManagerImpl) Start(getNodeAllocatable func() v1.ResourceList, activePods ActivePodsFunc) error { cm := m.cgroupManager - rootContainer := m.cgroupRoot + rootContainer := string(m.cgroupRoot) if !cm.Exists(CgroupName(rootContainer)) { return fmt.Errorf("root container %s doesn't exist", rootContainer) } @@ -95,8 +95,8 @@ func (m *qosContainerManagerImpl) Start(getNodeAllocatable func() v1.ResourceLis // Create containers for both qos classes for qosClass, containerName := range qosClasses { - // get the container's absolute name - absoluteContainerName := CgroupName(containerName) + // get the container's abstract name + abstractContainerName := CgroupName(containerName) resourceParameters := &ResourceConfig{} // the BestEffort QoS class has a statically configured minShares value @@ -107,7 +107,7 @@ func (m *qosContainerManagerImpl) Start(getNodeAllocatable func() v1.ResourceLis // containerConfig object stores the cgroup specifications containerConfig := &CgroupConfig{ - Name: absoluteContainerName, + Name: abstractContainerName, ResourceParameters: resourceParameters, } @@ -117,7 +117,7 @@ func (m *qosContainerManagerImpl) Start(getNodeAllocatable func() v1.ResourceLis } // check if it exists - if !cm.Exists(absoluteContainerName) { + if !cm.Exists(abstractContainerName) { if err := cm.Create(containerConfig); err != nil { return fmt.Errorf("failed to create top level %v QOS cgroup : %v", qosClass, err) } From 5d5fee8cabc335ac070d5e3d0c5d331dd4b9d8c7 Mon Sep 17 00:00:00 2001 From: yanxuean Date: Mon, 9 Oct 2017 11:17:52 +0800 Subject: [PATCH 3/3] capitalize the first letter capitalize the first letter for the field comment of containerManagerImpl Signed-off-by: yanxuean --- pkg/kubelet/cm/container_manager_linux.go | 2 +- pkg/kubelet/cm/qos_container_manager_linux.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/kubelet/cm/container_manager_linux.go b/pkg/kubelet/cm/container_manager_linux.go index e38dd90f3cb..9dfba9a055b 100644 --- a/pkg/kubelet/cm/container_manager_linux.go +++ b/pkg/kubelet/cm/container_manager_linux.go @@ -110,7 +110,7 @@ type containerManagerImpl struct { qosContainers QOSContainersInfo // Tasks that are run periodically periodicTasks []func() - // holds all the mounted cgroup subsystems + // Holds all the mounted cgroup subsystems subsystems *CgroupSubsystems nodeInfo *v1.Node // Interface for cgroup management diff --git a/pkg/kubelet/cm/qos_container_manager_linux.go b/pkg/kubelet/cm/qos_container_manager_linux.go index 3a3e897034b..96c327ef470 100644 --- a/pkg/kubelet/cm/qos_container_manager_linux.go +++ b/pkg/kubelet/cm/qos_container_manager_linux.go @@ -95,8 +95,8 @@ func (m *qosContainerManagerImpl) Start(getNodeAllocatable func() v1.ResourceLis // Create containers for both qos classes for qosClass, containerName := range qosClasses { - // get the container's abstract name - abstractContainerName := CgroupName(containerName) + // get the container's absolute name + absoluteContainerName := CgroupName(containerName) resourceParameters := &ResourceConfig{} // the BestEffort QoS class has a statically configured minShares value @@ -107,7 +107,7 @@ func (m *qosContainerManagerImpl) Start(getNodeAllocatable func() v1.ResourceLis // containerConfig object stores the cgroup specifications containerConfig := &CgroupConfig{ - Name: abstractContainerName, + Name: absoluteContainerName, ResourceParameters: resourceParameters, } @@ -117,7 +117,7 @@ func (m *qosContainerManagerImpl) Start(getNodeAllocatable func() v1.ResourceLis } // check if it exists - if !cm.Exists(abstractContainerName) { + if !cm.Exists(absoluteContainerName) { if err := cm.Create(containerConfig); err != nil { return fmt.Errorf("failed to create top level %v QOS cgroup : %v", qosClass, err) }