Skip to content

Commit

Permalink
Fixes around container resources (#68)
Browse files Browse the repository at this point in the history 
* fix: envtest version with available kubebuilder tools
* fix: improved logging when no resources are boosted
* fix: resource revert paniced when no limits were defined at all
  • Loading branch information
@mikouaj
mikouaj authored Oct 10, 2024
1 parent a31f672 commit fdbc102
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 71 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# Image URL to use all building/pushing image targets
IMG ?= ghcr.io/google/kube-startup-cpu-boost
# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
ENVTEST_K8S_VERSION = 1.31.0
ENVTEST_K8S_VERSION = 1.30.x

# Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
ifeq (,$(shell go env GOBIN))
Expand Down
9 changes: 8 additions & 1 deletion internal/boost/pod/pod.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,23 @@ func RevertResourceBoost(pod *corev1.Pod) error {
}
delete(pod.Labels, BoostLabelKey)
delete(pod.Annotations, BoostAnnotationKey)
for _, container := range pod.Spec.Containers {
for i := range pod.Spec.Containers {
container := &pod.Spec.Containers[i]
if request, ok := annotation.InitCPURequests[container.Name]; ok {
if reqQuantity, err := apiResource.ParseQuantity(request); err == nil {
if container.Resources.Requests == nil {
container.Resources.Requests = corev1.ResourceList{}
}
container.Resources.Requests[corev1.ResourceCPU] = reqQuantity
} else {
return fmt.Errorf("failed to parse CPU request: %s", err)
}
}
if limit, ok := annotation.InitCPULimits[container.Name]; ok {
if limitQuantity, err := apiResource.ParseQuantity(limit); err == nil {
if container.Resources.Limits == nil {
container.Resources.Limits = corev1.ResourceList{}
}
container.Resources.Limits[corev1.ResourceCPU] = limitQuantity
} else {
return fmt.Errorf("failed to parse CPU limit: %s", err)
Expand Down
53 changes: 53 additions & 0 deletions internal/boost/pod/pod_suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,64 @@ package pod_test
import (
"testing"

bpod "github.com/google/kube-startup-cpu-boost/internal/boost/pod"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
apiResource "k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestPod(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Pod Suite")
}

var (
podTemplate *corev1.Pod
containerOneName string
containerTwoName string
)

var _ = BeforeSuite(func() {
containerOneName = "container-one"
containerTwoName = "container-two"
reqQuantity, err := apiResource.ParseQuantity("1")
Expect(err).ShouldNot(HaveOccurred())
limitQuantity, err := apiResource.ParseQuantity("2")
Expect(err).ShouldNot(HaveOccurred())
podTemplate = &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test-pod",
Labels: map[string]string{
bpod.BoostLabelKey: "boost-001",
},
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: containerOneName,
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: reqQuantity,
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: limitQuantity,
},
},
},
{
Name: containerTwoName,
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: reqQuantity,
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: limitQuantity,
},
},
},
},
},
}
})
107 changes: 46 additions & 61 deletions internal/boost/pod/pod_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,88 +21,46 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
apiResource "k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var _ = Describe("Pod", func() {
var pod *corev1.Pod
var annot *bpod.BoostPodAnnotation
var containerOne, containerTwo string
var reqQuantity, limitQuantity apiResource.Quantity
var err error
var (
annot *bpod.BoostPodAnnotation
pod *corev1.Pod
err error
)

BeforeEach(func() {
containerOne = "container-one"
containerTwo = "container-one"
reqQuantity, err = apiResource.ParseQuantity("1")
Expect(err).ShouldNot(HaveOccurred())
limitQuantity, err = apiResource.ParseQuantity("2")
Expect(err).ShouldNot(HaveOccurred())
annot = &bpod.BoostPodAnnotation{
BoostTimestamp: time.Now(),
InitCPURequests: map[string]string{
containerOne: "500m",
containerTwo: "500m",
containerOneName: "500m",
containerTwoName: "500m",
},
InitCPULimits: map[string]string{
containerOne: "1",
containerTwo: "1",
containerOneName: "1",
containerTwoName: "1",
},
}
pod = &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test-pod",
Labels: map[string]string{
bpod.BoostLabelKey: "boost-001",
},
Annotations: map[string]string{
bpod.BoostAnnotationKey: annot.ToJSON(),
},
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: containerOne,
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: reqQuantity,
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: limitQuantity,
},
},
},
{
Name: containerTwo,
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: reqQuantity,
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: limitQuantity,
},
},
},
},
},
pod = podTemplate.DeepCopy()
pod.Annotations = map[string]string{
bpod.BoostAnnotationKey: annot.ToJSON(),
}
})

