feat: spec updates handled and reflected by boost manager (#89)

Author: mikouaj

Diff for: internal/boost/manager.go

@@ -21,6 +21,7 @@ import (
 	"sync"
 	"time"
 
+	autoscaling "github.com/google/kube-startup-cpu-boost/api/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -46,6 +47,8 @@ type Manager interface {
 	AddStartupCPUBoost(ctx context.Context, boost StartupCPUBoost) error
 	// RemoveStartupCPUBoost removes a startup-cpu-boost from a manager
 	RemoveStartupCPUBoost(ctx context.Context, namespace, name string)
+	// UpdateStartupCPUBoost updates a startup-cpu-boost in a manager
+	UpdateStartupCPUBoost(ctx context.Context, spec *autoscaling.StartupCPUBoost) error
 	// StartupCPUBoost returns a startup-cpu-boost with a given name and namespace
 	StartupCPUBoostForPod(ctx context.Context, pod *corev1.Pod) (StartupCPUBoost, bool)
 	// StartupCPUBoostForPod returns a startup-cpu-boost that matches a given pod
@@ -141,6 +144,23 @@ func (m *managerImpl) RemoveStartupCPUBoost(ctx context.Context, namespace, name
 	log.Info("boost deleted successfully")
 }
 
+func (m *managerImpl) UpdateStartupCPUBoost(ctx context.Context, spec *autoscaling.StartupCPUBoost) error {
+	m.Lock()
+	defer m.Unlock()
+	log := m.log.WithValues("boost", spec.ObjectMeta.Name, "namespace", spec.ObjectMeta.Namespace)
+	log.V(5).Info("handling boost update")
+	boost, ok := m.getStartupCPUBoost(spec.ObjectMeta.Namespace, spec.ObjectMeta.Name)
+	if !ok {
+		log.V(5).Info("boost object not found")
+		return nil
+	}
+	if err := boost.UpdateFromSpec(ctx, spec); err != nil {
+		return err
+	}
+	log.Info("boost updated successfully")
+	return nil
+}
+
 // StartupCPUBoost returns a startup-cpu-boost with a given name and namespace
 // if registered in a manager.
 func (m *managerImpl) StartupCPUBoost(namespace string, name string) (StartupCPUBoost, bool) {

Diff for: internal/boost/manager_test.go

@@ -20,6 +20,7 @@ import (
 
 	autoscaling "github.com/google/kube-startup-cpu-boost/api/v1alpha1"
 	cpuboost "github.com/google/kube-startup-cpu-boost/internal/boost"
+	"github.com/google/kube-startup-cpu-boost/internal/boost/duration"
 	"github.com/google/kube-startup-cpu-boost/internal/metrics"
 	"github.com/google/kube-startup-cpu-boost/internal/mock"
 	. "github.com/onsi/ginkgo/v2"
@@ -107,6 +108,44 @@ var _ = Describe("Manager", func() {
 			})
 		})
 	})
