From d2ac89237da5001f92e6f579943041be3cbccedb Mon Sep 17 00:00:00 2001 From: FillZpp Date: Tue, 16 Jan 2024 17:40:55 +0800 Subject: [PATCH] Optimize container launch priority performance Signed-off-by: FillZpp --- apis/apps/pub/launch_priority.go | 4 + .../container_launch_priority_controller.go | 132 ++++-- ...ntainer_launch_priority_controller_test.go | 382 +++++++++++++++++- ...irotiy.go => container_launch_priority.go} | 31 +- ...ontainer_launch_priority_initialization.go | 20 +- ...ner_launch_priority_initialization_test.go | 101 +++++ 6 files changed, 599 insertions(+), 71 deletions(-) rename pkg/util/containerlaunchpriority/{container_launch_prirotiy.go => container_launch_priority.go} (70%) create mode 100644 pkg/webhook/pod/mutating/container_launch_priority_initialization_test.go diff --git a/apis/apps/pub/launch_priority.go b/apis/apps/pub/launch_priority.go index 7743b725be..071901c384 100644 --- a/apis/apps/pub/launch_priority.go +++ b/apis/apps/pub/launch_priority.go @@ -29,4 +29,8 @@ const ( ContainerLaunchPriorityKey = "apps.kruise.io/container-launch-priority" // ContainerLaunchOrdered is the annotation value that indicates containers in pod should be launched by ordinal. ContainerLaunchOrdered = "Ordered" + + // ContainerLaunchPriorityCompletedKey is the annotation indicates the pod has all its priorities + // patched into its barrier configmap. + ContainerLaunchPriorityCompletedKey = "apps.kruise.io/container-launch-priority-completed" ) diff --git a/pkg/controller/containerlaunchpriority/container_launch_priority_controller.go b/pkg/controller/containerlaunchpriority/container_launch_priority_controller.go index 6f9557bc14..82d315ff86 100644 --- a/pkg/controller/containerlaunchpriority/container_launch_priority_controller.go +++ b/pkg/controller/containerlaunchpriority/container_launch_priority_controller.go @@ -19,18 +19,16 @@ package containerlauchpriority import ( "context" "fmt" - "strconv" + "sort" "time" - "github.com/openkruise/kruise/pkg/util" - utilclient "github.com/openkruise/kruise/pkg/util/client" - utilcontainerlaunchpriority "github.com/openkruise/kruise/pkg/util/containerlaunchpriority" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" "k8s.io/klog/v2" + "k8s.io/kube-openapi/pkg/util/sets" podutil "k8s.io/kubernetes/pkg/api/v1/pod" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" @@ -40,6 +38,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" + + appspub "github.com/openkruise/kruise/apis/apps/pub" + "github.com/openkruise/kruise/pkg/util" + utilclient "github.com/openkruise/kruise/pkg/util/client" + utilcontainerlaunchpriority "github.com/openkruise/kruise/pkg/util/containerlaunchpriority" ) const ( @@ -70,15 +73,11 @@ func add(mgr manager.Manager, r *ReconcileContainerLaunchPriority) error { err = c.Watch(&source.Kind{Type: &v1.Pod{}}, &handler.EnqueueRequestForObject{}, predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { pod := e.Object.(*v1.Pod) - _, containersReady := podutil.GetPodCondition(&pod.Status, v1.ContainersReady) - // If in vk scenario, there will be not containerReady condition - return utilcontainerlaunchpriority.ExistsPriorities(pod) && (containersReady == nil || containersReady.Status != v1.ConditionTrue) + return shouldEnqueue(pod, mgr.GetCache()) }, UpdateFunc: func(e event.UpdateEvent) bool { pod := e.ObjectNew.(*v1.Pod) - _, containersReady := podutil.GetPodCondition(&pod.Status, v1.ContainersReady) - // If in vk scenario, there will be not containerReady condition - return utilcontainerlaunchpriority.ExistsPriorities(pod) && (containersReady == nil || containersReady.Status != v1.ConditionTrue) + return shouldEnqueue(pod, mgr.GetCache()) }, DeleteFunc: func(e event.DeleteEvent) bool { return false @@ -94,6 +93,30 @@ func add(mgr manager.Manager, r *ReconcileContainerLaunchPriority) error { return nil } +func shouldEnqueue(pod *v1.Pod, r client.Reader) bool { + if pod.Annotations[appspub.ContainerLaunchPriorityCompletedKey] == "true" { + return false + } + if _, containersReady := podutil.GetPodCondition(&pod.Status, v1.ContainersReady); containersReady != nil && containersReady.Status == v1.ConditionTrue { + return false + } + + nextPriorities := findNextPriorities(pod) + if len(nextPriorities) == 0 { + return false + } + + var barrier = &v1.ConfigMap{} + var barrierNamespacedName = types.NamespacedName{ + Namespace: pod.GetNamespace(), + Name: pod.Name + "-barrier", + } + if err := r.Get(context.TODO(), barrierNamespacedName, barrier); err != nil { + return true + } + return !isExistsInBarrier(nextPriorities[len(nextPriorities)-1], barrier) +} + var _ reconcile.Reconciler = &ReconcileContainerLaunchPriority{} // ReconcileContainerLaunchPriority reconciles a Pod object @@ -151,52 +174,73 @@ func (r *ReconcileContainerLaunchPriority) Reconcile(_ context.Context, request return reconcile.Result{}, err } - // set next starting containers - _, containersReady := podutil.GetPodCondition(&pod.Status, v1.ContainersReady) - if containersReady != nil && containersReady.Status != v1.ConditionTrue { - patchKey := r.findNextPatchKey(pod) - if patchKey == nil { - return reconcile.Result{}, nil - } - key := "p_" + strconv.Itoa(*patchKey) - if err = r.patchOnKeyNotExist(barrier, key); err != nil { - return reconcile.Result{}, err - } + // handle the pod and barrier + if err = r.handle(pod, barrier); err != nil { + return reconcile.Result{}, err } return reconcile.Result{}, nil } -func (r *ReconcileContainerLaunchPriority) findNextPatchKey(pod *v1.Pod) *int { - var priority *int - var containerPendingSet = make(map[string]bool) +func (r *ReconcileContainerLaunchPriority) handle(pod *v1.Pod, barrier *v1.ConfigMap) error { + nextPriorities := findNextPriorities(pod) + + // If there is no more priorities, or the lowest priority exists in barrier, mask as completed. + if len(nextPriorities) == 0 || isExistsInBarrier(nextPriorities[0], barrier) { + return r.patchCompleted(pod) + } + + // Try to add the current priority if not exists. + if !isExistsInBarrier(nextPriorities[len(nextPriorities)-1], barrier) { + if err := r.addPriorityIntoBarrier(barrier, nextPriorities[len(nextPriorities)-1]); err != nil { + return err + } + } + + // After adding the current priority, if the lowest priority is same to the current one, mark as completed. + if nextPriorities[len(nextPriorities)-1] == nextPriorities[0] { + return r.patchCompleted(pod) + } + return nil +} + +func (r *ReconcileContainerLaunchPriority) addPriorityIntoBarrier(barrier *v1.ConfigMap, priority int) error { + klog.V(3).Infof("Adding priority %d into barrier %s/%s", priority, barrier.Namespace, barrier.Name) + body := fmt.Sprintf(`{"data":{"%s":"true"}}`, utilcontainerlaunchpriority.GetKey(priority)) + return r.Client.Patch(context.TODO(), barrier, client.RawPatch(types.StrategicMergePatchType, []byte(body))) +} + +func (r *ReconcileContainerLaunchPriority) patchCompleted(pod *v1.Pod) error { + klog.V(3).Infof("Marking pod %s/%s as launch priority completed", pod.Namespace, pod.Name) + body := fmt.Sprintf(`{"metadata":{"annotations":{"%s":"true"}}}`, appspub.ContainerLaunchPriorityCompletedKey) + return r.Client.Patch(context.TODO(), pod, client.RawPatch(types.StrategicMergePatchType, []byte(body))) +} + +func findNextPriorities(pod *v1.Pod) (priorities []int) { + containerReadySet := sets.NewString() for _, status := range pod.Status.ContainerStatuses { if status.Ready { - continue + containerReadySet.Insert(status.Name) } - containerPendingSet[status.Name] = true } for _, c := range pod.Spec.Containers { - if _, ok := containerPendingSet[c.Name]; ok { - p := utilcontainerlaunchpriority.GetContainerPriority(&c) - if p == nil { - continue - } - if priority == nil || *p > *priority { - priority = p - } + if containerReadySet.Has(c.Name) { + continue } + priority := utilcontainerlaunchpriority.GetContainerPriority(&c) + if priority == nil { + continue + } + + priorities = append(priorities, *priority) + } + if len(priorities) > 0 { + sort.Ints(priorities) } - return priority + return } -func (r *ReconcileContainerLaunchPriority) patchOnKeyNotExist(barrier *v1.ConfigMap, key string) error { - if _, ok := barrier.Data[key]; !ok { - body := fmt.Sprintf( - `{"data":{"%s":"true"}}`, - key, - ) - return r.Client.Patch(context.TODO(), barrier, client.RawPatch(types.StrategicMergePatchType, []byte(body))) - } - return nil +func isExistsInBarrier(priority int, barrier *v1.ConfigMap) bool { + _, exists := barrier.Data[utilcontainerlaunchpriority.GetKey(priority)] + return exists } diff --git a/pkg/controller/containerlaunchpriority/container_launch_priority_controller_test.go b/pkg/controller/containerlaunchpriority/container_launch_priority_controller_test.go index 26b7939b5b..b418be173d 100644 --- a/pkg/controller/containerlaunchpriority/container_launch_priority_controller_test.go +++ b/pkg/controller/containerlaunchpriority/container_launch_priority_controller_test.go @@ -18,15 +18,19 @@ package containerlauchpriority import ( "context" + "fmt" + "reflect" "testing" - appspub "github.com/openkruise/kruise/apis/apps/pub" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" + + appspub "github.com/openkruise/kruise/apis/apps/pub" + utilcontainerlaunchpriority "github.com/openkruise/kruise/pkg/util/containerlaunchpriority" ) func TestReconcile(t *testing.T) { @@ -141,3 +145,379 @@ func TestReconcile(t *testing.T) { t.Fatalf("expect barrier1 p_1000 to be true, but get %s", v) } } + +func TestFindNextPriorities(t *testing.T) { + podName := "fake" + cases := []struct { + pod *v1.Pod + expected []int + }{ + { + pod: &v1.Pod{ + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "a", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + {Name: "b", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(5, podName)}}, + {Name: "c", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(0, podName)}}, + {Name: "d"}, + {Name: "e", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-1, podName)}}, + {Name: "f", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(4, podName)}}, + }}, + Status: v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{ + {Name: "a", Ready: false}, + {Name: "b", Ready: true}, + }}, + }, + expected: []int{-1, 0, 3, 4}, + }, + { + pod: &v1.Pod{ + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "a", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + {Name: "b", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(5, podName)}}, + {Name: "c", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(0, podName)}}, + {Name: "d"}, + {Name: "e", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-1, podName)}}, + {Name: "f", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + }}, + Status: v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{ + {Name: "a", Ready: false}, + {Name: "b", Ready: true}, + {Name: "f", Ready: true}, + }}, + }, + expected: []int{-1, 0, 3}, + }, + } + + for i, tc := range cases { + t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) { + got := findNextPriorities(tc.pod) + if !reflect.DeepEqual(got, tc.expected) { + t.Fatalf("expected %v, got %v", tc.expected, got) + } + }) + } +} + +func TestHandle(t *testing.T) { + namespace := "default" + podName := "fake-pod" + configMapName := "fake-pod-barrier" + cases := []struct { + pod *v1.Pod + existedPriorities []int + expectedPriorities []int + expectedCompleted bool + }{ + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: podName}, + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "a", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + {Name: "b", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(5, podName)}}, + {Name: "c", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(0, podName)}}, + {Name: "d"}, + {Name: "e", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-1, podName)}}, + {Name: "f", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + }}, + Status: v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{}}, + }, + existedPriorities: []int{}, + expectedPriorities: []int{5}, + expectedCompleted: false, + }, + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: podName}, + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "a", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + {Name: "b", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(5, podName)}}, + {Name: "c", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(0, podName)}}, + {Name: "d"}, + {Name: "e", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-1, podName)}}, + {Name: "f", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + }}, + Status: v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{ + {Name: "a", Ready: false}, + {Name: "b", Ready: true}, + {Name: "f", Ready: true}, + }}, + }, + existedPriorities: []int{5, 3}, + expectedPriorities: []int{5, 3}, + expectedCompleted: false, + }, + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: podName}, + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "a", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + {Name: "b", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(5, podName)}}, + {Name: "c", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(0, podName)}}, + {Name: "d"}, + {Name: "e", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-1, podName)}}, + {Name: "f", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + }}, + Status: v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{ + {Name: "a", Ready: true}, + {Name: "b", Ready: true}, + {Name: "f", Ready: true}, + }}, + }, + existedPriorities: []int{5, 3}, + expectedPriorities: []int{5, 3, 0}, + expectedCompleted: false, + }, + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: podName}, + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "a", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + {Name: "b", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(5, podName)}}, + {Name: "c", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(0, podName)}}, + {Name: "d"}, + {Name: "e", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-1, podName)}}, + {Name: "f", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + }}, + Status: v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{ + {Name: "a", Ready: true}, + {Name: "b", Ready: true}, + {Name: "c", Ready: true}, + {Name: "f", Ready: true}, + }}, + }, + existedPriorities: []int{5, 3, 0}, + expectedPriorities: []int{5, 3, 0, -1}, + expectedCompleted: true, + }, + } + + var err error + for i, tc := range cases { + t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) { + barrier := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: configMapName}, + Data: map[string]string{}, + } + for _, priority := range tc.existedPriorities { + barrier.Data[utilcontainerlaunchpriority.GetKey(priority)] = "true" + } + cli := fake.NewClientBuilder().WithObjects(tc.pod, barrier).Build() + r := &ReconcileContainerLaunchPriority{Client: cli} + + // reconcile or handle multiple times + for i := 0; i < 2; i++ { + if err = r.handle(tc.pod, barrier); err != nil { + t.Fatal(err) + } + + if err = cli.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: configMapName}, barrier); err != nil { + t.Fatal(err) + } + if len(barrier.Data) != len(tc.expectedPriorities) { + t.Fatalf("expected %v, got %v", tc.expectedPriorities, barrier.Data) + } + for _, priority := range tc.expectedPriorities { + if barrier.Data[utilcontainerlaunchpriority.GetKey(priority)] != "true" { + t.Fatalf("expected %v, got %v", tc.expectedPriorities, barrier.Data) + } + } + + gotPod := &v1.Pod{} + if err = cli.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: podName}, gotPod); err != nil { + t.Fatal(err) + } + gotCompleted := gotPod.Annotations[appspub.ContainerLaunchPriorityCompletedKey] == "true" + if gotCompleted != tc.expectedCompleted { + t.Fatalf("expected completed %v, got %v", tc.expectedCompleted, gotCompleted) + } + } + }) + } +} + +func TestShouldEnqueue(t *testing.T) { + namespace := "default" + podName := "fake-pod" + configMapName := "fake-pod-barrier" + cases := []struct { + pod *v1.Pod + existedPriorities []int + expectedEnqueue bool + }{ + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: podName}, + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "a", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + {Name: "b", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(5, podName)}}, + {Name: "c", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(0, podName)}}, + {Name: "d"}, + {Name: "e", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-1, podName)}}, + {Name: "f", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + }}, + Status: v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{}}, + }, + existedPriorities: []int{}, + expectedEnqueue: true, + }, + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: podName}, + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "a", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + {Name: "b", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(5, podName)}}, + {Name: "c", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(0, podName)}}, + {Name: "d"}, + {Name: "e", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-1, podName)}}, + {Name: "f", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + }}, + Status: v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{}}, + }, + existedPriorities: []int{5}, + expectedEnqueue: false, + }, + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: podName}, + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "a", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + {Name: "b", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(5, podName)}}, + {Name: "c", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(0, podName)}}, + {Name: "d"}, + {Name: "e", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-1, podName)}}, + {Name: "f", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + }}, + Status: v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{ + {Name: "a", Ready: false}, + {Name: "b", Ready: true}, + {Name: "f", Ready: true}, + }}, + }, + existedPriorities: []int{5, 3}, + expectedEnqueue: false, + }, + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: podName}, + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "a", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + {Name: "b", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(5, podName)}}, + {Name: "c", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(0, podName)}}, + {Name: "d"}, + {Name: "e", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-1, podName)}}, + {Name: "f", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + }}, + Status: v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{ + {Name: "a", Ready: true}, + {Name: "b", Ready: true}, + {Name: "f", Ready: true}, + }}, + }, + existedPriorities: []int{5, 3}, + expectedEnqueue: true, + }, + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: podName}, + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "a", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + {Name: "b", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(5, podName)}}, + {Name: "c", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(0, podName)}}, + {Name: "d"}, + {Name: "e", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-1, podName)}}, + {Name: "f", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + }}, + Status: v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{ + {Name: "a", Ready: true}, + {Name: "b", Ready: true}, + {Name: "f", Ready: true}, + }}, + }, + existedPriorities: []int{5, 3, 0}, + expectedEnqueue: false, + }, + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: podName}, + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "a", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + {Name: "b", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(5, podName)}}, + {Name: "c", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(0, podName)}}, + {Name: "d"}, + {Name: "e", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-1, podName)}}, + {Name: "f", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + }}, + Status: v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{ + {Name: "a", Ready: true}, + {Name: "b", Ready: true}, + {Name: "c", Ready: true}, + {Name: "f", Ready: true}, + }}, + }, + existedPriorities: []int{5, 3, 0}, + expectedEnqueue: true, + }, + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: podName}, + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "a", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + {Name: "b", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(5, podName)}}, + {Name: "c", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(0, podName)}}, + {Name: "d"}, + {Name: "e", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-1, podName)}}, + {Name: "f", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + }}, + Status: v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{ + {Name: "a", Ready: true}, + {Name: "b", Ready: true}, + {Name: "c", Ready: true}, + {Name: "f", Ready: true}, + }}, + }, + existedPriorities: []int{5, 3, 0, -1}, + expectedEnqueue: false, + }, + { + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: podName, Annotations: map[string]string{appspub.ContainerLaunchPriorityCompletedKey: "true"}}, + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "a", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + {Name: "b", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(5, podName)}}, + {Name: "c", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(0, podName)}}, + {Name: "d"}, + {Name: "e", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-1, podName)}}, + {Name: "f", Env: []v1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(3, podName)}}, + }}, + Status: v1.PodStatus{ContainerStatuses: []v1.ContainerStatus{ + {Name: "a", Ready: true}, + {Name: "b", Ready: true}, + {Name: "c", Ready: true}, + {Name: "3", Ready: true}, + {Name: "f", Ready: false}, + }}, + }, + existedPriorities: []int{5, 3, 0, -1}, + expectedEnqueue: false, + }, + } + + for i, tc := range cases { + t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) { + barrier := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: configMapName}, + Data: map[string]string{}, + } + for _, priority := range tc.existedPriorities { + barrier.Data[utilcontainerlaunchpriority.GetKey(priority)] = "true" + } + cli := fake.NewClientBuilder().WithObjects(tc.pod, barrier).Build() + + gotEnqueue := shouldEnqueue(tc.pod, cli) + if gotEnqueue != tc.expectedEnqueue { + t.Fatalf("expected enqueue %v, got %v", tc.expectedEnqueue, gotEnqueue) + } + }) + } +} diff --git a/pkg/util/containerlaunchpriority/container_launch_prirotiy.go b/pkg/util/containerlaunchpriority/container_launch_priority.go similarity index 70% rename from pkg/util/containerlaunchpriority/container_launch_prirotiy.go rename to pkg/util/containerlaunchpriority/container_launch_priority.go index cd7a63bd1d..634cc0b540 100644 --- a/pkg/util/containerlaunchpriority/container_launch_prirotiy.go +++ b/pkg/util/containerlaunchpriority/container_launch_priority.go @@ -17,6 +17,7 @@ limitations under the License. package containerlaunchpriority import ( + "fmt" "strconv" appspub "github.com/openkruise/kruise/apis/apps/pub" @@ -28,20 +29,6 @@ const ( priorityStartIndex = 2 ) -func ExistsPriorities(pod *v1.Pod) bool { - if len(pod.Spec.Containers) == 0 { - return false - } - for i := range pod.Spec.Containers { - for _, env := range pod.Spec.Containers[i].Env { - if env.Name == appspub.ContainerLaunchBarrierEnvName { - return true - } - } - } - return false -} - func GetContainerPriority(c *v1.Container) *int { for _, e := range c.Env { if e.Name == appspub.ContainerLaunchBarrierEnvName { @@ -51,3 +38,19 @@ func GetContainerPriority(c *v1.Container) *int { } return nil } + +func GeneratePriorityEnv(priority int, podName string) v1.EnvVar { + return v1.EnvVar{ + Name: appspub.ContainerLaunchBarrierEnvName, + ValueFrom: &v1.EnvVarSource{ + ConfigMapKeyRef: &v1.ConfigMapKeySelector{ + LocalObjectReference: v1.LocalObjectReference{Name: podName + "-barrier"}, + Key: GetKey(priority), + }, + }, + } +} + +func GetKey(priority int) string { + return fmt.Sprintf("p_%d", priority) +} diff --git a/pkg/webhook/pod/mutating/container_launch_priority_initialization.go b/pkg/webhook/pod/mutating/container_launch_priority_initialization.go index 8c6ad450b9..b2d8d92ed6 100644 --- a/pkg/webhook/pod/mutating/container_launch_priority_initialization.go +++ b/pkg/webhook/pod/mutating/container_launch_priority_initialization.go @@ -4,15 +4,18 @@ import ( "context" "strconv" - appspub "github.com/openkruise/kruise/apis/apps/pub" admissionv1 "k8s.io/api/admission/v1" corev1 "k8s.io/api/core/v1" storagenames "k8s.io/apiserver/pkg/storage/names" + "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + appspub "github.com/openkruise/kruise/apis/apps/pub" + utilcontainerlaunchpriority "github.com/openkruise/kruise/pkg/util/containerlaunchpriority" ) // start containers based on priority order -func (h *PodCreateHandler) containerLaunchPriorityInitialization(ctx context.Context, req admission.Request, pod *corev1.Pod) (skip bool, err error) { +func (h *PodCreateHandler) containerLaunchPriorityInitialization(_ context.Context, req admission.Request, pod *corev1.Pod) (skip bool, err error) { if len(req.AdmissionRequest.SubResource) > 0 || req.AdmissionRequest.Operation != admissionv1.Create || req.AdmissionRequest.Resource.Resource != "pods" { @@ -30,6 +33,7 @@ func (h *PodCreateHandler) containerLaunchPriorityInitialization(ctx context.Con priority[i] = 0 - i } h.setPodEnv(priority, pod) + klog.V(3).Infof("Injected ordered container launch priority for Pod %s/%s", pod.Namespace, pod.Name) return false, nil } @@ -43,6 +47,7 @@ func (h *PodCreateHandler) containerLaunchPriorityInitialization(ctx context.Con } h.setPodEnv(priority, pod) + klog.V(3).Infof("Injected customized container launch priority for Pod %s/%s", pod.Namespace, pod.Name) return false, nil } @@ -87,15 +92,6 @@ func (h *PodCreateHandler) setPodEnv(priority []int, pod *corev1.Pod) { pod.Name = storagenames.SimpleNameGenerator.GenerateName(pod.GenerateName) } for i := range priority { - env := corev1.EnvVar{ - Name: appspub.ContainerLaunchBarrierEnvName, - ValueFrom: &corev1.EnvVarSource{ - ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: pod.Name + "-barrier"}, - Key: "p_" + strconv.Itoa(priority[i]), - }, - }, - } - pod.Spec.Containers[i].Env = append(pod.Spec.Containers[i].Env, env) + pod.Spec.Containers[i].Env = append(pod.Spec.Containers[i].Env, utilcontainerlaunchpriority.GeneratePriorityEnv(priority[i], pod.Name)) } } diff --git a/pkg/webhook/pod/mutating/container_launch_priority_initialization_test.go b/pkg/webhook/pod/mutating/container_launch_priority_initialization_test.go new file mode 100644 index 0000000000..ab733380ef --- /dev/null +++ b/pkg/webhook/pod/mutating/container_launch_priority_initialization_test.go @@ -0,0 +1,101 @@ +/* +Copyright 2024 The Kruise Authors. + +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 + + http://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 mutating + +import ( + "context" + "fmt" + "reflect" + "testing" + + admissionv1 "k8s.io/api/admission/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + appspub "github.com/openkruise/kruise/apis/apps/pub" + "github.com/openkruise/kruise/pkg/util" + utilcontainerlaunchpriority "github.com/openkruise/kruise/pkg/util/containerlaunchpriority" +) + +func TestContainerLaunchPriorityInitialization(t *testing.T) { + cases := []struct { + pod *corev1.Pod + expectedSkip bool + expectedContainers []corev1.Container + }{ + { + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "a"}, + {Name: "b"}, + }, + }, + }, + expectedSkip: true, + expectedContainers: []corev1.Container{ + {Name: "a"}, + {Name: "b"}, + }, + }, + { + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake", + Annotations: map[string]string{appspub.ContainerLaunchPriorityKey: appspub.ContainerLaunchOrdered}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "a"}, + {Name: "b"}, + {Name: "c"}, + }, + }, + }, + expectedSkip: false, + expectedContainers: []corev1.Container{ + {Name: "a", Env: []corev1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(0, "fake")}}, + {Name: "b", Env: []corev1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-1, "fake")}}, + {Name: "c", Env: []corev1.EnvVar{utilcontainerlaunchpriority.GeneratePriorityEnv(-2, "fake")}}, + }, + }, + } + + h := &PodCreateHandler{} + req := admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + Resource: metav1.GroupVersionResource{Resource: "pods", Version: "v1"}, + }} + for i, tc := range cases { + t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) { + skip, err := h.containerLaunchPriorityInitialization(context.TODO(), req, tc.pod) + if err != nil { + t.Fatal(err) + } + if skip != tc.expectedSkip { + t.Fatalf("expected skip %v, got %v", tc.expectedSkip, skip) + } + if !reflect.DeepEqual(tc.expectedContainers, tc.pod.Spec.Containers) { + t.Fatalf("expected containers\n%v\ngot\n%v", util.DumpJSON(tc.expectedContainers), util.DumpJSON(tc.pod.Spec.Containers)) + } + }) + } +}