From b6f188015d60e34a838de51ba700bb7034a37b55 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Wed, 7 Feb 2024 09:17:56 +0100 Subject: [PATCH] fix(lifecycle-operator): introduce separate controller for removing scheduling gates from pods (#2946) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché Signed-off-by: Florian Bacher Signed-off-by: RealAnna Signed-off-by: RealAnna <89971034+RealAnna@users.noreply.github.com> Signed-off-by: check-spelling-bot Signed-off-by: Meg McRoberts Signed-off-by: Moritz Wiesinger Co-authored-by: Charles-Edouard Brétéché Co-authored-by: Moritz Wiesinger Co-authored-by: RealAnna <89971034+RealAnna@users.noreply.github.com> Co-authored-by: Meg McRoberts Co-authored-by: odubajDT <93584209+odubajDT@users.noreply.github.com> Signed-off-by: vickysomtee --- .github/actions/spelling/expect.txt | 4 +- .../controllers/common/helperfunctions.go | 12 + .../common/helperfunctions_test.go | 81 +++- .../fake/schedulinggateshandler_mock.go | 115 ------ .../common/schedulinggates/handler.go | 108 ------ .../common/schedulinggates/handler_test.go | 365 ------------------ .../lifecycle/keptnworkload/controller.go | 1 + .../keptnworkloadversion/controller.go | 43 +-- .../keptnworkloadversion/controller_test.go | 168 -------- .../lifecycle/schedulinggates/controller.go | 114 ++++++ .../schedulinggates/controller_test.go | 337 ++++++++++++++++ lifecycle-operator/main.go | 36 +- .../workloadversion_suite_test.go | 20 +- .../00-assert.yaml | 20 + .../00-install.yaml | 35 ++ .../01-assert.yaml | 36 ++ .../02-assert.yaml | 6 + .../chainsaw-test.yaml | 56 +++ 18 files changed, 743 insertions(+), 814 deletions(-) delete mode 100644 lifecycle-operator/controllers/common/schedulinggates/fake/schedulinggateshandler_mock.go delete mode 100644 lifecycle-operator/controllers/common/schedulinggates/handler.go delete mode 100644 lifecycle-operator/controllers/common/schedulinggates/handler_test.go create mode 100644 lifecycle-operator/controllers/lifecycle/schedulinggates/controller.go create mode 100644 lifecycle-operator/controllers/lifecycle/schedulinggates/controller_test.go create mode 100755 test/chainsaw/scheduling-gates/simple-deployment-restart-pod/00-assert.yaml create mode 100644 test/chainsaw/scheduling-gates/simple-deployment-restart-pod/00-install.yaml create mode 100755 test/chainsaw/scheduling-gates/simple-deployment-restart-pod/01-assert.yaml create mode 100755 test/chainsaw/scheduling-gates/simple-deployment-restart-pod/02-assert.yaml create mode 100755 test/chainsaw/scheduling-gates/simple-deployment-restart-pod/chainsaw-test.yaml diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index f4ee4e2ef7a..2d586dbbdcb 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -261,7 +261,6 @@ iobjective IOperator IProviders IRound -IScheduling Isitobservable ISpan ITarget @@ -536,8 +535,7 @@ runtimes runtimespec Sambhav sbom -schedulinggate -schedulinggateshandler +schedulinggates sclient screenshot sdkmetric diff --git a/lifecycle-operator/controllers/common/helperfunctions.go b/lifecycle-operator/controllers/common/helperfunctions.go index 4458a5d9bd3..b6569a393ad 100644 --- a/lifecycle-operator/controllers/common/helperfunctions.go +++ b/lifecycle-operator/controllers/common/helperfunctions.go @@ -89,3 +89,15 @@ func GetRequestInfo(req ctrl.Request) map[string]string { "namespace": req.Namespace, } } + +func KeptnWorkloadVersionResourceRefUIDIndexFunc(rawObj client.Object) []string { + // Extract the ResourceReference UID name from the KeptnWorkloadVersion Spec, if one is provided + workloadVersion, ok := rawObj.(*klcv1beta1.KeptnWorkloadVersion) + if !ok { + return nil + } + if workloadVersion.Spec.ResourceReference.UID == "" { + return nil + } + return []string{string(workloadVersion.Spec.ResourceReference.UID)} +} diff --git a/lifecycle-operator/controllers/common/helperfunctions_test.go b/lifecycle-operator/controllers/common/helperfunctions_test.go index 04a4244a8a8..c4a423ae64a 100644 --- a/lifecycle-operator/controllers/common/helperfunctions_test.go +++ b/lifecycle-operator/controllers/common/helperfunctions_test.go @@ -2,6 +2,7 @@ package common import ( "context" + "reflect" "testing" klcv1beta1 "github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1beta1" @@ -9,10 +10,12 @@ import ( "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/config" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/testcommon" "github.com/stretchr/testify/require" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -130,7 +133,7 @@ func Test_GetTaskDefinition(t *testing.T) { { name: "taskDef not found", taskDef: &klcv1beta1.KeptnTaskDefinition{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "taskDef", Namespace: "some-other-namespace", }, @@ -143,7 +146,7 @@ func Test_GetTaskDefinition(t *testing.T) { { name: "taskDef found", taskDef: &klcv1beta1.KeptnTaskDefinition{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "taskDef", Namespace: "some-namespace", }, @@ -151,7 +154,7 @@ func Test_GetTaskDefinition(t *testing.T) { taskDefName: "taskDef", taskDefNamespace: "some-namespace", out: &klcv1beta1.KeptnTaskDefinition{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "taskDef", Namespace: "some-namespace", }, @@ -161,7 +164,7 @@ func Test_GetTaskDefinition(t *testing.T) { { name: "taskDef found in default Keptn namespace", taskDef: &klcv1beta1.KeptnTaskDefinition{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "taskDef", Namespace: testcommon.KeptnNamespace, }, @@ -169,7 +172,7 @@ func Test_GetTaskDefinition(t *testing.T) { taskDefName: "taskDef", taskDefNamespace: "some-namespace", out: &klcv1beta1.KeptnTaskDefinition{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "taskDef", Namespace: testcommon.KeptnNamespace, }, @@ -214,7 +217,7 @@ func Test_GetEvaluationDefinition(t *testing.T) { { name: "evalDef not found", evalDef: &klcv1beta1.KeptnEvaluationDefinition{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "evalDef", Namespace: "some-other-namespace", }, @@ -227,7 +230,7 @@ func Test_GetEvaluationDefinition(t *testing.T) { { name: "evalDef found", evalDef: &klcv1beta1.KeptnEvaluationDefinition{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "evalDef", Namespace: "some-namespace", }, @@ -235,7 +238,7 @@ func Test_GetEvaluationDefinition(t *testing.T) { evalDefName: "evalDef", evalDefNamespace: "some-namespace", out: &klcv1beta1.KeptnEvaluationDefinition{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "evalDef", Namespace: "some-namespace", }, @@ -245,7 +248,7 @@ func Test_GetEvaluationDefinition(t *testing.T) { { name: "evalDef found in default Keptn namespace", evalDef: &klcv1beta1.KeptnEvaluationDefinition{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "evalDef", Namespace: testcommon.KeptnNamespace, }, @@ -253,7 +256,7 @@ func Test_GetEvaluationDefinition(t *testing.T) { evalDefName: "evalDef", evalDefNamespace: "some-namespace", out: &klcv1beta1.KeptnEvaluationDefinition{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "evalDef", Namespace: testcommon.KeptnNamespace, }, @@ -381,3 +384,59 @@ func Test_MergeMaps(t *testing.T) { }) } } + +func Test_resourceRefUIDIndexFunc(t *testing.T) { + type args struct { + rawObj client.Object + } + tests := []struct { + name string + args args + want []string + }{ + { + name: "get uid of resource reference", + args: args{ + rawObj: &klcv1beta1.KeptnWorkloadVersion{ + Spec: klcv1beta1.KeptnWorkloadVersionSpec{ + KeptnWorkloadSpec: klcv1beta1.KeptnWorkloadSpec{ + ResourceReference: klcv1beta1.ResourceReference{ + UID: "my-uid", + }, + }, + }, + }, + }, + want: []string{"my-uid"}, + }, + { + name: "empty uid", + args: args{ + rawObj: &klcv1beta1.KeptnWorkloadVersion{ + Spec: klcv1beta1.KeptnWorkloadVersionSpec{ + KeptnWorkloadSpec: klcv1beta1.KeptnWorkloadSpec{ + ResourceReference: klcv1beta1.ResourceReference{ + UID: "", + }, + }, + }, + }, + }, + want: nil, + }, + { + name: "not a KeptnWorkloadVersion", + args: args{ + rawObj: &v1.Pod{}, + }, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := KeptnWorkloadVersionResourceRefUIDIndexFunc(tt.args.rawObj); !reflect.DeepEqual(got, tt.want) { + t.Errorf("KeptnWorkloadVersionResourceRefUIDIndexFunc() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/lifecycle-operator/controllers/common/schedulinggates/fake/schedulinggateshandler_mock.go b/lifecycle-operator/controllers/common/schedulinggates/fake/schedulinggateshandler_mock.go deleted file mode 100644 index b25a7adba83..00000000000 --- a/lifecycle-operator/controllers/common/schedulinggates/fake/schedulinggateshandler_mock.go +++ /dev/null @@ -1,115 +0,0 @@ -// Code generated by moq; DO NOT EDIT. -// github.com/matryer/moq - -package fake - -import ( - "context" - klcv1beta1 "github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1beta1" - "sync" -) - -// ISchedulingGatesHandlerMock is a mock implementation of schedulinggate.ISchedulingGatesHandler. -// -// func TestSomethingThatUsesISchedulingGatesHandler(t *testing.T) { -// -// // make and configure a mocked schedulinggate.ISchedulingGatesHandler -// mockedISchedulingGatesHandler := &ISchedulingGatesHandlerMock{ -// EnabledFunc: func() bool { -// panic("mock out the Enabled method") -// }, -// RemoveGatesFunc: func(ctx context.Context, workloadVersion *klcv1beta1.KeptnWorkloadVersion) error { -// panic("mock out the RemoveGates method") -// }, -// } -// -// // use mockedISchedulingGatesHandler in code that requires schedulinggate.ISchedulingGatesHandler -// // and then make assertions. -// -// } -type ISchedulingGatesHandlerMock struct { - // EnabledFunc mocks the Enabled method. - EnabledFunc func() bool - - // RemoveGatesFunc mocks the RemoveGates method. - RemoveGatesFunc func(ctx context.Context, workloadVersion *klcv1beta1.KeptnWorkloadVersion) error - - // calls tracks calls to the methods. - calls struct { - // Enabled holds details about calls to the Enabled method. - Enabled []struct { - } - // RemoveGates holds details about calls to the RemoveGates method. - RemoveGates []struct { - // Ctx is the ctx argument value. - Ctx context.Context - // WorkloadVersion is the workloadVersion argument value. - WorkloadVersion *klcv1beta1.KeptnWorkloadVersion - } - } - lockEnabled sync.RWMutex - lockRemoveGates sync.RWMutex -} - -// Enabled calls EnabledFunc. -func (mock *ISchedulingGatesHandlerMock) Enabled() bool { - if mock.EnabledFunc == nil { - panic("ISchedulingGatesHandlerMock.EnabledFunc: method is nil but ISchedulingGatesHandler.Enabled was just called") - } - callInfo := struct { - }{} - mock.lockEnabled.Lock() - mock.calls.Enabled = append(mock.calls.Enabled, callInfo) - mock.lockEnabled.Unlock() - return mock.EnabledFunc() -} - -// EnabledCalls gets all the calls that were made to Enabled. -// Check the length with: -// -// len(mockedISchedulingGatesHandler.EnabledCalls()) -func (mock *ISchedulingGatesHandlerMock) EnabledCalls() []struct { -} { - var calls []struct { - } - mock.lockEnabled.RLock() - calls = mock.calls.Enabled - mock.lockEnabled.RUnlock() - return calls -} - -// RemoveGates calls RemoveGatesFunc. -func (mock *ISchedulingGatesHandlerMock) RemoveGates(ctx context.Context, workloadVersion *klcv1beta1.KeptnWorkloadVersion) error { - if mock.RemoveGatesFunc == nil { - panic("ISchedulingGatesHandlerMock.RemoveGatesFunc: method is nil but ISchedulingGatesHandler.RemoveGates was just called") - } - callInfo := struct { - Ctx context.Context - WorkloadVersion *klcv1beta1.KeptnWorkloadVersion - }{ - Ctx: ctx, - WorkloadVersion: workloadVersion, - } - mock.lockRemoveGates.Lock() - mock.calls.RemoveGates = append(mock.calls.RemoveGates, callInfo) - mock.lockRemoveGates.Unlock() - return mock.RemoveGatesFunc(ctx, workloadVersion) -} - -// RemoveGatesCalls gets all the calls that were made to RemoveGates. -// Check the length with: -// -// len(mockedISchedulingGatesHandler.RemoveGatesCalls()) -func (mock *ISchedulingGatesHandlerMock) RemoveGatesCalls() []struct { - Ctx context.Context - WorkloadVersion *klcv1beta1.KeptnWorkloadVersion -} { - var calls []struct { - Ctx context.Context - WorkloadVersion *klcv1beta1.KeptnWorkloadVersion - } - mock.lockRemoveGates.RLock() - calls = mock.calls.RemoveGates - mock.lockRemoveGates.RUnlock() - return calls -} diff --git a/lifecycle-operator/controllers/common/schedulinggates/handler.go b/lifecycle-operator/controllers/common/schedulinggates/handler.go deleted file mode 100644 index 28016ec2217..00000000000 --- a/lifecycle-operator/controllers/common/schedulinggates/handler.go +++ /dev/null @@ -1,108 +0,0 @@ -package schedulinggates - -import ( - "context" - - "github.com/go-logr/logr" - klcv1beta1 "github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1beta1" - apicommon "github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1beta1/common" - controllererrors "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/errors" - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -//go:generate moq -pkg fake -skip-ensure -out ./fake/schedulinggateshandler_mock.go . ISchedulingGatesHandler -type ISchedulingGatesHandler interface { - RemoveGates(ctx context.Context, workloadVersion *klcv1beta1.KeptnWorkloadVersion) error - Enabled() bool -} - -type RemoveGatesFunc func(ctx context.Context, c client.Client, podName string, podNamespace string) error -type GetPodsFunc func(ctx context.Context, c client.Client, ownerUID types.UID, ownerKind string, namespace string) ([]string, error) - -type Handler struct { - client.Client - logr.Logger - enabled bool - removeGates RemoveGatesFunc - getPods GetPodsFunc -} - -func NewHandler(c client.Client, l logr.Logger, enabled bool) *Handler { - return &Handler{ - Client: c, - Logger: l, - enabled: enabled, - removeGates: removePodGates, - getPods: getPodsOfOwner, - } -} - -func (h *Handler) RemoveGates(ctx context.Context, workloadVersion *klcv1beta1.KeptnWorkloadVersion) error { - switch workloadVersion.Spec.ResourceReference.Kind { - case "Pod": - return h.removeGates(ctx, h.Client, workloadVersion.Spec.ResourceReference.Name, workloadVersion.Namespace) - case "ReplicaSet", "StatefulSet", "DaemonSet": - podList, err := h.getPods(ctx, h.Client, workloadVersion.Spec.ResourceReference.UID, workloadVersion.Spec.ResourceReference.Kind, workloadVersion.Namespace) - if err != nil { - h.Logger.Error(err, "cannot get pods") - return err - } - for _, pod := range podList { - err := h.removeGates(ctx, h.Client, pod, workloadVersion.Namespace) - if err != nil { - h.Logger.Error(err, "cannot remove gates from pod") - return err - } - } - default: - return controllererrors.ErrUnsupportedWorkloadVersionResourceReference - } - - return nil -} - -func (h *Handler) Enabled() bool { - return h.enabled -} - -func removePodGates(ctx context.Context, c client.Client, podName string, podNamespace string) error { - pod := &v1.Pod{} - err := c.Get(ctx, types.NamespacedName{Namespace: podNamespace, Name: podName}, pod) - if err != nil { - return err - } - - if pod.Annotations[apicommon.SchedulingGateRemoved] != "" { - return nil - } - - if len(pod.Annotations) == 0 { - pod.Annotations = make(map[string]string, 1) - } - pod.Annotations[apicommon.SchedulingGateRemoved] = "true" - pod.Spec.SchedulingGates = nil - return c.Update(ctx, pod) -} - -func getPodsOfOwner(ctx context.Context, c client.Client, ownerUID types.UID, ownerKind string, namespace string) ([]string, error) { - pods := &v1.PodList{} - err := c.List(ctx, pods, client.InNamespace(namespace)) - if err != nil { - return nil, err - } - - var resultPods []string - - for _, pod := range pods.Items { - for _, owner := range pod.OwnerReferences { - if owner.Kind == ownerKind && owner.UID == ownerUID { - resultPods = append(resultPods, pod.Name) - break - } - } - } - - return resultPods, nil -} diff --git a/lifecycle-operator/controllers/common/schedulinggates/handler_test.go b/lifecycle-operator/controllers/common/schedulinggates/handler_test.go deleted file mode 100644 index cb9b10ea4d4..00000000000 --- a/lifecycle-operator/controllers/common/schedulinggates/handler_test.go +++ /dev/null @@ -1,365 +0,0 @@ -package schedulinggates - -import ( - "context" - "fmt" - "testing" - - klcv1beta1 "github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1beta1" - apicommon "github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1beta1/common" - controllererrors "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/errors" - "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/kubernetes/scheme" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" -) - -func Test_RemovePodGates(t *testing.T) { - tests := []struct { - name string - podName string - pod *corev1.Pod - wantError bool - annotations map[string]string - }{ - { - name: "pod does not exist", - podName: "pod", - pod: &corev1.Pod{ - ObjectMeta: v1.ObjectMeta{ - Name: "pod2", - Namespace: "default", - }, - }, - wantError: true, - }, - { - name: "scheduling gates already removed", - podName: "pod", - pod: &corev1.Pod{ - ObjectMeta: v1.ObjectMeta{ - Name: "pod", - Namespace: "default", - Annotations: map[string]string{ - apicommon.SchedulingGateRemoved: "true", - }, - }, - }, - wantError: false, - annotations: map[string]string{ - apicommon.SchedulingGateRemoved: "true", - }, - }, - { - name: "scheduling gates removed - empty annotations", - podName: "pod", - pod: &corev1.Pod{ - ObjectMeta: v1.ObjectMeta{ - Name: "pod", - Namespace: "default", - }, - Spec: corev1.PodSpec{ - SchedulingGates: []corev1.PodSchedulingGate{ - { - Name: "gate", - }, - }, - }, - }, - wantError: false, - annotations: map[string]string{ - apicommon.SchedulingGateRemoved: "true", - }, - }, - { - name: "scheduling gates removed - not empty annotations", - podName: "pod", - pod: &corev1.Pod{ - ObjectMeta: v1.ObjectMeta{ - Name: "pod", - Namespace: "default", - Annotations: map[string]string{ - "test": "test", - }, - }, - Spec: corev1.PodSpec{ - SchedulingGates: []corev1.PodSchedulingGate{ - { - Name: "gate", - }, - }, - }, - }, - wantError: false, - annotations: map[string]string{ - apicommon.SchedulingGateRemoved: "true", - "test": "test", - }, - }, - } - - err := klcv1beta1.AddToScheme(scheme.Scheme) - require.Nil(t, err) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - client := fake.NewClientBuilder().WithObjects(tt.pod).WithStatusSubresource(tt.pod).Build() - err := removePodGates(context.TODO(), client, tt.podName, tt.pod.Namespace) - if tt.wantError != (err != nil) { - t.Errorf("want error: %t, got: %v", tt.wantError, err) - } - if !tt.wantError { - pod := &corev1.Pod{} - err := client.Get(context.TODO(), types.NamespacedName{Namespace: tt.pod.Namespace, Name: tt.podName}, pod) - require.Nil(t, err) - require.Equal(t, []corev1.PodSchedulingGate(nil), pod.Spec.SchedulingGates) - require.Equal(t, tt.annotations, pod.Annotations) - } - }) - } -} - -func Test_GetPodsOfOwner(t *testing.T) { - namespace := "default" - tests := []struct { - name string - uid types.UID - kind string - pods *corev1.PodList - result []string - }{ - { - name: "pod list empty", - pods: &corev1.PodList{}, - result: nil, - }, - { - name: "pod list not matching kind or uid", - pods: &corev1.PodList{ - Items: []corev1.Pod{ - { - ObjectMeta: v1.ObjectMeta{ - Name: "pod1", - Namespace: "default", - OwnerReferences: []v1.OwnerReference{ - { - Kind: "unknown", - UID: types.UID("uid"), - }, - }, - }, - }, - }, - }, - kind: "unknown2", - uid: types.UID("uid2"), - result: nil, - }, - { - name: "pod list matches one pod of list", - pods: &corev1.PodList{ - Items: []corev1.Pod{ - { - ObjectMeta: v1.ObjectMeta{ - Name: "pod1", - Namespace: "default", - OwnerReferences: []v1.OwnerReference{ - { - Kind: "unknown", - UID: types.UID("uid"), - }, - }, - }, - }, - { - ObjectMeta: v1.ObjectMeta{ - Name: "pod2", - Namespace: "default", - OwnerReferences: []v1.OwnerReference{ - { - Kind: "unknown2", - UID: types.UID("uid2"), - }, - }, - }, - }, - }, - }, - kind: "unknown", - uid: types.UID("uid"), - result: []string{"pod1"}, - }, - } - - err := klcv1beta1.AddToScheme(scheme.Scheme) - require.Nil(t, err) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - client := fake.NewClientBuilder().WithLists(tt.pods).Build() - res, err := getPodsOfOwner(context.TODO(), client, tt.uid, tt.kind, namespace) - require.Nil(t, err) - require.Equal(t, tt.result, res) - }) - } -} - -func Test_SchedulingGatesHandler_IsSchedulingGatesEnabled(t *testing.T) { - h := Handler{ - enabled: true, - } - - require.True(t, h.Enabled()) - - h.enabled = false - - require.False(t, h.Enabled()) -} - -func Test_SchedulingGatesHandler_IsSchedulingGatesEnabledRemoveGates(t *testing.T) { - tests := []struct { - name string - handler Handler - wi *klcv1beta1.KeptnWorkloadVersion - wantErr error - }{ - { - name: "unsuported resource ref", - handler: Handler{}, - wi: &klcv1beta1.KeptnWorkloadVersion{ - Spec: klcv1beta1.KeptnWorkloadVersionSpec{ - KeptnWorkloadSpec: klcv1beta1.KeptnWorkloadSpec{ - ResourceReference: klcv1beta1.ResourceReference{ - Kind: "unsupported", - }, - }, - }, - }, - wantErr: controllererrors.ErrUnsupportedWorkloadVersionResourceReference, - }, - { - name: "pod - happy path", - handler: Handler{ - removeGates: func(ctx context.Context, c client.Client, podName, podNamespace string) error { - return nil - }, - }, - wi: &klcv1beta1.KeptnWorkloadVersion{ - Spec: klcv1beta1.KeptnWorkloadVersionSpec{ - KeptnWorkloadSpec: klcv1beta1.KeptnWorkloadSpec{ - ResourceReference: klcv1beta1.ResourceReference{ - Kind: "Pod", - }, - }, - }, - }, - wantErr: nil, - }, - { - name: "pod - fail path", - handler: Handler{ - removeGates: func(ctx context.Context, c client.Client, podName, podNamespace string) error { - return fmt.Errorf("pod") - }, - }, - wi: &klcv1beta1.KeptnWorkloadVersion{ - Spec: klcv1beta1.KeptnWorkloadVersionSpec{ - KeptnWorkloadSpec: klcv1beta1.KeptnWorkloadSpec{ - ResourceReference: klcv1beta1.ResourceReference{ - Kind: "Pod", - }, - }, - }, - }, - wantErr: fmt.Errorf("pod"), - }, - { - name: "ReplicaSet, StatefulSet, DaemonSet - happy path", - handler: Handler{ - removeGates: func(ctx context.Context, c client.Client, podName, podNamespace string) error { - return nil - }, - getPods: func(ctx context.Context, c client.Client, ownerUID types.UID, ownerKind, namespace string) ([]string, error) { - return []string{"podName"}, nil - }, - }, - wi: &klcv1beta1.KeptnWorkloadVersion{ - Spec: klcv1beta1.KeptnWorkloadVersionSpec{ - KeptnWorkloadSpec: klcv1beta1.KeptnWorkloadSpec{ - ResourceReference: klcv1beta1.ResourceReference{ - Kind: "ReplicaSet", - }, - }, - }, - }, - wantErr: nil, - }, - { - name: "ReplicaSet, StatefulSet, DaemonSet - happy path - no pods found", - handler: Handler{ - getPods: func(ctx context.Context, c client.Client, ownerUID types.UID, ownerKind, namespace string) ([]string, error) { - return []string{}, nil - }, - }, - wi: &klcv1beta1.KeptnWorkloadVersion{ - Spec: klcv1beta1.KeptnWorkloadVersionSpec{ - KeptnWorkloadSpec: klcv1beta1.KeptnWorkloadSpec{ - ResourceReference: klcv1beta1.ResourceReference{ - Kind: "ReplicaSet", - }, - }, - }, - }, - wantErr: nil, - }, - { - name: "ReplicaSet, StatefulSet, DaemonSet - happy path - err getPods", - handler: Handler{ - getPods: func(ctx context.Context, c client.Client, ownerUID types.UID, ownerKind, namespace string) ([]string, error) { - return []string{}, fmt.Errorf("err") - }, - }, - wi: &klcv1beta1.KeptnWorkloadVersion{ - Spec: klcv1beta1.KeptnWorkloadVersionSpec{ - KeptnWorkloadSpec: klcv1beta1.KeptnWorkloadSpec{ - ResourceReference: klcv1beta1.ResourceReference{ - Kind: "ReplicaSet", - }, - }, - }, - }, - wantErr: fmt.Errorf("err"), - }, - { - name: "ReplicaSet, StatefulSet, DaemonSet - err removeGates", - handler: Handler{ - removeGates: func(ctx context.Context, c client.Client, podName, podNamespace string) error { - return fmt.Errorf("err") - }, - getPods: func(ctx context.Context, c client.Client, ownerUID types.UID, ownerKind, namespace string) ([]string, error) { - return []string{"podName"}, nil - }, - }, - wi: &klcv1beta1.KeptnWorkloadVersion{ - Spec: klcv1beta1.KeptnWorkloadVersionSpec{ - KeptnWorkloadSpec: klcv1beta1.KeptnWorkloadSpec{ - ResourceReference: klcv1beta1.ResourceReference{ - Kind: "ReplicaSet", - }, - }, - }, - }, - wantErr: fmt.Errorf("err"), - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := tt.handler.RemoveGates(context.TODO(), tt.wi) - require.Equal(t, tt.wantErr, err) - }) - } -} diff --git a/lifecycle-operator/controllers/lifecycle/keptnworkload/controller.go b/lifecycle-operator/controllers/lifecycle/keptnworkload/controller.go index b422c8cc717..6f167300c82 100644 --- a/lifecycle-operator/controllers/lifecycle/keptnworkload/controller.go +++ b/lifecycle-operator/controllers/lifecycle/keptnworkload/controller.go @@ -113,6 +113,7 @@ func (r *KeptnWorkloadReconciler) Reconcile(ctx context.Context, req ctrl.Reques r.Log.Error(err, "could not update Current Version of Workload") return ctrl.Result{}, err } + } else if !reflect.DeepEqual(workloadVersion.Spec.KeptnWorkloadSpec, workload.Spec) { r.Log.Info("updating spec of KeptnWorkloadVersion", "requestInfo", requestInfo, "workloadVersion", workloadVersion.Name) workloadVersion.Spec.KeptnWorkloadSpec = workload.Spec diff --git a/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller.go b/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller.go index 687fb1b6b03..1a27f98bba8 100644 --- a/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller.go +++ b/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller.go @@ -30,7 +30,6 @@ import ( "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/evaluation" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/eventsender" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/phase" - "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/schedulinggates" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/telemetry" controllererrors "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/errors" "go.opentelemetry.io/otel" @@ -47,20 +46,22 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ) -const traceComponentName = "keptn/lifecycle-operator/workloadversion" +const ( + traceComponentName = "keptn/lifecycle-operator/workloadversion" + resourceReferenceUIDField = ".spec.resourceReference.uid" +) // KeptnWorkloadVersionReconciler reconciles a KeptnWorkloadVersion object type KeptnWorkloadVersionReconciler struct { client.Client - Scheme *runtime.Scheme - EventSender eventsender.IEvent - Log logr.Logger - Meters apicommon.KeptnMeters - SpanHandler telemetry.ISpanHandler - TracerFactory telemetry.TracerFactory - SchedulingGatesHandler schedulinggates.ISchedulingGatesHandler - EvaluationHandler evaluation.IEvaluationHandler - PhaseHandler phase.IHandler + Scheme *runtime.Scheme + EventSender eventsender.IEvent + Log logr.Logger + Meters apicommon.KeptnMeters + SpanHandler telemetry.ISpanHandler + TracerFactory telemetry.TracerFactory + EvaluationHandler evaluation.IEvaluationHandler + PhaseHandler phase.IHandler } // +kubebuilder:rbac:groups=lifecycle.keptn.sh,resources=keptnworkloadversions,verbs=get;list;watch;create;update;patch;delete @@ -132,14 +133,6 @@ func (r *KeptnWorkloadVersionReconciler) Reconcile(ctx context.Context, req ctrl return result.Result, err } - if r.SchedulingGatesHandler.Enabled() { - // pre-evaluation checks done at this moment, we can remove the gate - if err := r.SchedulingGatesHandler.RemoveGates(ctx, workloadVersion); err != nil { - r.Log.Error(err, "could not remove SchedulingGates") - return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, err - } - } - // Wait for deployment of Workload if result, err := r.doDeploymentPhase(ctx, workloadVersion, ctxWorkloadTrace); !result.Continue { return result.Result, err @@ -287,10 +280,16 @@ func (r *KeptnWorkloadVersionReconciler) SetupWithManager(mgr ctrl.Manager) erro }); err != nil { return err } - return ctrl.NewControllerManagedBy(mgr). + if err := mgr.GetFieldIndexer().IndexField(context.Background(), &klcv1beta1.KeptnWorkloadVersion{}, resourceReferenceUIDField, func(rawObj client.Object) []string { + return controllercommon.KeptnWorkloadVersionResourceRefUIDIndexFunc(rawObj) + }); err != nil { + return err + } + controllerBuilder := ctrl.NewControllerManagedBy(mgr). // predicate disabling the auto reconciliation after updating the object status - For(&klcv1beta1.KeptnWorkloadVersion{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). - Complete(r) + For(&klcv1beta1.KeptnWorkloadVersion{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})) + + return controllerBuilder.Complete(r) } func (r *KeptnWorkloadVersionReconciler) sendUnfinishedPreEvaluationEvents(appPreEvalStatus apicommon.KeptnState, phase apicommon.KeptnPhaseType, workloadVersion *klcv1beta1.KeptnWorkloadVersion) { diff --git a/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller_test.go b/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller_test.go index 05a72811948..d52f849148f 100644 --- a/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller_test.go +++ b/lifecycle-operator/controllers/lifecycle/keptnworkloadversion/controller_test.go @@ -16,7 +16,6 @@ import ( "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/eventsender" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/phase" phasefake "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/phase/fake" - schedulinggatesfake "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/schedulinggates/fake" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/telemetry" telemetryfake "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/telemetry/fake" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/testcommon" @@ -794,11 +793,6 @@ func TestKeptnWorkloadVersionReconciler_ReconcileReachCompletion(t *testing.T) { }, ) r, eventChannel, _ := setupReconciler(wi, app) - r.SchedulingGatesHandler = &schedulinggatesfake.ISchedulingGatesHandlerMock{ - EnabledFunc: func() bool { - return false - }, - } req := ctrl.Request{ NamespacedName: types.NamespacedName{ @@ -840,168 +834,6 @@ func TestKeptnWorkloadVersionReconciler_ReconcileReachCompletion(t *testing.T) { require.Equal(t, "test", metadata["testy"]) } -func TestKeptnWorkloadVersionReconciler_ReconcileReachCompletion_SchedulingGates(t *testing.T) { - - testNamespace := "some-ns" - - wi := &klcv1beta1.KeptnWorkloadVersion{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Name: "some-wi", - Namespace: testNamespace, - }, - Spec: klcv1beta1.KeptnWorkloadVersionSpec{ - KeptnWorkloadSpec: klcv1beta1.KeptnWorkloadSpec{ - AppName: "some-app", - Version: "1.0.0", - }, - WorkloadName: "some-app-some-workload", - PreviousVersion: "", - TraceId: nil, - }, - Status: klcv1beta1.KeptnWorkloadVersionStatus{ - DeploymentStatus: apicommon.StateSucceeded, - PreDeploymentStatus: apicommon.StateSucceeded, - PostDeploymentStatus: apicommon.StateSucceeded, - PreDeploymentEvaluationStatus: apicommon.StateSucceeded, - PostDeploymentEvaluationStatus: apicommon.StateSucceeded, - CurrentPhase: apicommon.PhaseWorkloadPostEvaluation.ShortName, - Status: apicommon.StateSucceeded, - StartTime: metav1.Time{}, - EndTime: metav1.Time{}, - }, - } - - app := testcommon.ReturnAppVersion( - testNamespace, - "some-app", - "1.0.0", - []klcv1beta1.KeptnWorkloadRef{ - { - Name: "some-workload", - Version: "1.0.0", - }, - }, - klcv1beta1.KeptnAppVersionStatus{ - PreDeploymentEvaluationStatus: apicommon.StateSucceeded, - }, - ) - - schedulingGatesMock := &schedulinggatesfake.ISchedulingGatesHandlerMock{ - RemoveGatesFunc: func(ctx context.Context, workloadVersion *klcv1beta1.KeptnWorkloadVersion) error { - return nil - }, - EnabledFunc: func() bool { - return true - }, - } - r, eventChannel, _ := setupReconciler(wi, app) - r.SchedulingGatesHandler = schedulingGatesMock - - req := ctrl.Request{ - NamespacedName: types.NamespacedName{ - Namespace: testNamespace, - Name: "some-wi", - }, - } - - result, err := r.Reconcile(context.TODO(), req) - - require.Len(t, schedulingGatesMock.RemoveGatesCalls(), 1) - require.Nil(t, err) - - // do not requeue since we reached completion - require.False(t, result.Requeue) - - // here we do not expect an event about the application preEvaluation being finished since that will have been sent in - // one of the previous reconciliation loops that lead to the first phase being reached - expectedEvents := []string{ - "CompletedFinished", - } - - for _, e := range expectedEvents { - select { - case event := <-eventChannel: - assert.Equal(t, strings.Contains(event, req.Name), true, "wrong workloadVersion") - assert.Equal(t, strings.Contains(event, req.Namespace), true, "wrong namespace") - assert.Equal(t, strings.Contains(event, e), true, fmt.Sprintf("no %s found in %s", e, event)) - case <-time.After(5 * time.Second): - t.Error("Didn't receive the cloud event") - } - } -} - -func TestKeptnWorkloadVersionReconciler_RemoveGates_fail(t *testing.T) { - - testNamespace := "some-ns" - - wi := &klcv1beta1.KeptnWorkloadVersion{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Name: "some-wi", - Namespace: testNamespace, - }, - Spec: klcv1beta1.KeptnWorkloadVersionSpec{ - KeptnWorkloadSpec: klcv1beta1.KeptnWorkloadSpec{ - AppName: "some-app", - Version: "1.0.0", - }, - WorkloadName: "some-app-some-workload", - PreviousVersion: "", - TraceId: nil, - }, - Status: klcv1beta1.KeptnWorkloadVersionStatus{ - DeploymentStatus: apicommon.StateSucceeded, - PreDeploymentStatus: apicommon.StateSucceeded, - PostDeploymentStatus: apicommon.StateSucceeded, - PreDeploymentEvaluationStatus: apicommon.StateSucceeded, - PostDeploymentEvaluationStatus: apicommon.StateSucceeded, - CurrentPhase: apicommon.PhaseWorkloadPostEvaluation.ShortName, - Status: apicommon.StateSucceeded, - StartTime: metav1.Time{}, - EndTime: metav1.Time{}, - }, - } - - app := testcommon.ReturnAppVersion( - testNamespace, - "some-app", - "1.0.0", - []klcv1beta1.KeptnWorkloadRef{ - { - Name: "some-workload", - Version: "1.0.0", - }, - }, - klcv1beta1.KeptnAppVersionStatus{ - PreDeploymentEvaluationStatus: apicommon.StateSucceeded, - }, - ) - r, _, _ := setupReconciler(wi, app) - r.SchedulingGatesHandler = &schedulinggatesfake.ISchedulingGatesHandlerMock{ - RemoveGatesFunc: func(ctx context.Context, workloadVersion *klcv1beta1.KeptnWorkloadVersion) error { - return fmt.Errorf("err") - }, - EnabledFunc: func() bool { - return true - }, - } - - req := ctrl.Request{ - NamespacedName: types.NamespacedName{ - Namespace: testNamespace, - Name: "some-wi", - }, - } - - result, err := r.Reconcile(context.TODO(), req) - - require.NotNil(t, err) - - // do not requeue since we reached completion - require.True(t, result.Requeue) -} - func TestKeptnWorkloadVersionReconciler_ReconcileFailed(t *testing.T) { testNamespace := "some-ns" diff --git a/lifecycle-operator/controllers/lifecycle/schedulinggates/controller.go b/lifecycle-operator/controllers/lifecycle/schedulinggates/controller.go new file mode 100644 index 00000000000..2f741243cde --- /dev/null +++ b/lifecycle-operator/controllers/lifecycle/schedulinggates/controller.go @@ -0,0 +1,114 @@ +package schedulinggates + +import ( + "context" + "fmt" + "time" + + "github.com/go-logr/logr" + klcv1beta1 "github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1beta1" + apicommon "github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1beta1/common" + controllercommon "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +// SchedulingGatesReconciler reconciles a KeptnWorkloadVersion object +type SchedulingGatesReconciler struct { + client.Client + Scheme *runtime.Scheme + Log logr.Logger +} + +// +kubebuilder:rbac:groups=lifecycle.keptn.sh,resources=keptnworkloadversions,verbs=get;list;watch; +// +kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch;update + +func (r *SchedulingGatesReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + requestInfo := controllercommon.GetRequestInfo(req) + r.Log.Info("Searching for pod", "requestInfo", requestInfo) + + pod := &v1.Pod{} + + err := r.Get(ctx, req.NamespacedName, pod) + if errors.IsNotFound(err) { + return ctrl.Result{}, nil + } + + if err != nil { + r.Log.Error(err, "Could not retrieve pod", "requestInfo", requestInfo) + return ctrl.Result{}, fmt.Errorf("could not retrieve pod, %w", err) + } + + // check if the owner of the pod is the one that the KeptnWorkloadVersion is referring to + owner := pod.GetOwnerReferences() + if len(owner) == 0 { + return ctrl.Result{}, nil + } + listOps := &client.ListOptions{ + FieldSelector: fields.OneTermEqualSelector(".spec.resourceReference.uid", string(owner[0].UID)), + Namespace: pod.GetNamespace(), + } + + attachedWorkloadVersions := &klcv1beta1.KeptnWorkloadVersionList{} + + if err := r.List(ctx, attachedWorkloadVersions, listOps); err != nil { + r.Log.Error(err, "Could not list WorkloadVersions related to pod", "pod", pod.GetName(), "namespace", pod.GetNamespace()) + return ctrl.Result{}, err + } + + for _, workloadVersion := range attachedWorkloadVersions.Items { + if workloadVersion.Status.DeploymentStatus.IsCompleted() || workloadVersion.Status.DeploymentStatus == apicommon.StateProgressing { + return r.removeGate(ctx, pod) + } + } + return ctrl.Result{RequeueAfter: 10 * time.Second}, nil + +} + +func (r *SchedulingGatesReconciler) removeGate(ctx context.Context, pod *v1.Pod) (ctrl.Result, error) { + pod.Spec.SchedulingGates = nil + if len(pod.Annotations) == 0 { + pod.Annotations = make(map[string]string, 1) + } + pod.Annotations[apicommon.SchedulingGateRemoved] = "true" + r.Log.Info("removing scheduling gate of pod", "pod", pod.Name, "uid", pod.UID) + + if err := r.Update(ctx, pod); err != nil { + r.Log.Error(err, "Could not remove pod scheduling gate", "namespace", pod.Namespace, "pod", pod.Name) + return ctrl.Result{}, err + } + return ctrl.Result{}, nil +} + +// SetupWithManager sets up the controller with the Manager. +func (r *SchedulingGatesReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For( + &v1.Pod{}, + builder.WithPredicates( + predicate.NewPredicateFuncs(func(object client.Object) bool { + pod, ok := object.(*v1.Pod) + if !ok { + return false + } + return hasKeptnSchedulingGate(pod) + }), + ), + ). + Complete(r) +} + +func hasKeptnSchedulingGate(pod *v1.Pod) bool { + for _, gate := range pod.Spec.SchedulingGates { + if gate.Name == apicommon.KeptnGate { + return true + } + } + return false +} diff --git a/lifecycle-operator/controllers/lifecycle/schedulinggates/controller_test.go b/lifecycle-operator/controllers/lifecycle/schedulinggates/controller_test.go new file mode 100644 index 00000000000..ed285992fc4 --- /dev/null +++ b/lifecycle-operator/controllers/lifecycle/schedulinggates/controller_test.go @@ -0,0 +1,337 @@ +package schedulinggates + +import ( + "context" + "errors" + "testing" + "time" + + klcv1beta1 "github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1beta1" + apicommon "github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1beta1/common" + "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common" + "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/scheme" + controllerruntime "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + k8sfake "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/log/zap" +) + +func TestSchedulingGatesReconciler_Reconcile(t *testing.T) { + podMeta := metav1.ObjectMeta{ + Name: "my-pod", + Namespace: "my-namespace", + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "ReplicaSet", + Name: "my-replica-set", + UID: "some-uid", + }, + }, + } + + req := controllerruntime.Request{ + NamespacedName: types.NamespacedName{ + Namespace: podMeta.Namespace, + Name: podMeta.Name, + }, + } + + type args struct { + ctx context.Context + req controllerruntime.Request + } + tests := []struct { + name string + objects []client.Object + pod client.Object + args args + want controllerruntime.Result + lookupError bool + updateError bool + expectGatesRemoved bool + wantErr bool + }{ + { + name: "no pod found", + objects: []client.Object{}, + args: args{ + ctx: context.TODO(), + req: req, + }, + want: controllerruntime.Result{}, + wantErr: false, + }, + { + name: "no owner references", + objects: []client.Object{ + &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-pod", + Namespace: "my-namespace", + }, + Spec: v1.PodSpec{ + SchedulingGates: []v1.PodSchedulingGate{ + { + Name: apicommon.KeptnGate, + }, + }, + }, + }, + }, + args: args{ + ctx: context.TODO(), + req: req, + }, + want: controllerruntime.Result{}, + wantErr: false, + }, + { + name: "no related WorkloadVersions", + objects: []client.Object{ + &v1.Pod{ + ObjectMeta: podMeta, + Spec: v1.PodSpec{ + SchedulingGates: []v1.PodSchedulingGate{ + { + Name: apicommon.KeptnGate, + }, + }, + }, + }, + }, + args: args{ + ctx: context.TODO(), + req: req, + }, + want: controllerruntime.Result{ + RequeueAfter: 10 * time.Second, + }, + wantErr: false, + }, + { + name: "error when executing list request", + objects: []client.Object{ + &v1.Pod{ + ObjectMeta: podMeta, + Spec: v1.PodSpec{ + SchedulingGates: []v1.PodSchedulingGate{ + { + Name: apicommon.KeptnGate, + }, + }, + }, + }, + }, + args: args{ + ctx: context.TODO(), + req: req, + }, + lookupError: true, + want: controllerruntime.Result{}, + wantErr: true, + }, + { + name: "related WorkloadVersion is completed", + objects: []client.Object{ + &klcv1beta1.KeptnWorkloadVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-wlv", + Namespace: "my-namespace", + }, + Spec: klcv1beta1.KeptnWorkloadVersionSpec{ + KeptnWorkloadSpec: klcv1beta1.KeptnWorkloadSpec{ + ResourceReference: klcv1beta1.ResourceReference{ + UID: podMeta.OwnerReferences[0].UID, + }, + }, + }, + Status: klcv1beta1.KeptnWorkloadVersionStatus{DeploymentStatus: apicommon.StateSucceeded}, + }, + &klcv1beta1.KeptnWorkloadVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-other-wlv", + Namespace: "my-namespace", + }, + Spec: klcv1beta1.KeptnWorkloadVersionSpec{}, + }, + &klcv1beta1.KeptnWorkloadVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-wlv", + Namespace: "my-other-namespace", + }, + Spec: klcv1beta1.KeptnWorkloadVersionSpec{ + KeptnWorkloadSpec: klcv1beta1.KeptnWorkloadSpec{ + ResourceReference: klcv1beta1.ResourceReference{ + UID: podMeta.OwnerReferences[0].UID, + }, + }, + }, + }, + &v1.Pod{ + ObjectMeta: podMeta, + Spec: v1.PodSpec{ + SchedulingGates: []v1.PodSchedulingGate{ + { + Name: apicommon.KeptnGate, + }, + }, + }, + }, + }, + args: args{ + ctx: context.TODO(), + req: req, + }, + want: controllerruntime.Result{}, + wantErr: false, + expectGatesRemoved: true, + }, + { + name: "related WorkloadVersion is completed - error during update", + objects: []client.Object{ + &klcv1beta1.KeptnWorkloadVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-wlv", + Namespace: "my-namespace", + }, + Spec: klcv1beta1.KeptnWorkloadVersionSpec{ + KeptnWorkloadSpec: klcv1beta1.KeptnWorkloadSpec{ + ResourceReference: klcv1beta1.ResourceReference{ + UID: podMeta.OwnerReferences[0].UID, + }, + }, + }, + Status: klcv1beta1.KeptnWorkloadVersionStatus{DeploymentStatus: apicommon.StateSucceeded}, + }, + &v1.Pod{ + ObjectMeta: podMeta, + Spec: v1.PodSpec{ + SchedulingGates: []v1.PodSchedulingGate{ + { + Name: apicommon.KeptnGate, + }, + }, + }, + }, + }, + args: args{ + ctx: context.TODO(), + req: req, + }, + updateError: true, + want: controllerruntime.Result{}, + wantErr: true, + expectGatesRemoved: false, + }, + { + name: "related WorkloadVersion is not completed", + objects: []client.Object{ + &klcv1beta1.KeptnWorkloadVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-wlv", + Namespace: "my-namespace", + }, + Spec: klcv1beta1.KeptnWorkloadVersionSpec{ + KeptnWorkloadSpec: klcv1beta1.KeptnWorkloadSpec{ + ResourceReference: klcv1beta1.ResourceReference{ + UID: podMeta.OwnerReferences[0].UID, + }, + }, + }, + Status: klcv1beta1.KeptnWorkloadVersionStatus{DeploymentStatus: apicommon.StatePending}, + }, + &v1.Pod{ + ObjectMeta: podMeta, + Spec: v1.PodSpec{ + SchedulingGates: []v1.PodSchedulingGate{ + { + Name: apicommon.KeptnGate, + }, + }, + }, + }, + }, + args: args{ + ctx: context.TODO(), + req: req, + }, + want: controllerruntime.Result{RequeueAfter: 10 * time.Second}, + wantErr: false, + expectGatesRemoved: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := klcv1beta1.AddToScheme(scheme.Scheme) + require.Nil(t, err) + opts := zap.Options{ + Development: true, + } + controllerruntime.SetLogger(zap.New(zap.UseFlagOptions(&opts))) + mockClient := k8sfake. + NewClientBuilder(). + WithScheme(scheme.Scheme). + WithObjects(tt.objects...). + WithStatusSubresource(&klcv1beta1.KeptnWorkloadVersion{}). + WithIndex(&klcv1beta1.KeptnWorkloadVersion{}, ".spec.resourceReference.uid", func(object client.Object) []string { + return common.KeptnWorkloadVersionResourceRefUIDIndexFunc(object) + }). + WithInterceptorFuncs( + interceptor.Funcs{ + List: func(ctx context.Context, client client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + if tt.lookupError { + return errors.New("unexpected error") + } + return client.List(ctx, list, opts...) + }, + Update: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.UpdateOption) error { + if tt.updateError { + return errors.New("unexpected error") + } + return client.Update(ctx, obj, opts...) + }}). + Build() + + for _, obj := range tt.objects { + kwv, ok := obj.(*klcv1beta1.KeptnWorkloadVersion) + if ok && kwv.Status.DeploymentStatus != "" { + err := mockClient.Status().Update(context.TODO(), kwv) + require.Nil(t, err) + } + } + + r := &SchedulingGatesReconciler{ + Client: mockClient, + Scheme: scheme.Scheme, + Log: controllerruntime.Log.WithName("test-appController"), + } + + got, err := r.Reconcile(tt.args.ctx, tt.args.req) + if tt.wantErr { + require.NotNil(t, err) + } else { + require.Nil(t, err) + } + require.Equal(t, tt.want, got) + + resultingPod := &v1.Pod{} + + if tt.expectGatesRemoved { + err = mockClient.Get(context.TODO(), types.NamespacedName{ + Namespace: podMeta.Namespace, + Name: podMeta.Name, + }, resultingPod) + + require.Nil(t, err) + + require.Empty(t, resultingPod.Spec.SchedulingGates) + require.Equal(t, "true", resultingPod.Annotations[apicommon.SchedulingGateRemoved]) + } + }) + } +} diff --git a/lifecycle-operator/main.go b/lifecycle-operator/main.go index 986f6f21e62..91f5fe33d8e 100644 --- a/lifecycle-operator/main.go +++ b/lifecycle-operator/main.go @@ -40,7 +40,6 @@ import ( "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/evaluation" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/eventsender" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/phase" - "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/schedulinggates" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/telemetry" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/lifecycle/keptnapp" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/lifecycle/keptnappcreationrequest" @@ -50,6 +49,7 @@ import ( "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/lifecycle/keptntaskdefinition" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/lifecycle/keptnworkload" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/lifecycle/keptnworkloadversion" + "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/lifecycle/schedulinggates" controlleroptions "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/options" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/webhooks/pod_mutator" "github.com/prometheus/client_golang/prometheus/promhttp" @@ -99,6 +99,7 @@ type envConfig struct { KeptnTaskDefinitionControllerLogLevel int `envconfig:"KEPTN_TASK_DEFINITION_CONTROLLER_LOG_LEVEL" default:"0"` KeptnWorkloadControllerLogLevel int `envconfig:"KEPTN_WORKLOAD_CONTROLLER_LOG_LEVEL" default:"0"` KeptnWorkloadVersionControllerLogLevel int `envconfig:"KEPTN_WORKLOAD_VERSION_CONTROLLER_LOG_LEVEL" default:"0"` + KeptnSchedulingGatesControllerLogLevel int `envconfig:"KEPTN_SCHEDULING_GATES_CONTROLLER_LOG_LEVEL" default:"0"` KeptnDoraMetricsPort int `envconfig:"KEPTN_DORA_METRICS_PORT" default:"2222"` KeptnOptionsControllerLogLevel int `envconfig:"OPTIONS_CONTROLLER_LOG_LEVEL" default:"0"` @@ -292,16 +293,15 @@ func main() { spanHandler, ) workloadVersionReconciler := &keptnworkloadversion.KeptnWorkloadVersionReconciler{ - SchedulingGatesHandler: schedulinggates.NewHandler(mgr.GetClient(), workloadVersionLogger, env.SchedulingGatesEnabled), - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Log: workloadVersionLogger, - EventSender: workloadVersionEventSender, - Meters: keptnMeters, - TracerFactory: telemetry.GetOtelInstance(), - SpanHandler: spanHandler, - EvaluationHandler: workloadVersionEvaluationHandler, - PhaseHandler: workloadVersionPhaseHandler, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: workloadVersionLogger, + EventSender: workloadVersionEventSender, + Meters: keptnMeters, + TracerFactory: telemetry.GetOtelInstance(), + SpanHandler: spanHandler, + EvaluationHandler: workloadVersionEvaluationHandler, + PhaseHandler: workloadVersionPhaseHandler, } if err = (workloadVersionReconciler).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "KeptnWorkloadVersion") @@ -366,6 +366,20 @@ func main() { os.Exit(1) } + schedulingGatesLogger := ctrl.Log.WithName("SchedulingGates Controller").V(env.KeptnSchedulingGatesControllerLogLevel) + if env.SchedulingGatesEnabled { + schedulingGatesReconciler := &schedulinggates.SchedulingGatesReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: schedulingGatesLogger, + } + + if err := schedulingGatesReconciler.SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "SchedulingGates") + os.Exit(1) + } + } + if err = (&lifecyclev1beta1.KeptnApp{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "KeptnApp") os.Exit(1) diff --git a/lifecycle-operator/test/component/workloadversion/workloadversion_suite_test.go b/lifecycle-operator/test/component/workloadversion/workloadversion_suite_test.go index 5e7b5319d6b..6b59ec4b5c0 100644 --- a/lifecycle-operator/test/component/workloadversion/workloadversion_suite_test.go +++ b/lifecycle-operator/test/component/workloadversion/workloadversion_suite_test.go @@ -10,7 +10,6 @@ import ( "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/evaluation" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/eventsender" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/phase" - "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/schedulinggates" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/telemetry" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/lifecycle/keptnworkloadversion" "github.com/keptn/lifecycle-toolkit/lifecycle-operator/test/component/common" @@ -65,16 +64,15 @@ var _ = BeforeSuite(func() { // //setup controllers here config.Instance().SetDefaultNamespace(KeptnNamespace) controller := &keptnworkloadversion.KeptnWorkloadVersionReconciler{ - SchedulingGatesHandler: schedulinggates.NewHandler(nil, GinkgoLogr, false), - Client: k8sManager.GetClient(), - Scheme: k8sManager.GetScheme(), - EventSender: eventSender, - Log: GinkgoLogr, - Meters: common.InitKeptnMeters(), - SpanHandler: &telemetry.Handler{}, - TracerFactory: tracerFactory, - EvaluationHandler: evaluationHandler, - PhaseHandler: phaseHandler, + Client: k8sManager.GetClient(), + Scheme: k8sManager.GetScheme(), + EventSender: eventSender, + Log: GinkgoLogr, + Meters: common.InitKeptnMeters(), + SpanHandler: &telemetry.Handler{}, + TracerFactory: tracerFactory, + EvaluationHandler: evaluationHandler, + PhaseHandler: phaseHandler, } Eventually(controller.SetupWithManager(k8sManager)).WithTimeout(30 * time.Second).WithPolling(time.Second).Should(Succeed()) close(readyToStart) diff --git a/test/chainsaw/scheduling-gates/simple-deployment-restart-pod/00-assert.yaml b/test/chainsaw/scheduling-gates/simple-deployment-restart-pod/00-assert.yaml new file mode 100755 index 00000000000..e48de2eb59e --- /dev/null +++ b/test/chainsaw/scheduling-gates/simple-deployment-restart-pod/00-assert.yaml @@ -0,0 +1,20 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: test + name: test +status: + readyReplicas: 1 +--- +apiVersion: v1 +kind: Pod +metadata: + annotations: + keptn.sh/scheduling-gate-removed: "true" + labels: + app: test +spec: + schedulerName: default-scheduler +status: + phase: Running diff --git a/test/chainsaw/scheduling-gates/simple-deployment-restart-pod/00-install.yaml b/test/chainsaw/scheduling-gates/simple-deployment-restart-pod/00-install.yaml new file mode 100644 index 00000000000..a6154eb23c9 --- /dev/null +++ b/test/chainsaw/scheduling-gates/simple-deployment-restart-pod/00-install.yaml @@ -0,0 +1,35 @@ +apiVersion: lifecycle.keptn.sh/v1beta1 +kind: KeptnTaskDefinition +metadata: + name: pre-deployment-hello +spec: + function: + inline: + code: | + console.log("Pre-Deployment Task has been executed"); +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: test + name: test +spec: + replicas: 1 + selector: + matchLabels: + app: test + strategy: {} + template: + metadata: + labels: + app: test + annotations: + keptn.sh/workload: waiter + keptn.sh/version: "0.4" + keptn.sh/pre-deployment-tasks: pre-deployment-hello + spec: + containers: + - image: busybox + name: busybox + command: ['sh', '-c', 'echo The app is running! && sleep infinity'] diff --git a/test/chainsaw/scheduling-gates/simple-deployment-restart-pod/01-assert.yaml b/test/chainsaw/scheduling-gates/simple-deployment-restart-pod/01-assert.yaml new file mode 100755 index 00000000000..7f5309c7d06 --- /dev/null +++ b/test/chainsaw/scheduling-gates/simple-deployment-restart-pod/01-assert.yaml @@ -0,0 +1,36 @@ +apiVersion: lifecycle.keptn.sh/v1beta1 +kind: KeptnWorkload +metadata: + name: waiter-waiter +--- +apiVersion: lifecycle.keptn.sh/v1beta1 +kind: KeptnWorkloadVersion +metadata: + name: waiter-waiter-0.4 +status: + currentPhase: Completed + deploymentStatus: Succeeded + postDeploymentEvaluationStatus: Succeeded + postDeploymentStatus: Succeeded + preDeploymentEvaluationStatus: Succeeded + preDeploymentStatus: Succeeded + preDeploymentTaskStatus: + - definitionName: pre-deployment-hello + status: Succeeded +--- +apiVersion: lifecycle.keptn.sh/v1beta1 +kind: KeptnApp +metadata: + name: waiter +--- +apiVersion: lifecycle.keptn.sh/v1beta1 +kind: KeptnAppVersion +metadata: + name: waiter-0.4-6b86b273 +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test +status: + readyReplicas: 1 diff --git a/test/chainsaw/scheduling-gates/simple-deployment-restart-pod/02-assert.yaml b/test/chainsaw/scheduling-gates/simple-deployment-restart-pod/02-assert.yaml new file mode 100755 index 00000000000..0bee1b6f5e4 --- /dev/null +++ b/test/chainsaw/scheduling-gates/simple-deployment-restart-pod/02-assert.yaml @@ -0,0 +1,6 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test +status: + readyReplicas: 2 diff --git a/test/chainsaw/scheduling-gates/simple-deployment-restart-pod/chainsaw-test.yaml b/test/chainsaw/scheduling-gates/simple-deployment-restart-pod/chainsaw-test.yaml new file mode 100755 index 00000000000..33eb63e3514 --- /dev/null +++ b/test/chainsaw/scheduling-gates/simple-deployment-restart-pod/chainsaw-test.yaml @@ -0,0 +1,56 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + creationTimestamp: null + name: simple-deployment-restart-pod +spec: + steps: + - name: step-00 + try: + - script: + content: kubectl annotate ns $NAMESPACE keptn.sh/lifecycle-toolkit='enabled' + - apply: + file: 00-install.yaml + - assert: + file: 00-assert.yaml + catch: + - podLogs: + selector: app=test + - script: + content: kubectl describe keptnworkloadversion -n $NAMESPACE + - script: + content: kubectl describe keptnappversion -n $NAMESPACE + - podLogs: + namespace: keptn-system + selector: control-plane=lifecycle-operator + - name: step-01 + try: + - assert: + file: 01-assert.yaml + catch: + - podLogs: + selector: app=test + - script: + content: kubectl describe keptnworkloadversion -n $NAMESPACE + - script: + content: kubectl describe keptnappversion -n $NAMESPACE + - podLogs: + namespace: keptn-system + selector: control-plane=lifecycle-operator + - name: step-02 + try: + - script: + content: kubectl -n $NAMESPACE scale deployment test --replicas=2 + - assert: + file: 02-assert.yaml + catch: + - podLogs: + selector: app=test + - script: + content: kubectl describe keptnworkloadversion -n $NAMESPACE + - script: + content: kubectl describe keptnappversion -n $NAMESPACE + - podLogs: + namespace: keptn-system + selector: control-plane=lifecycle-operator