+	Describe("updates startup-cpu-boost from spec", func() {
+		var (
+			boost       cpuboost.StartupCPUBoost
+			err         error
+			spec        *autoscaling.StartupCPUBoost
+			updatedSpec *autoscaling.StartupCPUBoost
+		)
+		BeforeEach(func() {
+			spec = specTemplate.DeepCopy()
+			updatedSpec = spec.DeepCopy()
+			updatedSpec.Spec.DurationPolicy.Fixed = &autoscaling.FixedDurationPolicy{
+				Unit:  autoscaling.FixedDurationPolicyUnitMin,
+				Value: 1000,
+			}
+		})
+		JustBeforeEach(func() {
+			boost, err = cpuboost.NewStartupCPUBoost(nil, spec)
+			Expect(err).ToNot(HaveOccurred())
+		})
+		When("startup-cpu-boost is registered", func() {
+			JustBeforeEach(func() {
+				err = manager.AddStartupCPUBoost(context.TODO(), boost)
+				Expect(err).ToNot(HaveOccurred())
+				err = manager.UpdateStartupCPUBoost(context.TODO(), updatedSpec)
+				Expect(err).ToNot(HaveOccurred())
+			})
+			It("updates the startup-cpu-boost", func() {
+				boost, ok := manager.StartupCPUBoost(updatedSpec.Namespace, updatedSpec.Name)
+				Expect(ok).To(BeTrue())
+				durationPolicies := boost.DurationPolicies()
+				durationPolicy, ok := durationPolicies[duration.FixedDurationPolicyName]
+				Expect(ok).To(BeTrue())
+				Expect(durationPolicy).To(BeAssignableToTypeOf(&duration.FixedDurationPolicy{}))
+				fixedDurationPolicy := durationPolicy.(*duration.FixedDurationPolicy)
+				Expect(fixedDurationPolicy.Duration()).To(Equal(1000 * time.Minute))
+			})
+		})
+	})
 	Describe("retrieves startup-cpu-boost for a POD", func() {
 		var (
 			pod               *corev1.Pod

Diff for: internal/boost/startupcpuboost.go

@@ -60,6 +60,8 @@ type StartupCPUBoost interface {
 	Matches(pod *corev1.Pod) bool
 	// Stats returns the StartupCPUBoost usage statistics
 	Stats() StartupCPUBoostStats
+	// UpdateFromSpec updates the StartupCPUBoost from the API spec
+	UpdateFromSpec(ctx context.Context, boost *autoscaling.StartupCPUBoost) error
 }
 
 const (
@@ -226,6 +228,26 @@ func (b *StartupCPUBoostImpl) Stats() StartupCPUBoostStats {
 	return b.stats
 }
 
+// UpdateFromSpec updates the StartupCPUBoost from the API spec
+func (b *StartupCPUBoostImpl) UpdateFromSpec(ctx context.Context, boost *autoscaling.StartupCPUBoost) error {
+	b.Lock()
+	defer b.Unlock()
+	log := b.loggerFromContext(ctx)
+	log.V(5).Info("handling boost update from API spec")
+	selector, err := metav1.LabelSelectorAsSelector(&boost.Selector)
+	if err != nil {
+		return err
+	}
+	resourcePolicies, err := mapResourcePolicy(boost.Spec.ResourcePolicy)
+	if err != nil {
+		return err
+	}
+	b.selector = selector
+	b.resourcePolicies = resourcePolicies
+	b.durationPolicies = mapDurationPolicy(boost.Spec.DurationPolicy)
+	return nil
+}
+
 // loggerFromContext provides Logger from a current context with configured
 // values common for startup-cpu-boost like name or namespace
 func (b *StartupCPUBoostImpl) loggerFromContext(ctx context.Context) logr.Logger {

Diff for: internal/boost/startupcpuboost_test.go

@@ -299,4 +299,113 @@ var _ = Describe("StartupCPUBoost", func() {
 			})
 		})
 	})
+	Describe("Updates boost from the spec", func() {
+		var (
+			updatedSpec *autoscaling.StartupCPUBoost
+		)
+		BeforeEach(func() {
+			spec.Selector = metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"app": "test",
+				},
+			}
+			spec.Spec.DurationPolicy.Fixed = &autoscaling.FixedDurationPolicy{
+				Unit:  autoscaling.FixedDurationPolicyUnitMin,
+				Value: 2,
+			}
+			spec.Spec.DurationPolicy.PodCondition = &autoscaling.PodConditionDurationPolicy{
+				Status: corev1.ConditionTrue,
+				Type:   corev1.PodReady,
+			}
+			updatedSpec = spec.DeepCopy()
+		})
+		JustBeforeEach(func() {
+			boost, err = cpuboost.NewStartupCPUBoost(nil, spec)
+			Expect(err).ShouldNot(HaveOccurred())
+			err = boost.UpdateFromSpec(context.TODO(), updatedSpec)
+		})
+		When("selector is changed", func() {
+			var (
+				podToSelect *corev1.Pod
+			)
+			BeforeEach(func() {
+				updatedSpec.Selector = metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"app": "newApp",
+					},
+				}
+				podToSelect = &corev1.Pod{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test-pod",
+						Namespace: specTemplate.Namespace,
+						Labels: map[string]string{
+							"app": "newApp",
+						}}}
+			})
+			It("matches pod with new selector", func() {
+				Expect(boost.Matches(podToSelect)).To(BeTrue())
+			})
+		})
+		When("duration policy is changed", func() {
+			var (
+				durationPolicies map[string]duration.Policy
+			)
+			BeforeEach(func() {
+				updatedSpec.Spec.DurationPolicy.Fixed = &autoscaling.FixedDurationPolicy{
+					Unit:  autoscaling.FixedDurationPolicyUnitMin,
+					Value: 1000,
+				}
+				updatedSpec.Spec.DurationPolicy.PodCondition = &autoscaling.PodConditionDurationPolicy{
+					Type:   corev1.PodInitialized,
+					Status: corev1.ConditionTrue,
+				}
+			})
+			JustBeforeEach(func() {
+				durationPolicies = boost.DurationPolicies()
+			})
+			It("has valid fixed duration policy", func() {
+				durationPolicy := durationPolicies[duration.FixedDurationPolicyName]
+				Expect(durationPolicy).To(BeAssignableToTypeOf(&duration.FixedDurationPolicy{}))
+				fixedDurationPolicy := durationPolicy.(*duration.FixedDurationPolicy)
+				Expect(fixedDurationPolicy.Duration()).To(Equal(1000 * time.Minute))
+			})
+			It("has valid pod condition policy", func() {
+				durationPolicy := durationPolicies[duration.PodConditionPolicyName]
+				Expect(durationPolicy).To(BeAssignableToTypeOf(&duration.PodConditionPolicy{}))
+				podConditionDurationPolicy := durationPolicy.(*duration.PodConditionPolicy)
+				Expect(podConditionDurationPolicy.Condition()).To(Equal(corev1.PodInitialized))
+				Expect(podConditionDurationPolicy.Status()).To(Equal(corev1.ConditionTrue))
+			})
+		})
+		When("resource policy is changed", func() {
+			var (
+				resourcePolicy      resource.ContainerPolicy
+				resourcePolicyFound bool
+			)
+			BeforeEach(func() {
+				updatedSpec.Spec.ResourcePolicy = autoscaling.ResourcePolicy{
+					ContainerPolicies: []autoscaling.ContainerPolicy{
+						{
+							ContainerName: "test",
+							PercentageIncrease: &autoscaling.PercentageIncrease{
+								Value: 1000,
+							},
+						},
+					},
+				}
+
+			})
+			JustBeforeEach(func() {
+				resourcePolicy, resourcePolicyFound = boost.ResourcePolicy("test")
+			})
+			It("finds resource policy", func() {
+				Expect(resourcePolicyFound).To(BeTrue())
+			})
+			It("has valid resource policy", func() {
+				Expect(resourcePolicy).To(BeAssignableToTypeOf(&resource.PercentageContainerPolicy{}))
+				percentagePolicy := resourcePolicy.(*resource.PercentageContainerPolicy)
+				Expect(percentagePolicy.Percentage()).To(Equal(int64(1000)))
+			})
+		})
+	})
 })

