From 8678a00d3e8e2fbb3362c6e35be1b419cd0e437d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Stefaniak?= Date: Tue, 21 May 2024 15:33:51 +0200 Subject: [PATCH] feat: status and conditions for StartupCPUBoost (#39) --- api/v1alpha1/startupcpuboost_types.go | 19 ++- api/v1alpha1/zz_generated.deepcopy.go | 12 +- cmd/main.go | 9 +- ...autoscaling.x-k8s.io_startupcpuboosts.yaml | 89 +++++++++++ hack/boilerplate.go.txt | 2 +- internal/boost/manager.go | 45 +++++- internal/boost/manager_test.go | 17 ++- internal/boost/startupcpuboost.go | 135 ++++++++++++---- internal/boost/startupcpuboost_test.go | 16 +- ...oost_controller.go => boost_controller.go} | 79 +++++++--- internal/controller/boost_controller_test.go | 144 ++++++++++++++++++ internal/controller/boost_pod_handler.go | 31 +++- internal/controller/boost_pod_handler_test.go | 136 +++++++++++++---- internal/mock/boost_manager.go | 16 +- internal/mock/k8s_subresourcewriter.go | 112 ++++++++++++++ internal/mock/reconciler.go | 70 +++++++++ internal/mock/startupcpuboost.go | 17 ++- 17 files changed, 847 insertions(+), 102 deletions(-) rename internal/controller/{startupcpuboost_controller.go => boost_controller.go} (58%) create mode 100644 internal/controller/boost_controller_test.go create mode 100644 internal/mock/k8s_subresourcewriter.go create mode 100644 internal/mock/reconciler.go diff --git a/api/v1alpha1/startupcpuboost_types.go b/api/v1alpha1/startupcpuboost_types.go index 6626b07..0992148 100644 --- a/api/v1alpha1/startupcpuboost_types.go +++ b/api/v1alpha1/startupcpuboost_types.go @@ -117,8 +117,23 @@ type StartupCPUBoostSpec struct { // StartupCPUBoostStatus defines the observed state of StartupCPUBoost type StartupCPUBoostStatus struct { - // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster - // Important: Run "make" to regenerate code after modifying this file + // activeContainerBoosts is the number of containers which CPU + // resources were increased by the StartupCPUBoost and not yet + // reverted back to the original values + // +kubebuilder:validation:Optional + ActiveContainerBoosts int32 `json:"activeContainerBoosts,omitempty"` + // totalContainerBoosts is the number of containers which CPU + // resources were increased by the StartupCPUBoost + // +kubebuilder:validation:Optional + TotalContainerBoosts int32 `json:"totalContainerBoosts,omitempty"` + // Conditions hold the latest available observations of the StartupCPUBoost + // current state. + // +optional + // +listType=map + // +listMapKey=type + // +patchStrategy=merge + // +patchMergeKey=type + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` } //+kubebuilder:object:root=true diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index d9c8a18..10ae163 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -1,6 +1,6 @@ //go:build !ignore_autogenerated -// Copyright 2023 Google LLC +// Copyright 2024 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ package v1alpha1 import ( + "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -163,7 +164,7 @@ func (in *StartupCPUBoost) DeepCopyInto(out *StartupCPUBoost) { in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Selector.DeepCopyInto(&out.Selector) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StartupCPUBoost. @@ -236,6 +237,13 @@ func (in *StartupCPUBoostSpec) DeepCopy() *StartupCPUBoostSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *StartupCPUBoostStatus) DeepCopyInto(out *StartupCPUBoostStatus) { *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StartupCPUBoostStatus. diff --git a/cmd/main.go b/cmd/main.go index 00e9605..5ae8c14 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -139,13 +139,14 @@ func setupControllers(mgr ctrl.Manager, boostMgr boost.Manager, certsReady chan } cpuBoostWebHook := boostWebhook.NewPodCPUBoostWebHook(boostMgr, scheme) mgr.GetWebhookServer().Register("/mutate-v1-pod", cpuBoostWebHook) - - if err := (&controller.StartupCPUBoostReconciler{ + boostCtrl := &controller.StartupCPUBoostReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), - Log: ctrl.Log.WithName("startup-cpu-boost-reconciler"), + Log: ctrl.Log.WithName("boost-reconciler"), Manager: boostMgr, - }).SetupWithManager(mgr); err != nil { + } + boostMgr.SetStartupCPUBoostReconciler(boostCtrl) + if err := boostCtrl.SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "StartupCPUBoost") os.Exit(1) } diff --git a/config/crd/bases/autoscaling.x-k8s.io_startupcpuboosts.yaml b/config/crd/bases/autoscaling.x-k8s.io_startupcpuboosts.yaml index 8de9384..56ecaf2 100644 --- a/config/crd/bases/autoscaling.x-k8s.io_startupcpuboosts.yaml +++ b/config/crd/bases/autoscaling.x-k8s.io_startupcpuboosts.yaml @@ -170,6 +170,95 @@ spec: type: object status: description: StartupCPUBoostStatus defines the observed state of StartupCPUBoost + properties: + activeContainerBoosts: + description: |- + activeContainerBoosts is the number of containers which CPU + resources were increased by the StartupCPUBoost and not yet + reverted back to the original values + format: int32 + type: integer + conditions: + description: |- + Conditions hold the latest available observations of the StartupCPUBoost + current state. + items: + description: "Condition contains details for one aspect of the current + state of this API Resource.\n---\nThis struct is intended for + direct use as an array at the field path .status.conditions. For + example,\n\n\n\ttype FooStatus struct{\n\t // Represents the + observations of a foo's current state.\n\t // Known .status.conditions.type + are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // + +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t + \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" + patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t + \ // other fields\n\t}" + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: |- + type of condition in CamelCase or in foo.example.com/CamelCase. + --- + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be + useful (see .node.status.conditions), the ability to deconflict is important. + The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + totalContainerBoosts: + description: |- + totalContainerBoosts is the number of containers which CPU + resources were increased by the StartupCPUBoost + format: int32 + type: integer type: object type: object served: true diff --git a/hack/boilerplate.go.txt b/hack/boilerplate.go.txt index 4916f45..5b7625c 100644 --- a/hack/boilerplate.go.txt +++ b/hack/boilerplate.go.txt @@ -1,4 +1,4 @@ -// Copyright 2023 Google LLC +// Copyright 2024 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/internal/boost/manager.go b/internal/boost/manager.go index babd0bd..d9b62c1 100644 --- a/internal/boost/manager.go +++ b/internal/boost/manager.go @@ -22,7 +22,9 @@ import ( "time" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/go-logr/logr" "github.com/google/kube-startup-cpu-boost/internal/boost/duration" @@ -47,7 +49,7 @@ type Manager interface { StartupCPUBoostForPod(ctx context.Context, pod *corev1.Pod) (StartupCPUBoost, bool) // StartupCPUBoostForPod returns a startup-cpu-boost that matches a given pod StartupCPUBoost(namespace, name string) (StartupCPUBoost, bool) - // Start runs the manager's control loop + SetStartupCPUBoostReconciler(reconciler reconcile.Reconciler) Start(ctx context.Context) error } @@ -77,6 +79,7 @@ func newTimeTickerImpl(d time.Duration) TimeTicker { type managerImpl struct { sync.RWMutex client client.Client + reconciler reconcile.Reconciler ticker TimeTicker checkInterval time.Duration startupCPUBoosts map[string]map[string]StartupCPUBoost @@ -158,7 +161,10 @@ func (m *managerImpl) StartupCPUBoostForPod(ctx context.Context, pod *corev1.Pod return nil, false } -// Start runs the manager's control loop +func (m *managerImpl) SetStartupCPUBoostReconciler(reconciler reconcile.Reconciler) { + m.reconciler = reconciler +} + func (m *managerImpl) Start(ctx context.Context) error { log := m.loggerFromContext(ctx) defer m.ticker.Stop() @@ -209,6 +215,7 @@ func (m *managerImpl) validateTimePolicyBoosts(ctx context.Context) { m.RLock() defer m.RUnlock() revertTasks := make(chan *podRevertTask, m.maxGoroutines) + reconcileTasks := make(chan *reconcile.Request, m.maxGoroutines) errors := make(chan error, m.maxGoroutines) log := m.loggerFromContext(ctx) @@ -235,15 +242,33 @@ func (m *managerImpl) validateTimePolicyBoosts(ctx context.Context) { log.V(5).Info("updating pod with initial resources") if err := task.boost.RevertResources(ctx, task.pod); err != nil { errors <- fmt.Errorf("pod %s/%s: %w", task.pod.Namespace, task.pod.Name, err) + } else { + reconcileTasks <- &reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: task.boost.Name(), + Namespace: task.boost.Namespace(), + }, + } } } }() } wg.Wait() + close(reconcileTasks) close(errors) }() - for err := range errors { - log.Error(err, "failed to revert resources") + + go func() { + for err := range errors { + log.Error(err, "failed to revert resources") + } + }() + + reconcileRequests := dedupeReconcileRequests(reconcileTasks) + if m.reconciler != nil { + for _, req := range reconcileRequests { + m.reconciler.Reconcile(ctx, req) + } } } @@ -252,3 +277,15 @@ func (m *managerImpl) loggerFromContext(ctx context.Context) logr.Logger { return ctrl.LoggerFrom(ctx). WithName("boost-manager") } + +func dedupeReconcileRequests(reconcileTasks chan *reconcile.Request) []reconcile.Request { + result := make([]reconcile.Request, 0, len(reconcileTasks)) + requests := make(map[reconcile.Request]bool) + for task := range reconcileTasks { + requests[*task] = true + } + for k := range requests { + result = append(result, k) + } + return result +} diff --git a/internal/boost/manager_test.go b/internal/boost/manager_test.go index 190f73b..b141481 100644 --- a/internal/boost/manager_test.go +++ b/internal/boost/manager_test.go @@ -26,6 +26,8 @@ import ( "go.uber.org/mock/gomock" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/reconcile" ) var _ = Describe("Manager", func() { @@ -192,11 +194,12 @@ var _ = Describe("Manager", func() { }) When("There are startup-cpu-boosts with fixed duration policy", func() { var ( - spec *autoscaling.StartupCPUBoost - boost cpuboost.StartupCPUBoost - pod *corev1.Pod - mockClient *mock.MockClient - c chan time.Time + spec *autoscaling.StartupCPUBoost + boost cpuboost.StartupCPUBoost + pod *corev1.Pod + mockClient *mock.MockClient + mockReconciler *mock.MockReconciler + c chan time.Time ) BeforeEach(func() { spec = specTemplate.DeepCopy() @@ -209,13 +212,17 @@ var _ = Describe("Manager", func() { creationTimestamp := time.Now().Add(-1 * time.Duration(seconds) * time.Second).Add(-1 * time.Minute) pod.CreationTimestamp = metav1.NewTime(creationTimestamp) mockClient = mock.NewMockClient(mockCtrl) + mockReconciler = mock.NewMockReconciler(mockCtrl) c = make(chan time.Time, 1) mockTicker.EXPECT().Tick().MinTimes(1).Return(c) mockTicker.EXPECT().Stop().Return() mockClient.EXPECT().Update(gomock.Any(), gomock.Eq(pod)).MinTimes(1).Return(nil) + reconcileReq := reconcile.Request{NamespacedName: types.NamespacedName{Name: spec.Name, Namespace: spec.Namespace}} + mockReconciler.EXPECT().Reconcile(gomock.Any(), gomock.Eq(reconcileReq)).Times(1) }) JustBeforeEach(func() { + manager.SetStartupCPUBoostReconciler(mockReconciler) boost, err = cpuboost.NewStartupCPUBoost(mockClient, spec) Expect(err).ShouldNot(HaveOccurred()) err = boost.UpsertPod(ctx, pod) diff --git a/internal/boost/startupcpuboost.go b/internal/boost/startupcpuboost.go index b35a508..a21c204 100644 --- a/internal/boost/startupcpuboost.go +++ b/internal/boost/startupcpuboost.go @@ -36,16 +36,52 @@ import ( // StartupCPUBoost is an implementation of a StartupCPUBoost CRD type StartupCPUBoost interface { + // Name returns startup-cpu-boost name Name() string + // Namespace returns startup-cpu-boost namespace Namespace() string + // ResourcePolicy returns the resource policy for a given container ResourcePolicy(containerName string) (resource.ContainerPolicy, bool) + // DurationPolicies returns configured duration policies DurationPolicies() map[string]duration.Policy + // Pod returns a POD if tracked by startup-cpu-boost Pod(name string) (*corev1.Pod, bool) + // UpsertPod inserts new or updates existing POD to startup-cpu-boost tracking UpsertPod(ctx context.Context, pod *corev1.Pod) error + // DeletePod removes the POD from the startup-cpu-boost tracking DeletePod(ctx context.Context, pod *corev1.Pod) error + // ValidatePolicy validates policy with a given name on all startup-cpu-boost PODs. ValidatePolicy(ctx context.Context, name string) []*corev1.Pod + // RevertResources updates POD's container resource requests and limits to their original + // values using the data from StartupCPUBoost annotation RevertResources(ctx context.Context, pod *corev1.Pod) error + // Matches verifies if a boost selector matches the given POD Matches(pod *corev1.Pod) bool + // Stats returns the StartupCPUBoost usage statistics + Stats() StartupCPUBoostStats +} + +const ( + StartupCPUBoostStatsPodCreateEvent = 1 + StartupCPUBoostStatsPodUpdateEvent = 2 + StartupCPUBoostStatsPodDeleteEvent = 3 +) + +type StartupCPUBoostStatsEventType int32 + +type StartupCPUBoostStatsEvent struct { + Type StartupCPUBoostStatsEventType + Object interface{} +} + +// StartupCPUBoostStats holds the StartupCPUBoost usage statistics +type StartupCPUBoostStats struct { + // activeContainerBoosts is a number of a containers which CPU resources + // were increased (boosted) and not yet reverted to their original values + ActiveContainerBoosts int + // totalContainerBoosts is a number of a containers which CPU resources + // were increased (boosted) + TotalContainerBoosts int } // StartupCPUBoostImpl is an implementation of a StartupCPUBoost CRD @@ -56,8 +92,9 @@ type StartupCPUBoostImpl struct { selector labels.Selector durationPolicies map[string]duration.Policy resourcePolicies map[string]resource.ContainerPolicy - pods sync.Map + pods map[string]*corev1.Pod client client.Client + stats StartupCPUBoostStats } // NewStartupCPUBoost constructs startup-cpu-boost implementation from a given API spec @@ -76,7 +113,9 @@ func NewStartupCPUBoost(client client.Client, boost *autoscaling.StartupCPUBoost selector: selector, durationPolicies: mapDurationPolicy(boost.Spec.DurationPolicy), resourcePolicies: resourcePolicies, + pods: make(map[string]*corev1.Pod), client: client, + stats: StartupCPUBoostStats{}, }, nil } @@ -103,22 +142,27 @@ func (b *StartupCPUBoostImpl) DurationPolicies() map[string]duration.Policy { // Pod returns a POD if tracked by startup-cpu-boost. func (b *StartupCPUBoostImpl) Pod(name string) (*corev1.Pod, bool) { - if v, ok := b.pods.Load(name); ok { - return v.(*corev1.Pod), ok - } - return nil, false + b.RLock() + defer b.RUnlock() + pod, ok := b.pods[name] + return pod, ok } // UpsertPod inserts new or updates existing POD to startup-cpu-boost tracking // The update of existing POD triggers validation logic and may result in POD update func (b *StartupCPUBoostImpl) UpsertPod(ctx context.Context, pod *corev1.Pod) error { + b.Lock() + defer b.Unlock() log := b.loggerFromContext(ctx).WithValues("pod", pod.Name) log.V(5).Info("upserting a pod") - if _, loaded := b.pods.Swap(pod.Name, pod); !loaded { - log.V(5).Info("inserted non-existing pod") - return nil + _, existing := b.pods[pod.Name] + b.pods[pod.Name] = pod + statsEvent := StartupCPUBoostStatsEvent{StartupCPUBoostStatsPodCreateEvent, pod} + if existing { + statsEvent.Type = StartupCPUBoostStatsPodUpdateEvent } - log.V(5).Info("updating existing pod") + b.updateStats(statsEvent) + condPolicy, ok := b.durationPolicies[duration.PodConditionPolicyName] if !ok { log.V(5).Info("skipping pod update as podCondition policy is missing") @@ -126,7 +170,7 @@ func (b *StartupCPUBoostImpl) UpsertPod(ctx context.Context, pod *corev1.Pod) er } if valid := b.validatePolicyOnPod(ctx, condPolicy, pod); !valid { log.V(2).Info("updating pod with initial resources") - if err := b.RevertResources(ctx, pod); err != nil { + if err := b.revertResources(ctx, pod); err != nil { return fmt.Errorf("failed to update pod: %s", err) } } @@ -135,43 +179,39 @@ func (b *StartupCPUBoostImpl) UpsertPod(ctx context.Context, pod *corev1.Pod) er // DeletePod removes the POD from the startup-cpu-boost tracking func (b *StartupCPUBoostImpl) DeletePod(ctx context.Context, pod *corev1.Pod) error { + b.Lock() + defer b.Unlock() log := b.loggerFromContext(ctx).WithValues("pod", pod.Name) log.V(5).Info("handling pod delete") - if _, loaded := b.pods.LoadAndDelete(pod.Name); loaded { - log.Info("deletion of untracked pod") - } + delete(b.pods, pod.Name) + b.updateStats(StartupCPUBoostStatsEvent{StartupCPUBoostStatsPodDeleteEvent, pod}) return nil } // ValidatePolicy validates policy with a given name on all startup-cpu-boost PODs. // The function returns slice of PODs that violated the policy. func (b *StartupCPUBoostImpl) ValidatePolicy(ctx context.Context, name string) (violated []*corev1.Pod) { + b.RLock() + defer b.RUnlock() violated = make([]*corev1.Pod, 0) policy, ok := b.durationPolicies[name] if !ok { return } - b.pods.Range(func(key, value any) bool { - pod := value.(*corev1.Pod) + for _, pod := range b.pods { if !b.validatePolicyOnPod(ctx, policy, pod) { violated = append(violated, pod) } - return true - }) + } return } // RevertResources updates POD's container resource requests and limits to their original // values using the data from StartupCPUBoost annotation func (b *StartupCPUBoostImpl) RevertResources(ctx context.Context, pod *corev1.Pod) error { - if err := bpod.RevertResourceBoost(pod); err != nil { - return fmt.Errorf("failed to update pod spec: %s", err) - } - if err := b.client.Update(ctx, pod); err != nil { - return err - } - b.pods.Delete(pod.Name) - return nil + b.Lock() + defer b.Unlock() + return b.revertResources(ctx, pod) } // Matches verifies if a boost selector matches the given POD @@ -179,11 +219,16 @@ func (b *StartupCPUBoostImpl) Matches(pod *corev1.Pod) bool { return b.selector.Matches(labels.Set(pod.Labels)) } +// Stats returns the StartupCPUBoost usage statistics +func (b *StartupCPUBoostImpl) Stats() StartupCPUBoostStats { + return b.stats +} + // 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 { return ctrl.LoggerFrom(ctx). - WithName("startup-cpu-boost"). + WithName("boost"). WithValues( "name", b.name, "namespace", b.namespace, @@ -200,6 +245,44 @@ func (b *StartupCPUBoostImpl) validatePolicyOnPod(ctx context.Context, p duratio return } +// revertResources updates POD's container resource requests and limits to their original +// values using the data from StartupCPUBoost annotation +func (b *StartupCPUBoostImpl) revertResources(ctx context.Context, pod *corev1.Pod) error { + if err := bpod.RevertResourceBoost(pod); err != nil { + return fmt.Errorf("failed to update pod spec: %s", err) + } + if err := b.client.Update(ctx, pod); err != nil { + return err + } + delete(b.pods, pod.Name) + b.updateStats(StartupCPUBoostStatsEvent{StartupCPUBoostStatsPodDeleteEvent, pod}) + return nil +} + +// updateStats updates the StartupCPUBoost usage statistics based on the +// received update event +func (b *StartupCPUBoostImpl) updateStats(e StartupCPUBoostStatsEvent) { + var activeCnt int + for _, pod := range b.pods { + activeCnt += boostContainersLen(pod) + } + b.stats.ActiveContainerBoosts = activeCnt + switch e.Type { + case StartupCPUBoostStatsPodCreateEvent: + pod := e.Object.(*corev1.Pod) + b.stats.TotalContainerBoosts += boostContainersLen(pod) + } +} + +// boostContainersLen returns the number of containers that were boosted +// by StartupCPUBoost in a given Pod +func boostContainersLen(pod *corev1.Pod) (cnt int) { + if annot, err := bpod.BoostAnnotationFromPod(pod); err == nil { + return len(annot.InitCPURequests) + } + return +} + // mapDurationPolicy maps the Duration Policy from the API spec to the map of policy // implementations with policy name keys func mapDurationPolicy(policiesSpec autoscaling.DurationPolicy) map[string]duration.Policy { diff --git a/internal/boost/startupcpuboost_test.go b/internal/boost/startupcpuboost_test.go index 717d858..a4eb889 100644 --- a/internal/boost/startupcpuboost_test.go +++ b/internal/boost/startupcpuboost_test.go @@ -170,7 +170,6 @@ var _ = Describe("StartupCPUBoost", func() { }) }) }) - Describe("Upserts a POD", func() { var ( mockCtrl *gomock.Controller @@ -196,6 +195,11 @@ var _ = Describe("StartupCPUBoost", func() { Expect(ok).To(BeTrue()) Expect(p.Name).To(Equal(pod.Name)) }) + It("updates statistics", func() { + stats := boost.Stats() + Expect(stats.ActiveContainerBoosts).To(Equal(2)) + Expect(stats.TotalContainerBoosts).To(Equal(2)) + }) }) When("POD exists", func() { var existingPod *corev1.Pod @@ -219,6 +223,11 @@ var _ = Describe("StartupCPUBoost", func() { Expect(p.Name).To(Equal(pod.Name)) Expect(p.CreationTimestamp).To(Equal(createTimestamp)) }) + It("updates statistics", func() { + stats := boost.Stats() + Expect(stats.ActiveContainerBoosts).To(Equal(2)) + Expect(stats.TotalContainerBoosts).To(Equal(2)) + }) When("boost spec has pod condition policy", func() { BeforeEach(func() { spec.Spec.DurationPolicy.PodCondition = &autoscaling.PodConditionDurationPolicy{ @@ -270,6 +279,11 @@ var _ = Describe("StartupCPUBoost", func() { _, found := boost.Pod(pod.Name) Expect(found).To(BeFalse()) }) + It("updates statistics", func() { + stats := boost.Stats() + Expect(stats.ActiveContainerBoosts).To(Equal(0)) + Expect(stats.TotalContainerBoosts).To(Equal(2)) + }) }) }) }) diff --git a/internal/controller/startupcpuboost_controller.go b/internal/controller/boost_controller.go similarity index 58% rename from internal/controller/startupcpuboost_controller.go rename to internal/controller/boost_controller.go index e9f5c35..9b90208 100644 --- a/internal/controller/startupcpuboost_controller.go +++ b/internal/controller/boost_controller.go @@ -17,6 +17,9 @@ package controller import ( "context" + "k8s.io/apimachinery/pkg/api/equality" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" @@ -29,6 +32,14 @@ import ( autoscaling "github.com/google/kube-startup-cpu-boost/api/v1alpha1" "github.com/google/kube-startup-cpu-boost/internal/boost" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const ( + BoostActiveConditionTrueReason = "Ready" + BoostActiveConditionTrueMessage = "Can boost new containers" + BoostActiveConditionFalseReason = "NotFound" + BoostActiveConditionFalseMessage = "StartupCPUBoost not found" ) // StartupCPUBoostReconciler reconciles a StartupCPUBoost object @@ -46,20 +57,44 @@ type StartupCPUBoostReconciler struct { // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. -// TODO(user): Modify the Reconcile function to compare the state specified by -// the StartupCPUBoost object against the actual cluster state, and then -// perform operations to make the cluster state reflect the state specified by -// the user. -// -// For more details, check Reconcile and its Result here: -// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.14.4/pkg/reconcile func (r *StartupCPUBoostReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { var boostObj autoscaling.StartupCPUBoost - if err := r.Client.Get(ctx, req.NamespacedName, &boostObj); err != nil { + var err error + if err = r.Client.Get(ctx, req.NamespacedName, &boostObj); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + log := r.Log.WithName("reconcile").WithValues("name", boostObj.Name, "namespace", boostObj.Namespace) + log.V(2).Info("reconciling") + newBoostObj := boostObj.DeepCopy() + activeCondition := metav1.Condition{ + Type: "Active", + Status: metav1.ConditionFalse, + Reason: BoostActiveConditionFalseReason, + Message: BoostActiveConditionFalseMessage, + } + boost, ok := r.Manager.StartupCPUBoost(boostObj.Namespace, boostObj.Name) + if ok { + log.V(5).Info("found boost in a manager") + stats := boost.Stats() + activeCondition.Status = metav1.ConditionTrue + activeCondition.Reason = BoostActiveConditionTrueReason + activeCondition.Message = BoostActiveConditionTrueMessage + newBoostObj.Status.ActiveContainerBoosts = int32(stats.ActiveContainerBoosts) + newBoostObj.Status.TotalContainerBoosts = int32(stats.TotalContainerBoosts) + } + meta.SetStatusCondition(&newBoostObj.Status.Conditions, activeCondition) + if !equality.Semantic.DeepEqual(newBoostObj.Status, boostObj.Status) { + log.V(5).Info("updating status") + err = r.Client.Status().Update(ctx, newBoostObj) + } + if err != nil { + if apierrors.IsConflict(err) { + log.V(5).Info("status update conflict, requeueing") + return ctrl.Result{Requeue: true}, nil + } + log.V(5).Error(err, "failed to update status") return ctrl.Result{}, client.IgnoreNotFound(err) } - log := ctrl.LoggerFrom(ctx) - log.V(2).Info("Reconciling StartupCPUBoost") return ctrl.Result{}, nil } @@ -80,14 +115,14 @@ func (r *StartupCPUBoostReconciler) SetupWithManager(mgr ctrl.Manager) error { } func (r *StartupCPUBoostReconciler) Create(e event.CreateEvent) bool { - spec, ok := e.Object.(*autoscaling.StartupCPUBoost) + boostObj, ok := e.Object.(*autoscaling.StartupCPUBoost) if !ok { return true } - log := r.Log.WithValues("StartupCPUBoost", klog.KObj(spec)) - log.V(2).Info("handling startup-cpu-boost create") + log := r.Log.WithName("create").WithValues("name", boostObj.Name, "namespace", boostObj.Namespace) + log.V(2).Info("creating") ctx := ctrl.LoggerInto(context.Background(), log) - boost, err := boost.NewStartupCPUBoost(r.Client, spec) + boost, err := boost.NewStartupCPUBoost(r.Client, boostObj) if err != nil { log.Error(err, "failed to create startup-cpu-boost from spec") } @@ -102,21 +137,25 @@ func (r *StartupCPUBoostReconciler) Delete(e event.DeleteEvent) bool { if !ok { return true } - log := r.Log.WithValues("StartupCPUBoost", klog.KObj(e.Object)) - log.V(2).Info("handling startup-cpu-boost delete") + log := r.Log.WithName("delete").WithValues("name", boostObj.Name, "namespace", boostObj.Namespace) + log.V(2).Info("deleting") ctx := ctrl.LoggerInto(context.Background(), log) r.Manager.RemoveStartupCPUBoost(ctx, boostObj.Namespace, boostObj.Name) return true } func (r *StartupCPUBoostReconciler) Update(e event.UpdateEvent) bool { - log := r.Log.WithValues("StartupCPUBoost", klog.KObj(e.ObjectNew)) - log.V(2).Info("handling startup-cpu-boost update") + boostObj, ok := e.ObjectNew.(*autoscaling.StartupCPUBoost) + if !ok { + return true + } + log := r.Log.WithName("update").WithValues("name", boostObj.Name, "namespace", boostObj.Namespace) + log.V(2).Info("updating") return true } func (r *StartupCPUBoostReconciler) Generic(e event.GenericEvent) bool { - log := r.Log.WithValues("StartupCPUBoost", klog.KObj(e.Object)) - log.V(2).Info("handling startup-cpu-boost generic event") + log := r.Log.WithName("generic").WithValues("object", klog.KObj(e.Object)) + log.V(2).Info("handling generic event") return true } diff --git a/internal/controller/boost_controller_test.go b/internal/controller/boost_controller_test.go new file mode 100644 index 0000000..0a65455 --- /dev/null +++ b/internal/controller/boost_controller_test.go @@ -0,0 +1,144 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package controller_test + +import ( + "context" + + "github.com/go-logr/logr" + autoscaling "github.com/google/kube-startup-cpu-boost/api/v1alpha1" + "github.com/google/kube-startup-cpu-boost/internal/boost" + "github.com/google/kube-startup-cpu-boost/internal/controller" + "github.com/google/kube-startup-cpu-boost/internal/mock" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "go.uber.org/mock/gomock" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("BoostController", func() { + var ( + mockCtrl *gomock.Controller + mockClient *mock.MockClient + mockManager *mock.MockManager + mockBoost *mock.MockStartupCPUBoost + boostCtrl controller.StartupCPUBoostReconciler + ) + BeforeEach(func() { + mockCtrl = gomock.NewController(GinkgoT()) + mockClient = mock.NewMockClient(mockCtrl) + mockManager = mock.NewMockManager(mockCtrl) + mockBoost = mock.NewMockStartupCPUBoost(mockCtrl) + boostCtrl = controller.StartupCPUBoostReconciler{ + Log: logr.Discard(), + Client: mockClient, + Manager: mockManager, + } + }) + Describe("Receives reconcile request", func() { + var ( + req ctrl.Request + name string + namespace string + result ctrl.Result + err error + ) + BeforeEach(func() { + name = "boost-001" + namespace = "demo" + req = ctrl.Request{ + NamespacedName: types.NamespacedName{Name: name, Namespace: namespace}, + } + }) + JustBeforeEach(func() { + result, err = boostCtrl.Reconcile(context.TODO(), req) + }) + When("boost is registered in boost manager", func() { + var ( + totalContainerBoosts = 10 + activeContainerBoosts = 5 + activeConditionTrue = metav1.Condition{ + Type: "Active", + Status: metav1.ConditionTrue, + Reason: controller.BoostActiveConditionTrueReason, + Message: controller.BoostActiveConditionTrueMessage, + } + ) + BeforeEach(func() { + stats := boost.StartupCPUBoostStats{ + TotalContainerBoosts: totalContainerBoosts, + ActiveContainerBoosts: activeContainerBoosts, + } + mockManager.EXPECT().StartupCPUBoost(gomock.Eq(namespace), gomock.Eq(name)).Times(1).Return(mockBoost, true) + mockBoost.EXPECT().Stats().Times(1).Return(stats) + }) + When("there existing status is up to date", func() { + BeforeEach(func() { + mockClient.EXPECT().Get(gomock.Any(), gomock.Eq(req.NamespacedName), gomock.Any()). + Times(1).DoAndReturn(func(c context.Context, cc client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + boostObj := obj.(*autoscaling.StartupCPUBoost) + boostObj.Name = name + boostObj.Namespace = namespace + meta.SetStatusCondition(&boostObj.Status.Conditions, activeConditionTrue) + boostObj.Status.TotalContainerBoosts = int32(totalContainerBoosts) + boostObj.Status.ActiveContainerBoosts = int32(activeContainerBoosts) + return nil + }) + }) + It("does not error", func() { + Expect(err).To(BeNil()) + }) + It("returns empty result", func() { + Expect(result).To(Equal(ctrl.Result{})) + }) + }) + When("there existing status is not up to date", func() { + var mockSubResWriter *mock.MockSubResourceWriter + BeforeEach(func() { + mockSubResWriter = mock.NewMockSubResourceWriter(mockCtrl) + mockSubResWriter.EXPECT().Update( + gomock.Any(), + gomock.Cond(func(b any) bool { + boostObj := b.(*autoscaling.StartupCPUBoost) + ret := boostObj.Status.ActiveContainerBoosts == int32(activeContainerBoosts) + ret = ret && boostObj.Status.TotalContainerBoosts == int32(totalContainerBoosts) + ret = ret && boostObj.Name == name + ret = ret && boostObj.Namespace == namespace + return ret + })). + Return(nil).Times(1) + mockClient.EXPECT().Get(gomock.Any(), gomock.Eq(req.NamespacedName), gomock.Any()). + Times(1).DoAndReturn(func(c context.Context, cc client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + boostObj := obj.(*autoscaling.StartupCPUBoost) + boostObj.Name = name + boostObj.Namespace = namespace + return nil + }) + mockClient.EXPECT().Status().Return(mockSubResWriter).Times(1) + }) + It("does not error", func() { + Expect(err).To(BeNil()) + }) + It("returns empty result", func() { + Expect(result).To(Equal(ctrl.Result{})) + }) + }) + }) + }) +}) diff --git a/internal/controller/boost_pod_handler.go b/internal/controller/boost_pod_handler.go index 4b2ebd3..3416ce0 100644 --- a/internal/controller/boost_pod_handler.go +++ b/internal/controller/boost_pod_handler.go @@ -21,9 +21,12 @@ import ( "github.com/google/kube-startup-cpu-boost/internal/boost" bpod "github.com/google/kube-startup-cpu-boost/internal/boost/pod" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/workqueue" "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/reconcile" ) type BoostPodHandler interface { @@ -58,10 +61,17 @@ func (h *boostPodHandler) Create(ctx context.Context, e event.CreateEvent, wq wo log.V(5).Info("failed to get boost for pod") return } - log.WithValues("boost", boost.Name()) + boostName := boost.Name() + log.WithValues("boost", boostName) if err := boost.UpsertPod(ctx, pod); err != nil { log.Error(err, "failed to handle pod create") } + wq.Add(reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: boostName, + Namespace: boost.Namespace(), + }, + }) } func (h *boostPodHandler) Delete(ctx context.Context, e event.DeleteEvent, wq workqueue.RateLimitingInterface) { @@ -79,16 +89,25 @@ func (h *boostPodHandler) Delete(ctx context.Context, e event.DeleteEvent, wq wo if err := boost.DeletePod(ctx, pod); err != nil { log.Error(err, "failed to handle pod delete") } + wq.Add(reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: boost.Name(), + Namespace: boost.Namespace(), + }, + }) } func (h *boostPodHandler) Update(ctx context.Context, e event.UpdateEvent, wq workqueue.RateLimitingInterface) { pod, ok := e.ObjectNew.(*corev1.Pod) - if !ok { + oldPod, ok_ := e.ObjectOld.(*corev1.Pod) + if !ok || !ok_ { return } log := h.log.WithValues("pod", pod.Name, "namespace", pod.Namespace) log.V(5).Info("handling pod update") - //TODO react only on POD or container condition updates + if equality.Semantic.DeepEqual(pod.Status.Conditions, oldPod.Status.Conditions) { + return + } boost, ok := h.boostForPod(pod) if !ok { log.V(5).Info("failed to get boost for pod") @@ -97,6 +116,12 @@ func (h *boostPodHandler) Update(ctx context.Context, e event.UpdateEvent, wq wo if err := boost.UpsertPod(ctx, pod); err != nil { log.Error(err, "failed to handle pod update") } + wq.Add(reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: boost.Name(), + Namespace: boost.Namespace(), + }, + }) } func (h *boostPodHandler) Generic(ctx context.Context, e event.GenericEvent, wq workqueue.RateLimitingInterface) { diff --git a/internal/controller/boost_pod_handler_test.go b/internal/controller/boost_pod_handler_test.go index a4f283c..aa2c8f8 100644 --- a/internal/controller/boost_pod_handler_test.go +++ b/internal/controller/boost_pod_handler_test.go @@ -26,7 +26,9 @@ import ( "go.uber.org/mock/gomock" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/util/workqueue" "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/reconcile" ) var _ = Describe("BoostPodHandler", func() { @@ -35,10 +37,12 @@ var _ = Describe("BoostPodHandler", func() { mgrMock *mock.MockManager mgrMockCall *gomock.Call podHandler controller.BoostPodHandler + wq workqueue.RateLimitingInterface ) BeforeEach(func() { mockCtrl = gomock.NewController(GinkgoT()) mgrMock = mock.NewMockManager(mockCtrl) + wq = workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()) }) JustBeforeEach(func() { podHandler = controller.NewBoostPodHandler(mgrMock, logr.Discard()) @@ -59,7 +63,7 @@ var _ = Describe("BoostPodHandler", func() { ) }) JustBeforeEach(func() { - podHandler.Create(context.TODO(), createEvent, nil) + podHandler.Create(context.TODO(), createEvent, wq) }) When("There is no boost matching the POD", func() { BeforeEach(func() { @@ -71,13 +75,16 @@ var _ = Describe("BoostPodHandler", func() { }) When("There is a boost matching the POD", func() { var ( - boostMockNameCall *gomock.Call - boostMockUpsertCall *gomock.Call + boostMockNameCall *gomock.Call + boostMockNamespaceCall *gomock.Call + boostMockUpsertCall *gomock.Call ) BeforeEach(func() { boostMock := mock.NewMockStartupCPUBoost(mockCtrl) boostMockNameCall = boostMock.EXPECT().Name(). Return(specTemplate.Name) + boostMockNamespaceCall = boostMock.EXPECT().Namespace(). + Return(specTemplate.Namespace) boostMockUpsertCall = boostMock.EXPECT().UpsertPod( gomock.Any(), gomock.Eq(pod), @@ -86,9 +93,17 @@ var _ = Describe("BoostPodHandler", func() { }) It("sends a valid call to the boost manager and a boost", func() { mgrMockCall.Times(1) - boostMockNameCall.Times(1) + boostMockNameCall.Times(2) + boostMockNamespaceCall.Times(1) boostMockUpsertCall.Times(1) }) + It("sends reconciliation request", func() { + Expect(wq.Len()).To(Equal(1)) + r, _ := wq.Get() + req := r.(reconcile.Request) + Expect(req.Name).To(Equal(specTemplate.Name)) + Expect(req.Namespace).To(Equal(specTemplate.Namespace)) + }) }) }) Describe("Receives delete event", func() { @@ -107,7 +122,7 @@ var _ = Describe("BoostPodHandler", func() { ) }) JustBeforeEach(func() { - podHandler.Delete(context.TODO(), deleteEvent, nil) + podHandler.Delete(context.TODO(), deleteEvent, wq) }) When("There is no boost matching the POD", func() { BeforeEach(func() { @@ -119,10 +134,16 @@ var _ = Describe("BoostPodHandler", func() { }) When("There is a boost matching the POD", func() { var ( - boostMockDeleteCall *gomock.Call + boostMockDeleteCall *gomock.Call + boostMockNameCall *gomock.Call + boostMockNamespaceCall *gomock.Call ) BeforeEach(func() { boostMock := mock.NewMockStartupCPUBoost(mockCtrl) + boostMockNameCall = boostMock.EXPECT().Name(). + Return(specTemplate.Name) + boostMockNamespaceCall = boostMock.EXPECT().Namespace(). + Return(specTemplate.Namespace) boostMockDeleteCall = boostMock.EXPECT().DeletePod( gomock.Any(), gomock.Eq(pod), @@ -131,51 +152,102 @@ var _ = Describe("BoostPodHandler", func() { }) It("sends a valid call to the boost manager and a boost", func() { mgrMockCall.Times(1) + boostMockNameCall.Times(1) + boostMockNamespaceCall.Times(1) boostMockDeleteCall.Times(1) }) + It("sends reconciliation request", func() { + Expect(wq.Len()).To(Equal(1)) + r, _ := wq.Get() + req := r.(reconcile.Request) + Expect(req.Name).To(Equal(specTemplate.Name)) + Expect(req.Namespace).To(Equal(specTemplate.Namespace)) + }) }) }) Describe("Receives an update event", func() { var ( - pod *corev1.Pod + oldPod *corev1.Pod + newPod *corev1.Pod updateEvent event.UpdateEvent ) BeforeEach(func() { - pod = podTemplate.DeepCopy() + oldPod = podTemplate.DeepCopy() + newPod = podTemplate.DeepCopy() updateEvent = event.UpdateEvent{ - ObjectNew: pod, + ObjectNew: newPod, + ObjectOld: oldPod, } - mgrMockCall = mgrMock.EXPECT().StartupCPUBoost( - gomock.Eq(pod.Namespace), - gomock.Eq(specTemplate.Name), - ) }) JustBeforeEach(func() { - podHandler.Update(context.TODO(), updateEvent, nil) + podHandler.Update(context.TODO(), updateEvent, wq) }) - When("There is no boost matching the POD", func() { - BeforeEach(func() { - mgrMockCall.Return(nil, false) + When("Pod status conditions has not change", func() { + It("does not send a call to the boost manager", func() { + mgrMockCall.Times(0) }) - It("sends a valid call to the boost manager", func() { - mgrMockCall.Times(1) + It("does not send reconciliation request", func() { + Expect(wq.Len()).To(Equal(0)) }) }) - When("There is a boost matching the POD", func() { - var ( - boostMockUpsertCall *gomock.Call - ) + When("Pod status conditions has changed", func() { BeforeEach(func() { - boostMock := mock.NewMockStartupCPUBoost(mockCtrl) - boostMockUpsertCall = boostMock.EXPECT().UpsertPod( - gomock.Any(), - gomock.Eq(pod), - ).Return(nil) - mgrMockCall.Return(boostMock, true) + oldPod.Status.Conditions = []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: corev1.ConditionFalse, + }, + } + oldPod.Status.Conditions = []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }, + } + mgrMockCall = mgrMock.EXPECT().StartupCPUBoost( + gomock.Eq(newPod.Namespace), + gomock.Eq(specTemplate.Name), + ) }) - It("sends a valid call to the boost manager and a boost", func() { - mgrMockCall.Times(1) - boostMockUpsertCall.Times(1) + When("There is no boost matching the POD", func() { + BeforeEach(func() { + mgrMockCall.Return(nil, false) + }) + It("sends a valid call to the boost manager", func() { + mgrMockCall.Times(1) + }) + }) + When("There is a boost matching the POD", func() { + var ( + boostMockNameCall *gomock.Call + boostMockNamespaceCall *gomock.Call + boostMockUpsertCall *gomock.Call + ) + BeforeEach(func() { + boostMock := mock.NewMockStartupCPUBoost(mockCtrl) + boostMockNameCall = boostMock.EXPECT().Name(). + Return(specTemplate.Name) + boostMockNamespaceCall = boostMock.EXPECT().Namespace(). + Return(specTemplate.Namespace) + boostMockUpsertCall = boostMock.EXPECT().UpsertPod( + gomock.Any(), + gomock.Eq(newPod), + ).Return(nil) + mgrMockCall.Return(boostMock, true) + }) + It("sends a valid call to the boost manager and a boost", func() { + mgrMockCall.Times(1) + boostMockNameCall.Times(1) + boostMockNamespaceCall.Times(1) + boostMockUpsertCall.Times(1) + }) + It("sends reconciliation request", func() { + Expect(wq.Len()).To(Equal(1)) + r, _ := wq.Get() + req := r.(reconcile.Request) + Expect(req.Name).To(Equal(specTemplate.Name)) + Expect(req.Namespace).To(Equal(specTemplate.Namespace)) + }) }) }) }) diff --git a/internal/mock/boost_manager.go b/internal/mock/boost_manager.go index 24c261e..bb8f064 100644 --- a/internal/mock/boost_manager.go +++ b/internal/mock/boost_manager.go @@ -1,4 +1,4 @@ -// Copyright 2023 Google LLC +// Copyright 2024 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -11,6 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. +// // Code generated by MockGen. DO NOT EDIT. // Source: github.com/google/kube-startup-cpu-boost/internal/boost (interfaces: Manager) @@ -29,6 +30,7 @@ import ( boost "github.com/google/kube-startup-cpu-boost/internal/boost" gomock "go.uber.org/mock/gomock" v1 "k8s.io/api/core/v1" + reconcile "sigs.k8s.io/controller-runtime/pkg/reconcile" ) // MockManager is a mock of Manager interface. @@ -80,6 +82,18 @@ func (mr *MockManagerMockRecorder) RemoveStartupCPUBoost(arg0, arg1, arg2 any) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveStartupCPUBoost", reflect.TypeOf((*MockManager)(nil).RemoveStartupCPUBoost), arg0, arg1, arg2) } +// SetStartupCPUBoostReconciler mocks base method. +func (m *MockManager) SetStartupCPUBoostReconciler(arg0 reconcile.Reconciler) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetStartupCPUBoostReconciler", arg0) +} + +// SetStartupCPUBoostReconciler indicates an expected call of SetStartupCPUBoostReconciler. +func (mr *MockManagerMockRecorder) SetStartupCPUBoostReconciler(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetStartupCPUBoostReconciler", reflect.TypeOf((*MockManager)(nil).SetStartupCPUBoostReconciler), arg0) +} + // Start mocks base method. func (m *MockManager) Start(arg0 context.Context) error { m.ctrl.T.Helper() diff --git a/internal/mock/k8s_subresourcewriter.go b/internal/mock/k8s_subresourcewriter.go new file mode 100644 index 0000000..a4ec4b3 --- /dev/null +++ b/internal/mock/k8s_subresourcewriter.go @@ -0,0 +1,112 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// Code generated by MockGen. DO NOT EDIT. +// Source: sigs.k8s.io/controller-runtime/pkg/client (interfaces: SubResourceWriter) +// +// Generated by this command: +// +// mockgen -package mock --copyright_file hack/boilerplate.go.txt --destination internal/mock/k8s_subresourcewriter.go sigs.k8s.io/controller-runtime/pkg/client SubResourceWriter +// + +package mock + +import ( + context "context" + reflect "reflect" + + gomock "go.uber.org/mock/gomock" + client "sigs.k8s.io/controller-runtime/pkg/client" +) + +// MockSubResourceWriter is a mock of SubResourceWriter interface. +type MockSubResourceWriter struct { + ctrl *gomock.Controller + recorder *MockSubResourceWriterMockRecorder +} + +// MockSubResourceWriterMockRecorder is the mock recorder for MockSubResourceWriter. +type MockSubResourceWriterMockRecorder struct { + mock *MockSubResourceWriter +} + +// NewMockSubResourceWriter creates a new mock instance. +func NewMockSubResourceWriter(ctrl *gomock.Controller) *MockSubResourceWriter { + mock := &MockSubResourceWriter{ctrl: ctrl} + mock.recorder = &MockSubResourceWriterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockSubResourceWriter) EXPECT() *MockSubResourceWriterMockRecorder { + return m.recorder +} + +// Create mocks base method. +func (m *MockSubResourceWriter) Create(arg0 context.Context, arg1, arg2 client.Object, arg3 ...client.SubResourceCreateOption) error { + m.ctrl.T.Helper() + varargs := []any{arg0, arg1, arg2} + for _, a := range arg3 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "Create", varargs...) + ret0, _ := ret[0].(error) + return ret0 +} + +// Create indicates an expected call of Create. +func (mr *MockSubResourceWriterMockRecorder) Create(arg0, arg1, arg2 any, arg3 ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0, arg1, arg2}, arg3...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Create", reflect.TypeOf((*MockSubResourceWriter)(nil).Create), varargs...) +} + +// Patch mocks base method. +func (m *MockSubResourceWriter) Patch(arg0 context.Context, arg1 client.Object, arg2 client.Patch, arg3 ...client.SubResourcePatchOption) error { + m.ctrl.T.Helper() + varargs := []any{arg0, arg1, arg2} + for _, a := range arg3 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "Patch", varargs...) + ret0, _ := ret[0].(error) + return ret0 +} + +// Patch indicates an expected call of Patch. +func (mr *MockSubResourceWriterMockRecorder) Patch(arg0, arg1, arg2 any, arg3 ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0, arg1, arg2}, arg3...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Patch", reflect.TypeOf((*MockSubResourceWriter)(nil).Patch), varargs...) +} + +// Update mocks base method. +func (m *MockSubResourceWriter) Update(arg0 context.Context, arg1 client.Object, arg2 ...client.SubResourceUpdateOption) error { + m.ctrl.T.Helper() + varargs := []any{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "Update", varargs...) + ret0, _ := ret[0].(error) + return ret0 +} + +// Update indicates an expected call of Update. +func (mr *MockSubResourceWriterMockRecorder) Update(arg0, arg1 any, arg2 ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Update", reflect.TypeOf((*MockSubResourceWriter)(nil).Update), varargs...) +} diff --git a/internal/mock/reconciler.go b/internal/mock/reconciler.go new file mode 100644 index 0000000..4f79661 --- /dev/null +++ b/internal/mock/reconciler.go @@ -0,0 +1,70 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// Code generated by MockGen. DO NOT EDIT. +// Source: sigs.k8s.io/controller-runtime/pkg/reconcile (interfaces: Reconciler) +// +// Generated by this command: +// +// mockgen -package mock --copyright_file hack/boilerplate.go.txt --destination internal/mock/reconciler.go sigs.k8s.io/controller-runtime/pkg/reconcile Reconciler +// + +package mock + +import ( + context "context" + reflect "reflect" + + gomock "go.uber.org/mock/gomock" + reconcile "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +// MockReconciler is a mock of Reconciler interface. +type MockReconciler struct { + ctrl *gomock.Controller + recorder *MockReconcilerMockRecorder +} + +// MockReconcilerMockRecorder is the mock recorder for MockReconciler. +type MockReconcilerMockRecorder struct { + mock *MockReconciler +} + +// NewMockReconciler creates a new mock instance. +func NewMockReconciler(ctrl *gomock.Controller) *MockReconciler { + mock := &MockReconciler{ctrl: ctrl} + mock.recorder = &MockReconcilerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockReconciler) EXPECT() *MockReconcilerMockRecorder { + return m.recorder +} + +// Reconcile mocks base method. +func (m *MockReconciler) Reconcile(arg0 context.Context, arg1 reconcile.Request) (reconcile.Result, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Reconcile", arg0, arg1) + ret0, _ := ret[0].(reconcile.Result) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Reconcile indicates an expected call of Reconcile. +func (mr *MockReconcilerMockRecorder) Reconcile(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Reconcile", reflect.TypeOf((*MockReconciler)(nil).Reconcile), arg0, arg1) +} diff --git a/internal/mock/startupcpuboost.go b/internal/mock/startupcpuboost.go index e8820fc..4326ae0 100644 --- a/internal/mock/startupcpuboost.go +++ b/internal/mock/startupcpuboost.go @@ -1,4 +1,4 @@ -// Copyright 2023 Google LLC +// Copyright 2024 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -27,6 +27,7 @@ import ( context "context" reflect "reflect" + boost "github.com/google/kube-startup-cpu-boost/internal/boost" duration "github.com/google/kube-startup-cpu-boost/internal/boost/duration" resource "github.com/google/kube-startup-cpu-boost/internal/boost/resource" gomock "go.uber.org/mock/gomock" @@ -170,6 +171,20 @@ func (mr *MockStartupCPUBoostMockRecorder) RevertResources(arg0, arg1 any) *gomo return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RevertResources", reflect.TypeOf((*MockStartupCPUBoost)(nil).RevertResources), arg0, arg1) } +// Stats mocks base method. +func (m *MockStartupCPUBoost) Stats() boost.StartupCPUBoostStats { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Stats") + ret0, _ := ret[0].(boost.StartupCPUBoostStats) + return ret0 +} + +// Stats indicates an expected call of Stats. +func (mr *MockStartupCPUBoostMockRecorder) Stats() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Stats", reflect.TypeOf((*MockStartupCPUBoost)(nil).Stats)) +} + // UpsertPod mocks base method. func (m *MockStartupCPUBoost) UpsertPod(arg0 context.Context, arg1 *v1.Pod) error { m.ctrl.T.Helper()