Describe("Reverts the POD container resources to original values", func() {
JustBeforeEach(func() {
err = bpod.RevertResourceBoost(pod)
})
When("POD is missing startup-cpu-boost annotation", func() {
BeforeEach(func() {
delete(pod.ObjectMeta.Annotations, bpod.BoostAnnotationKey)
err = bpod.RevertResourceBoost(pod)
})
It("errors", func() {
Expect(err).Should(HaveOccurred())
})
})
When("POD has valid startup-cpu-boost annotation", func() {
BeforeEach(func() {
err = bpod.RevertResourceBoost(pod)
})
It("does not error", func() {
Expect(err).ShouldNot(HaveOccurred())
})
Expand All @@ -115,14 +73,41 @@ var _ = Describe("Pod", func() {
It("reverts CPU requests to initial values", func() {
cpuReqOne := pod.Spec.Containers[0].Resources.Requests[corev1.ResourceCPU]
cpuReqTwo := pod.Spec.Containers[1].Resources.Requests[corev1.ResourceCPU]
Expect(cpuReqOne.String()).Should(Equal(annot.InitCPURequests[containerOne]))
Expect(cpuReqTwo.String()).Should(Equal(annot.InitCPURequests[containerTwo]))
Expect(cpuReqOne.String()).Should(Equal(annot.InitCPURequests[containerOneName]))
Expect(cpuReqTwo.String()).Should(Equal(annot.InitCPURequests[containerTwoName]))
})
It("reverts CPU limits to initial values", func() {
cpuReqOne := pod.Spec.Containers[0].Resources.Limits[corev1.ResourceCPU]
cpuReqTwo := pod.Spec.Containers[1].Resources.Limits[corev1.ResourceCPU]
Expect(cpuReqOne.String()).Should(Equal(annot.InitCPULimits[containerOne]))
Expect(cpuReqTwo.String()).Should(Equal(annot.InitCPULimits[containerTwo]))
Expect(cpuReqOne.String()).Should(Equal(annot.InitCPULimits[containerOneName]))
Expect(cpuReqTwo.String()).Should(Equal(annot.InitCPULimits[containerTwoName]))
})
When("Limits were removed during boost", func() {
BeforeEach(func() {
pod.Spec.Containers[0].Resources.Limits = nil
pod.Spec.Containers[1].Resources.Limits = nil
})
It("does not error", func() {
Expect(err).ShouldNot(HaveOccurred())
})
It("removes startup-cpu-boost label", func() {
Expect(pod.Labels).NotTo(HaveKey(bpod.BoostLabelKey))
})
It("removes startup-cpu-boost annotation", func() {
Expect(pod.Annotations).NotTo(HaveKey(bpod.BoostAnnotationKey))
})
It("reverts CPU requests to initial values", func() {
cpuReqOne := pod.Spec.Containers[0].Resources.Requests[corev1.ResourceCPU]
cpuReqTwo := pod.Spec.Containers[1].Resources.Requests[corev1.ResourceCPU]
Expect(cpuReqOne.String()).Should(Equal(annot.InitCPURequests[containerOneName]))
Expect(cpuReqTwo.String()).Should(Equal(annot.InitCPURequests[containerTwoName]))
})
It("reverts CPU limits to initial values", func() {
cpuReqOne := pod.Spec.Containers[0].Resources.Limits[corev1.ResourceCPU]
cpuReqTwo := pod.Spec.Containers[1].Resources.Limits[corev1.ResourceCPU]
Expect(cpuReqOne.String()).Should(Equal(annot.InitCPULimits[containerOneName]))
Expect(cpuReqTwo.String()).Should(Equal(annot.InitCPULimits[containerTwoName]))
})
})
})
})
Expand Down
30 changes: 22 additions & 8 deletions internal/webhook/podcpuboost_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,27 @@ func (h *podCPUBoostHandler) boostContainerResources(ctx context.Context, b boos
log.Info("skipping container due to restart policy")
continue
}
updateBoostAnnotation(annotation, container.Name, container.Resources)
if !hasResourcesToIncrease(container) {
log.Info("skipping container due to lack of resources to increase")
continue
}
resources := policy.NewResources(ctx, &container)
log = log.WithValues(
"newCpuRequests", resources.Requests.Cpu().String(),
"newCpuLimits", resources.Limits.Cpu().String(),
)
if h.removeLimits {
delete(resources.Limits, corev1.ResourceCPU)
if !resources.Requests.Cpu().IsZero() {
log = log.WithValues(
"newCpuRequests", resources.Requests.Cpu().String(),
)
}
if !resources.Limits.Cpu().IsZero() {
if h.removeLimits {
delete(resources.Limits, corev1.ResourceCPU)
log = log.WithValues("newCpuLimits", "<removed>")
} else {
log = log.WithValues("newCpuLimits", resources.Limits.Cpu().String())
}
}
updateBoostAnnotation(annotation, container.Name, container.Resources)
pod.Spec.Containers[i].Resources = *resources
log.Info("pod resources increased")
log.Info("container resources increased")
}
if len(annotation.InitCPULimits) > 0 || len(annotation.InitCPURequests) > 0 {
if pod.Annotations == nil {
Expand All @@ -109,6 +119,10 @@ func (h *podCPUBoostHandler) boostContainerResources(ctx context.Context, b boos
}
}

func hasResourcesToIncrease(c corev1.Container) bool {
return !c.Resources.Requests.Cpu().IsZero() || !c.Resources.Limits.Cpu().IsZero()
}

func updateBoostAnnotation(annot *bpod.BoostPodAnnotation, containerName string, resources corev1.ResourceRequirements) {
if cpuRequests, ok := resources.Requests[corev1.ResourceCPU]; ok {
annot.InitCPURequests[containerName] = cpuRequests.String()
Expand Down

0 comments on commit fdbc102

Please sign in to comment.