Diff for: internal/controller/boost_controller.go

@@ -150,6 +150,10 @@ func (r *StartupCPUBoostReconciler) Update(e event.UpdateEvent) bool {
 	}
 	log := r.Log.WithValues("name", boostObj.Name, "namespace", boostObj.Namespace)
 	log.V(5).Info("handling boost update event")
+	ctx := ctrl.LoggerInto(context.Background(), log)
+	if err := r.Manager.UpdateStartupCPUBoost(ctx, boostObj); err != nil {
+		log.Error(err, "boost update error")
+	}
 	return true
 }
 

Diff for: internal/controller/boost_controller_test.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 )
 
 var _ = Describe("BoostController", func() {
@@ -141,4 +142,29 @@ var _ = Describe("BoostController", func() {
 			})
 		})
 	})
+	Describe("receives update event", func() {
+		var (
+			updateEvent event.UpdateEvent
+			mgrMockCall *gomock.Call
+		)
+		BeforeEach(func() {
+			updateEvent = event.UpdateEvent{
+				ObjectNew: &autoscaling.StartupCPUBoost{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "boost-001",
+						Namespace: "demo",
+					},
+				},
+			}
+			mgrMockCall = mockManager.EXPECT().UpdateStartupCPUBoost(
+				gomock.Any(), gomock.Eq(updateEvent.ObjectNew))
+		})
+		JustBeforeEach(func() {
+			ok := boostCtrl.Update(updateEvent)
+			Expect(ok).To(BeTrue())
+		})
+		It("calls manager with valid update", func() {
+			mgrMockCall.Times(1)
+		})
+	})
 })