From 8766d1503157a6a05027266eb02c77bd388936a7 Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Wed, 12 Jun 2024 14:54:23 +0200 Subject: [PATCH 01/25] Remove custom CR state check, only check for deployment state and map pod state to amnifest CR --- internal/controller/manifest/controller.go | 2 +- internal/declarative/v2/ready_check.go | 3 +- internal/manifest/ready_check.go | 236 +++------- internal/manifest/ready_check_test.go | 445 ------------------ .../custom_resource_check/suite_test.go | 2 +- .../controller/manifest/ready_check_test.go | 2 +- 6 files changed, 61 insertions(+), 629 deletions(-) delete mode 100644 internal/manifest/ready_check_test.go diff --git a/internal/controller/manifest/controller.go b/internal/controller/manifest/controller.go index a73f02056c..be6af3b9be 100644 --- a/internal/controller/manifest/controller.go +++ b/internal/controller/manifest/controller.go @@ -26,7 +26,7 @@ func NewReconciler(mgr manager.Manager, declarativev2.WithSpecResolver( manifest.NewSpecResolver(kcp, extractor), ), - declarativev2.WithCustomReadyCheck(manifest.NewCustomResourceReadyCheck()), + declarativev2.WithCustomReadyCheck(manifest.NewDeploymentReadyCheck()), declarativev2.WithRemoteTargetCluster(lookup.ConfigResolver), manifest.WithClientCacheKey(), declarativev2.WithPostRun{manifest.PostRunCreateCR}, diff --git a/internal/declarative/v2/ready_check.go b/internal/declarative/v2/ready_check.go index 69a0c1d86a..dde1daa65f 100644 --- a/internal/declarative/v2/ready_check.go +++ b/internal/declarative/v2/ready_check.go @@ -19,7 +19,7 @@ type StateInfo struct { } type ReadyCheck interface { - Run(ctx context.Context, clnt Client, obj Object, resources []*resource.Info) (StateInfo, error) + Run(ctx context.Context, clnt Client, resources []*resource.Info) (StateInfo, error) } func NewExistsReadyCheck() *ExistsReadyCheck { @@ -31,7 +31,6 @@ type ExistsReadyCheck struct{} func (c *ExistsReadyCheck) Run( ctx context.Context, clnt Client, - _ Object, resources []*resource.Info, ) (StateInfo, error) { for i := range resources { diff --git a/internal/manifest/ready_check.go b/internal/manifest/ready_check.go index 9c133e20f8..a3112414f8 100644 --- a/internal/manifest/ready_check.go +++ b/internal/manifest/ready_check.go @@ -2,23 +2,17 @@ package manifest import ( "context" - "encoding/json" "errors" - "fmt" - "strings" "time" + "github.com/kyma-project/lifecycle-manager/api/shared" + declarativev2 "github.com/kyma-project/lifecycle-manager/internal/declarative/v2" apiappsv1 "k8s.io/api/apps/v1" apicorev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + k8slabels "k8s.io/apimachinery/pkg/labels" "k8s.io/cli-runtime/pkg/resource" "k8s.io/kubectl/pkg/util/deployment" "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/kyma-project/lifecycle-manager/api/shared" - "github.com/kyma-project/lifecycle-manager/api/v1beta2" - declarativev2 "github.com/kyma-project/lifecycle-manager/internal/declarative/v2" - "github.com/kyma-project/lifecycle-manager/pkg/util" ) const ( @@ -28,201 +22,58 @@ const ( ModuleCRWithNoCustomCheckWarning = "module CR state not found" ) -// NewCustomResourceReadyCheck creates a readiness check that verifies that the Resource in the Manifest +// NewDeploymentReadyCheck creates a readiness check that verifies that the Resource in the Manifest // returns the ready state, if not it returns not ready. -func NewCustomResourceReadyCheck() *CustomResourceReadyCheck { - return &CustomResourceReadyCheck{} +func NewDeploymentReadyCheck() *DeploymentReadyCheck { + return &DeploymentReadyCheck{} } -type CustomResourceReadyCheck struct{} +type DeploymentReadyCheck struct{} var ( - ErrNotSupportedState = errors.New("module CR state not support") - ErrRequiredStateMissing = errors.New("required Ready and Error state mapping are missing") + ErrNotSupportedState = errors.New("module CR state not support") ) -func (c *CustomResourceReadyCheck) Run(ctx context.Context, +func (c *DeploymentReadyCheck) Run(ctx context.Context, clnt declarativev2.Client, - obj declarativev2.Object, resources []*resource.Info, ) (declarativev2.StateInfo, error) { - if !isDeploymentReady(clnt, resources) { - return declarativev2.StateInfo{ - State: shared.StateProcessing, - Info: "module operator deployment is not ready", - }, nil - } - manifest, ok := obj.(*v1beta2.Manifest) - if !ok { - return declarativev2.StateInfo{State: shared.StateError}, v1beta2.ErrTypeAssertManifest - } - if manifest.Spec.Resource == nil { - return declarativev2.StateInfo{State: shared.StateReady}, nil - } - moduleCR := manifest.Spec.Resource.DeepCopy() - - err := clnt.Get(ctx, client.ObjectKeyFromObject(moduleCR), moduleCR) - if err != nil { - if util.IsNotFound(err) && !manifest.DeletionTimestamp.IsZero() { - return declarativev2.StateInfo{State: shared.StateDeleting}, nil - } - return declarativev2.StateInfo{State: shared.StateError}, fmt.Errorf("failed to fetch resource: %w", err) - } - return HandleState(manifest, moduleCR) -} - -func HandleState(manifest *v1beta2.Manifest, moduleCR *unstructured.Unstructured) (declarativev2.StateInfo, error) { - stateChecks, customStateFound, err := parseStateChecks(manifest) - if err != nil { - return declarativev2.StateInfo{State: shared.StateError}, fmt.Errorf( - "could not get state from module CR %s to determine readiness: %w", - moduleCR.GetName(), err, - ) - } - typedState, stateExists, err := mappingState(stateChecks, moduleCR, customStateFound) - if err != nil { - // Only happens for kyma module CR - if errors.Is(err, ErrNotSupportedState) { - return declarativev2.StateInfo{ - State: shared.StateWarning, - Info: ErrNotSupportedState.Error(), - }, nil - } - return declarativev2.StateInfo{State: shared.StateError}, fmt.Errorf( - "could not get state from module CR %s to determine readiness: %w", - moduleCR.GetName(), err, - ) - } - if !stateExists { - info := ModuleCRWithNoCustomCheckWarning - if customStateFound { - info = ModuleCRWithCustomCheckWarning - } - state := shared.StateProcessing - // If wait for certain period of time, state still not found, put manifest state into Warning - if manifest.CreationTimestamp.Add(ToWarningDuration).Before(time.Now()) { - state = shared.StateWarning - } - - return declarativev2.StateInfo{State: state, Info: info}, nil - } - - return declarativev2.StateInfo{State: typedState}, nil + deploymentState := getDeploymentState(clnt, resources) + return declarativev2.StateInfo{State: deploymentState}, nil } -func mappingState(stateChecks []*v1beta2.CustomStateCheck, - moduleCR *unstructured.Unstructured, - customStateFound bool, -) (shared.State, bool, error) { - // make sure ready and error state exists, for other missing customized state, can be ignored. - if requiredStateMissing(stateChecks) { - return "", false, ErrRequiredStateMissing - } - stateResult := map[shared.State]bool{} - foundStateInCR := false - for _, stateCheck := range stateChecks { - stateFromCR, stateExists, err := unstructured.NestedString(moduleCR.Object, - strings.Split(stateCheck.JSONPath, ".")...) - if err != nil { - return "", false, fmt.Errorf("could not get state from module CR %s at path %s "+ - "to determine readiness: %w", moduleCR.GetName(), stateCheck.JSONPath, err) - } - if !stateExists { - continue - } - if !customStateFound && !shared.State(stateFromCR).IsSupportedState() { - return "", false, ErrNotSupportedState - } - foundStateInCR = true - _, found := stateResult[stateCheck.MappedState] - if found { - stateResult[stateCheck.MappedState] = stateResult[stateCheck.MappedState] && stateFromCR == stateCheck.Value - } else { - stateResult[stateCheck.MappedState] = stateFromCR == stateCheck.Value - } +func getDeploymentState(clt declarativev2.Client, resources []*resource.Info) shared.State { + deploy, found := findDeployment(clt, resources) + // Not every module operator use Deployment by default, e.g: StatefulSet also a valid approach + if !found { + return shared.StateReady } - return calculateFinalState(stateResult), foundStateInCR, nil -} -func calculateFinalState(stateResult map[shared.State]bool) shared.State { - if stateResult[shared.StateError] { - return shared.StateError - } - if stateResult[shared.StateReady] { + if isDeploymentReady(deploy) { return shared.StateReady } - if stateResult[shared.StateWarning] { - return shared.StateWarning - } - if stateResult[shared.StateDeleting] { - return shared.StateDeleting - } - - // by default, if ready/error state condition not match, assume module CR under processing - return shared.StateProcessing -} -func requiredStateMissing(stateChecks []*v1beta2.CustomStateCheck) bool { - readyMissing := true - errorMissing := true - for _, stateCheck := range stateChecks { - if stateCheck.MappedState == shared.StateReady { - readyMissing = false - } - if stateCheck.MappedState == shared.StateError { - errorMissing = false - } + // Since deployment is not ready check if pods are ready or in error state + // Get all Pods associated with the Deployment + podList, err := getPodsForDeployment(clt, deploy) + if err != nil { + return shared.StateError } - return readyMissing || errorMissing -} -func parseStateChecks(manifest *v1beta2.Manifest) ([]*v1beta2.CustomStateCheck, bool, error) { - customStateCheckAnnotation, found := manifest.Annotations[shared.CustomStateCheckAnnotation] - if !found { - return []*v1beta2.CustomStateCheck{ - { - JSONPath: customResourceStatePath, - Value: string(shared.StateReady), - MappedState: shared.StateReady, - }, - { - JSONPath: customResourceStatePath, - Value: string(shared.StateError), - MappedState: shared.StateError, - }, - { - JSONPath: customResourceStatePath, - Value: string(shared.StateDeleting), - MappedState: shared.StateDeleting, - }, - { - JSONPath: customResourceStatePath, - Value: string(shared.StateWarning), - MappedState: shared.StateWarning, - }, - }, false, nil - } - var stateCheck []*v1beta2.CustomStateCheck - if err := json.Unmarshal([]byte(customStateCheckAnnotation), &stateCheck); err != nil { - return stateCheck, true, fmt.Errorf("failed to unmarshal stateCheck: %w", err) - } - return stateCheck, true, nil + return getPodsState(podList) } -func isDeploymentReady(clt declarativev2.Client, resources []*resource.Info) bool { +func findDeployment(clt declarativev2.Client, resources []*resource.Info) (*apiappsv1.Deployment, bool) { deploy := &apiappsv1.Deployment{} - found := false for _, res := range resources { - err := clt.Scheme().Convert(res.Object, deploy, nil) - if err == nil { - found = true - break + if err := clt.Scheme().Convert(res.Object, deploy, nil); err == nil { + return deploy, true } } - // not every module operator use Deployment by default, e.g: StatefulSet also a valid approach - if !found { - return true - } + return nil, false +} + +func isDeploymentReady(deploy *apiappsv1.Deployment) bool { availableCond := deployment.GetDeploymentCondition(deploy.Status, apiappsv1.DeploymentAvailable) if availableCond != nil && availableCond.Status == apicorev1.ConditionTrue { return true @@ -232,3 +83,30 @@ func isDeploymentReady(clt declarativev2.Client, resources []*resource.Info) boo } return false } + +func getPodsForDeployment(clt declarativev2.Client, deploy *apiappsv1.Deployment) (*apicorev1.PodList, error) { + podList := &apicorev1.PodList{} + listOptions := &client.ListOptions{ + Namespace: deploy.Namespace, + LabelSelector: k8slabels.SelectorFromSet(deploy.Spec.Selector.MatchLabels), + } + if err := clt.List(context.TODO(), podList, listOptions); err != nil { + return nil, err + } + return podList, nil +} + +func getPodsState(podList *apicorev1.PodList) shared.State { + for _, pod := range podList.Items { + for _, condition := range pod.Status.ContainerStatuses { + if *condition.Started && condition.Ready { + return shared.StateReady + } else if *condition.Started && !condition.Ready { + return shared.StateProcessing + } else if !*condition.Started && !condition.Ready { + return shared.StateError + } + } + } + return shared.StateError +} diff --git a/internal/manifest/ready_check_test.go b/internal/manifest/ready_check_test.go deleted file mode 100644 index a2a2b6c23a..0000000000 --- a/internal/manifest/ready_check_test.go +++ /dev/null @@ -1,445 +0,0 @@ -package manifest_test - -import ( - "encoding/json" - "reflect" - "testing" - "time" - - apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - - "github.com/kyma-project/lifecycle-manager/api/shared" - "github.com/kyma-project/lifecycle-manager/api/v1beta2" - declarativev2 "github.com/kyma-project/lifecycle-manager/internal/declarative/v2" - "github.com/kyma-project/lifecycle-manager/internal/manifest" - "github.com/kyma-project/lifecycle-manager/pkg/testutils" - "github.com/kyma-project/lifecycle-manager/pkg/testutils/builder" -) - -func TestHandleState(t *testing.T) { - t.Parallel() - type moduleCheck struct { - fields []string - value string - } - definedValueForError := "customStateForError" - definedValueForReady := "customStateForReady" - tests := []struct { - name string - customState []*v1beta2.CustomStateCheck - customStateExpected bool - checkInModuleCR []moduleCheck - want declarativev2.StateInfo - wantErr bool - }{ - { - "kyma module with Ready state, expected mapped to StateReady", - nil, - false, - []moduleCheck{ - { - []string{"status", "state"}, - string(shared.StateReady), - }, - }, - declarativev2.StateInfo{State: shared.StateReady}, - false, - }, - { - "kyma module with unsupported State, expected mapped to StateWarning", - nil, - false, - []moduleCheck{ - { - []string{"status", "state"}, - "not support state", - }, - }, - declarativev2.StateInfo{State: shared.StateWarning, Info: manifest.ErrNotSupportedState.Error()}, - false, - }, - { - "custom module with no CustomStateCheckAnnotation, expected mapped to StateProcessing", - nil, - true, - []moduleCheck{ - { - []string{"fieldLevel1", "fieldLevel2"}, - "customState", - }, - }, - declarativev2.StateInfo{State: shared.StateProcessing, Info: manifest.ModuleCRWithNoCustomCheckWarning}, - false, - }, - { - "custom module with not all required StateCheck, expected mapped to StateError with error", - []*v1beta2.CustomStateCheck{ - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: "customState", - MappedState: shared.StateReady, - }, - }, - true, - []moduleCheck{ - { - []string{"fieldLevel1", "fieldLevel2"}, - "customState", - }, - }, - declarativev2.StateInfo{State: shared.StateError}, - true, - }, - { - "custom module found mapped value with StateReady, expected mapped to StateReady", - []*v1beta2.CustomStateCheck{ - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForReady, - MappedState: shared.StateReady, - }, - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForError, - MappedState: shared.StateError, - }, - }, - true, - []moduleCheck{ - { - []string{"fieldLevel1", "fieldLevel2"}, - definedValueForReady, - }, - }, - declarativev2.StateInfo{State: shared.StateReady}, - false, - }, - { - "custom module found mapped value with StateError, expected mapped to StateError", - []*v1beta2.CustomStateCheck{ - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForReady, - MappedState: shared.StateReady, - }, - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForError, - MappedState: shared.StateError, - }, - }, - true, - []moduleCheck{ - { - []string{"fieldLevel1", "fieldLevel2"}, - definedValueForError, - }, - }, - declarativev2.StateInfo{State: shared.StateError}, - false, - }, - { - "custom module with additional StateCheck, expected mapped to correct state", - []*v1beta2.CustomStateCheck{ - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForReady, - MappedState: shared.StateReady, - }, - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForError, - MappedState: shared.StateError, - }, - { - JSONPath: "fieldLevel3.fieldLevel4.fieldLevel5", - Value: "customStateForWarning", - MappedState: shared.StateWarning, - }, - }, - true, - []moduleCheck{ - { - []string{"fieldLevel1", "fieldLevel2"}, - "customStateWithOtherValue", - }, - { - []string{"fieldLevel3", "fieldLevel4", "fieldLevel5"}, - "customStateForWarning", - }, - }, - declarativev2.StateInfo{State: shared.StateWarning}, - false, - }, - { - "custom module with multiple StateReady condition, expected mapped to StateReady when all condition matched", - []*v1beta2.CustomStateCheck{ - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForReady, - MappedState: shared.StateReady, - }, - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForError, - MappedState: shared.StateError, - }, - { - JSONPath: "fieldLevel3.fieldLevel4.fieldLevel5", - Value: definedValueForReady, - MappedState: shared.StateReady, - }, - }, - true, - []moduleCheck{ - { - []string{"fieldLevel1", "fieldLevel2"}, - definedValueForReady, - }, - { - []string{"fieldLevel3", "fieldLevel4", "fieldLevel5"}, - definedValueForReady, - }, - }, - declarativev2.StateInfo{State: shared.StateReady}, - false, - }, - { - "custom module with multiple StateReady condition, expected " + - "mapped to StateProcessing when not all condition matched", - []*v1beta2.CustomStateCheck{ - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForReady, - MappedState: shared.StateReady, - }, - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForError, - MappedState: shared.StateError, - }, - { - JSONPath: "fieldLevel3.fieldLevel4.fieldLevel5", - Value: definedValueForReady, - MappedState: shared.StateReady, - }, - }, - true, - []moduleCheck{ - { - []string{"fieldLevel1", "fieldLevel2"}, - "not in defined value", - }, - { - []string{"fieldLevel3", "fieldLevel4", "fieldLevel5"}, - definedValueForReady, - }, - }, - declarativev2.StateInfo{State: shared.StateProcessing}, - false, - }, - { - "custom module with additional StateCheck but no mapped value found, expected mapped to StateProcessing", - []*v1beta2.CustomStateCheck{ - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForReady, - MappedState: shared.StateReady, - }, - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForError, - MappedState: shared.StateError, - }, - { - JSONPath: "fieldLevel3.fieldLevel4.fieldLevel5", - Value: "customStateForWarning", - MappedState: shared.StateWarning, - }, - }, - true, - []moduleCheck{ - { - []string{"fieldLevel1", "fieldLevel2"}, - "customStateWithOtherValue", - }, - { - []string{"fieldLevel3", "fieldLevel4", "fieldLevel5"}, - "customStateWithOtherValue", - }, - }, - declarativev2.StateInfo{State: shared.StateProcessing}, - false, - }, - { - "custom module not in mapped value, expected mapped to StateProcessing", - []*v1beta2.CustomStateCheck{ - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForReady, - MappedState: shared.StateReady, - }, - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForError, - MappedState: shared.StateError, - }, - }, - true, - []moduleCheck{ - { - []string{"fieldLevel1", "fieldLevel2"}, - "customStateWithOtherValue", - }, - }, - declarativev2.StateInfo{State: shared.StateProcessing}, - false, - }, - } - for _, testCase := range tests { - testCase := testCase - t.Run(testCase.name, func(t *testing.T) { - t.Parallel() - manifestCR := testutils.NewTestManifest("test") - if testCase.customStateExpected { - if testCase.customState != nil { - marshal, err := json.Marshal(testCase.customState) - if err != nil { - t.Errorf("HandleState() error = %v", err) - return - } - manifestCR.Annotations[shared.CustomStateCheckAnnotation] = string(marshal) - } - } - manifestCR.CreationTimestamp = apimetav1.Now() - moduleCR := builder.NewModuleCRBuilder().WithName("test").WithNamespace(apimetav1.NamespaceDefault). - WithGroupVersionKind(v1beta2.GroupVersion.Group, "v1", "TestCR").Build() - for _, check := range testCase.checkInModuleCR { - err := unstructured.SetNestedField(moduleCR.Object, check.value, check.fields...) - if err != nil { - t.Errorf("HandleState() error = %v", err) - return - } - } - got, err := manifest.HandleState(manifestCR, moduleCR) - if !reflect.DeepEqual(got, testCase.want) { - t.Errorf("HandleState() got = %v, want %v", got, testCase.want) - } - if (err != nil) != testCase.wantErr { - t.Errorf("HandleState() error = %v, wantErr %v", err, testCase.wantErr) - } - }) - } -} - -func TestHandleStateWithDuration(t *testing.T) { - t.Parallel() - type moduleCheck struct { - fields []string - value string - } - definedValueForError := "customStateForError" - definedValueForReady := "customStateForReady" - tests := []struct { - name string - customState []*v1beta2.CustomStateCheck - customStateExpected bool - manifestCreatedAt apimetav1.Time - checkInModuleCR []moduleCheck - want declarativev2.StateInfo - wantErr bool - }{ - { - "kyma module just created with no state, expected to StateProcessing", - nil, - false, - apimetav1.Now(), - nil, - declarativev2.StateInfo{State: shared.StateProcessing, Info: manifest.ModuleCRWithNoCustomCheckWarning}, - false, - }, - { - "kyma module with state updated, expected to StateReady", - nil, - false, - apimetav1.Now(), - []moduleCheck{ - { - []string{"status", "state"}, - string(shared.StateReady), - }, - }, - declarativev2.StateInfo{State: shared.StateReady}, - false, - }, - { - "kyma module with no state after certain time, expected to StateWarning", - nil, - false, - apimetav1.NewTime(apimetav1.Now().Add(-10 * time.Minute)), - nil, - declarativev2.StateInfo{State: shared.StateWarning, Info: manifest.ModuleCRWithNoCustomCheckWarning}, - false, - }, - { - "custom module with wrong JSON path after certain time, expected to StateWarning", - []*v1beta2.CustomStateCheck{ - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForReady, - MappedState: shared.StateReady, - }, - { - JSONPath: "fieldLevel1.fieldLevel2", - Value: definedValueForError, - MappedState: shared.StateError, - }, - }, - true, - apimetav1.NewTime(apimetav1.Now().Add(-10 * time.Minute)), - []moduleCheck{ - { - []string{"fieldLevel1", "fieldLevel3"}, - definedValueForReady, - }, - }, - declarativev2.StateInfo{State: shared.StateWarning, Info: manifest.ModuleCRWithCustomCheckWarning}, - false, - }, - } - for _, testCase := range tests { - testCase := testCase - t.Run(testCase.name, func(t *testing.T) { - t.Parallel() - manifestCR := testutils.NewTestManifest("test") - if testCase.customStateExpected { - if testCase.customState != nil { - marshal, err := json.Marshal(testCase.customState) - if err != nil { - t.Errorf("HandleState() error = %v", err) - return - } - manifestCR.Annotations[shared.CustomStateCheckAnnotation] = string(marshal) - } - } - manifestCR.CreationTimestamp = testCase.manifestCreatedAt - moduleCR := builder.NewModuleCRBuilder().WithName("test").WithNamespace(apimetav1.NamespaceDefault). - WithGroupVersionKind(v1beta2.GroupVersion.Group, "v1", "TestCR").Build() - for _, check := range testCase.checkInModuleCR { - err := unstructured.SetNestedField(moduleCR.Object, check.value, check.fields...) - if err != nil { - t.Errorf("HandleState() error = %v", err) - return - } - } - got, err := manifest.HandleState(manifestCR, moduleCR) - if !reflect.DeepEqual(got, testCase.want) { - t.Errorf("HandleState() got = %v, want %v", got, testCase.want) - } - if (err != nil) != testCase.wantErr { - t.Errorf("HandleState() error = %v, wantErr %v", err, testCase.wantErr) - } - }) - } -} diff --git a/tests/integration/controller/manifest/custom_resource_check/suite_test.go b/tests/integration/controller/manifest/custom_resource_check/suite_test.go index 439ecb779c..2d4ee5533f 100644 --- a/tests/integration/controller/manifest/custom_resource_check/suite_test.go +++ b/tests/integration/controller/manifest/custom_resource_check/suite_test.go @@ -149,7 +149,7 @@ var _ = BeforeSuite(func() { }, ), manifest.WithClientCacheKey(), declarativev2.WithPostRun{manifest.PostRunCreateCR}, declarativev2.WithPreDelete{manifest.PreDeleteDeleteCR}, - declarativev2.WithCustomReadyCheck(manifest.NewCustomResourceReadyCheck())) + declarativev2.WithCustomReadyCheck(manifest.NewDeploymentReadyCheck())) err = ctrl.NewControllerManagedBy(mgr). For(&v1beta2.Manifest{}). diff --git a/tests/integration/controller/manifest/ready_check_test.go b/tests/integration/controller/manifest/ready_check_test.go index fe706448c5..07bd470827 100644 --- a/tests/integration/controller/manifest/ready_check_test.go +++ b/tests/integration/controller/manifest/ready_check_test.go @@ -76,7 +76,7 @@ var _ = Describe("Manifest readiness check", Ordered, func() { Expect(resources).ToNot(BeEmpty()) By("Executing the CR readiness check") - customReadyCheck := manifest.NewCustomResourceReadyCheck() + customReadyCheck := manifest.NewDeploymentReadyCheck() stateInfo, err := customReadyCheck.Run(ctx, testClient, testManifest, resources) Expect(err).NotTo(HaveOccurred()) Expect(stateInfo.State).To(Equal(shared.StateReady)) From 249c173bc2bd68091ac1f54759db4df1f955bbdb Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Wed, 12 Jun 2024 14:56:10 +0200 Subject: [PATCH 02/25] Remove custom CR state check, only check for deployment state and map pod state to amnifest CR --- internal/declarative/v2/reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/declarative/v2/reconciler.go b/internal/declarative/v2/reconciler.go index 57f131956a..1f6f9e1388 100644 --- a/internal/declarative/v2/reconciler.go +++ b/internal/declarative/v2/reconciler.go @@ -394,7 +394,7 @@ func (r *Reconciler) checkTargetReadiness( resourceReadyCheck := r.CustomReadyCheck - crStateInfo, err := resourceReadyCheck.Run(ctx, clnt, manifest, target) + crStateInfo, err := resourceReadyCheck.Run(ctx, clnt, target) if err != nil { manifest.SetStatus(status.WithState(shared.StateError).WithErr(err)) return err From 5fbed0c90ab0422ac5e59f426e903e0a995d830d Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Wed, 12 Jun 2024 16:09:31 +0200 Subject: [PATCH 03/25] Remove arg from test func --- tests/integration/controller/manifest/ready_check_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/controller/manifest/ready_check_test.go b/tests/integration/controller/manifest/ready_check_test.go index 07bd470827..734c954477 100644 --- a/tests/integration/controller/manifest/ready_check_test.go +++ b/tests/integration/controller/manifest/ready_check_test.go @@ -77,7 +77,7 @@ var _ = Describe("Manifest readiness check", Ordered, func() { By("Executing the CR readiness check") customReadyCheck := manifest.NewDeploymentReadyCheck() - stateInfo, err := customReadyCheck.Run(ctx, testClient, testManifest, resources) + stateInfo, err := customReadyCheck.Run(ctx, testClient, resources) Expect(err).NotTo(HaveOccurred()) Expect(stateInfo.State).To(Equal(shared.StateReady)) From 6e2a04393c553adcd4496428585847fe98493d72 Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Fri, 14 Jun 2024 13:36:18 +0200 Subject: [PATCH 04/25] Fix integration test --- .../manifest/custom_resource_check/manifest_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/controller/manifest/custom_resource_check/manifest_test.go b/tests/integration/controller/manifest/custom_resource_check/manifest_test.go index 93aa0be8a5..8d722bd92f 100644 --- a/tests/integration/controller/manifest/custom_resource_check/manifest_test.go +++ b/tests/integration/controller/manifest/custom_resource_check/manifest_test.go @@ -78,8 +78,8 @@ var _ = Describe("Warning state propagation test", Ordered, func() { Eventually(setCRStatus(ctx, kcpClient, sampleCR, shared.StateWarning), standardTimeout, standardInterval).Should(Succeed()) - By("Verify the Manifest CR state also changes to \"Warning\"") - Eventually(testutils.ExpectManifestStateIn(ctx, kcpClient, shared.StateWarning), standardTimeout, + By("Verify the Manifest CR stays in the \"Ready\" state") + Eventually(testutils.ExpectManifestStateIn(ctx, kcpClient, shared.StateReady), standardTimeout, standardInterval). WithArguments(manifestName).Should(Succeed()) @@ -87,7 +87,7 @@ var _ = Describe("Warning state propagation test", Ordered, func() { Eventually(setCRStatus(ctx, kcpClient, sampleCR, shared.StateReady), standardTimeout, standardInterval).Should(Succeed()) - By("Verify the Manifest CR state changes back to \"Ready\"") + By("Verify the Manifest CR stays in the \"Ready\"") Eventually(testutils.ExpectManifestStateIn(ctx, kcpClient, shared.StateReady), standardTimeout, standardInterval). WithArguments(manifestName).Should(Succeed()) From b90f7e368db027b372c277034220e4f197f0c79e Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Fri, 14 Jun 2024 14:01:23 +0200 Subject: [PATCH 05/25] Fix integration test --- internal/manifest/ready_check.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/internal/manifest/ready_check.go b/internal/manifest/ready_check.go index a3112414f8..c219260eff 100644 --- a/internal/manifest/ready_check.go +++ b/internal/manifest/ready_check.go @@ -3,16 +3,18 @@ package manifest import ( "context" "errors" + "fmt" "time" - "github.com/kyma-project/lifecycle-manager/api/shared" - declarativev2 "github.com/kyma-project/lifecycle-manager/internal/declarative/v2" apiappsv1 "k8s.io/api/apps/v1" apicorev1 "k8s.io/api/core/v1" k8slabels "k8s.io/apimachinery/pkg/labels" "k8s.io/cli-runtime/pkg/resource" "k8s.io/kubectl/pkg/util/deployment" "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kyma-project/lifecycle-manager/api/shared" + declarativev2 "github.com/kyma-project/lifecycle-manager/internal/declarative/v2" ) const ( @@ -30,9 +32,7 @@ func NewDeploymentReadyCheck() *DeploymentReadyCheck { type DeploymentReadyCheck struct{} -var ( - ErrNotSupportedState = errors.New("module CR state not support") -) +var ErrNotSupportedState = errors.New("module CR state not support") func (c *DeploymentReadyCheck) Run(ctx context.Context, clnt declarativev2.Client, @@ -91,7 +91,7 @@ func getPodsForDeployment(clt declarativev2.Client, deploy *apiappsv1.Deployment LabelSelector: k8slabels.SelectorFromSet(deploy.Spec.Selector.MatchLabels), } if err := clt.List(context.TODO(), podList, listOptions); err != nil { - return nil, err + return nil, fmt.Errorf("failed to list pods: %w", err) } return podList, nil } @@ -99,11 +99,12 @@ func getPodsForDeployment(clt declarativev2.Client, deploy *apiappsv1.Deployment func getPodsState(podList *apicorev1.PodList) shared.State { for _, pod := range podList.Items { for _, condition := range pod.Status.ContainerStatuses { - if *condition.Started && condition.Ready { + switch { + case *condition.Started && condition.Ready: return shared.StateReady - } else if *condition.Started && !condition.Ready { + case *condition.Started && !condition.Ready: return shared.StateProcessing - } else if !*condition.Started && !condition.Ready { + case !*condition.Started && !condition.Ready: return shared.StateError } } From d8a49c42961b975d532beda89ee9566b35e2dcb0 Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Thu, 20 Jun 2024 11:32:38 +0200 Subject: [PATCH 06/25] Refactor Status propagation --- .../actions/deploy-template-operator/action.yaml | 2 +- .github/workflows/test-e2e.yaml | 2 +- internal/manifest/ready_check.go | 11 ----------- tests/e2e/Makefile | 6 +++--- tests/e2e/warning_status_propagation_test.go | 14 +++++++------- 5 files changed, 12 insertions(+), 23 deletions(-) diff --git a/.github/actions/deploy-template-operator/action.yaml b/.github/actions/deploy-template-operator/action.yaml index 4e492ba822..741e153563 100644 --- a/.github/actions/deploy-template-operator/action.yaml +++ b/.github/actions/deploy-template-operator/action.yaml @@ -48,7 +48,7 @@ runs: kubectl apply -f tests/moduletemplates/moduletemplate_template_operator_v2_fast.yaml - name: Create Template Operator Module with final state and final deletion state as `Warning` and apply working-directory: template-operator - if: ${{ matrix.e2e-test == 'module-status-propagation'}} + if: ${{ matrix.e2e-test == 'module-status-decoupling'}} shell: bash run: | pushd config/default diff --git a/.github/workflows/test-e2e.yaml b/.github/workflows/test-e2e.yaml index 85d38c5a02..bba540ca74 100644 --- a/.github/workflows/test-e2e.yaml +++ b/.github/workflows/test-e2e.yaml @@ -29,7 +29,7 @@ jobs: - watcher-enqueue - kyma-deprovision-with-foreground-propagation - kyma-deprovision-with-background-propagation - - module-status-propagation + - module-status-decoupling - kyma-metrics - module-without-default-cr - module-consistency diff --git a/internal/manifest/ready_check.go b/internal/manifest/ready_check.go index c219260eff..31845f0eb9 100644 --- a/internal/manifest/ready_check.go +++ b/internal/manifest/ready_check.go @@ -2,9 +2,7 @@ package manifest import ( "context" - "errors" "fmt" - "time" apiappsv1 "k8s.io/api/apps/v1" apicorev1 "k8s.io/api/core/v1" @@ -17,13 +15,6 @@ import ( declarativev2 "github.com/kyma-project/lifecycle-manager/internal/declarative/v2" ) -const ( - ToWarningDuration = 5 * time.Minute - customResourceStatePath = "status.state" - ModuleCRWithCustomCheckWarning = "module CR state not found or given customStateCheck.jsonPath is not exists" - ModuleCRWithNoCustomCheckWarning = "module CR state not found" -) - // NewDeploymentReadyCheck creates a readiness check that verifies that the Resource in the Manifest // returns the ready state, if not it returns not ready. func NewDeploymentReadyCheck() *DeploymentReadyCheck { @@ -32,8 +23,6 @@ func NewDeploymentReadyCheck() *DeploymentReadyCheck { type DeploymentReadyCheck struct{} -var ErrNotSupportedState = errors.New("module CR state not support") - func (c *DeploymentReadyCheck) Run(ctx context.Context, clnt declarativev2.Client, resources []*resource.Info, diff --git a/tests/e2e/Makefile b/tests/e2e/Makefile index 02c11cd66d..ca4e64681c 100644 --- a/tests/e2e/Makefile +++ b/tests/e2e/Makefile @@ -72,7 +72,7 @@ test: kyma-deprovision-with-foreground-propagation \ kyma-metrics \ mandatory-module-metrics \ watcher-enqueue \ - module-status-propagation \ + module-status-decoupling \ module-without-default-cr \ module-consistency \ mandatory-module \ @@ -102,8 +102,8 @@ mandatory-module-metrics: watcher-enqueue: go test -timeout 20m -ginkgo.v -ginkgo.focus "Enqueue Event from Watcher" -module-status-propagation: - go test -timeout 20m -ginkgo.v -ginkgo.focus "Warning Status Propagation" +module-status-decoupling: + go test -timeout 20m -ginkgo.v -ginkgo.focus "Module Status Decoupling" module-without-default-cr: go test -timeout 20m -ginkgo.v -ginkgo.focus "Module Without Default CR" diff --git a/tests/e2e/warning_status_propagation_test.go b/tests/e2e/warning_status_propagation_test.go index c38f3887bf..c993fb65e6 100644 --- a/tests/e2e/warning_status_propagation_test.go +++ b/tests/e2e/warning_status_propagation_test.go @@ -14,7 +14,7 @@ import ( . "github.com/kyma-project/lifecycle-manager/pkg/testutils" ) -var _ = Describe("Warning Status Propagation", Ordered, func() { +var _ = Describe("Module Status Decoupling", Ordered, func() { kyma := NewKymaWithSyncLabel("kyma-sample", ControlPlaneNamespace, v1beta2.DefaultChannel) module := NewTemplateOperator(v1beta2.DefaultChannel) moduleCR := NewTestModuleCR(RemoteNamespace) @@ -54,10 +54,10 @@ var _ = Describe("Warning Status Propagation", Ordered, func() { module.Name, shared.StateWarning). Should(Succeed()) - By("And KCP kyma CR is in \"Warning\" State") + By("And KCP kyma CR is in \"Ready\" State") Eventually(KymaIsInState). WithContext(ctx). - WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateWarning). + WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateReady). Should(Succeed()) }) @@ -74,17 +74,17 @@ var _ = Describe("Warning Status Propagation", Ordered, func() { WithArguments(runtimeClient, defaultRemoteKymaName, RemoteNamespace, module.Name). Should(Succeed()) - By("And KCP Kyma CR is in \"Warning\" State") + By("And KCP Kyma CR is in \"Ready\" State") Eventually(KymaIsInState). WithContext(ctx). - WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateWarning). + WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateReady). Should(Succeed()) Consistently(KymaIsInState). WithContext(ctx). - WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateWarning). + WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateReady). Should(Succeed()) - By("And Module Manifest CR is in a \"Warning\" State") + By("And Module Manifest CR is in a \"Ready\" State") Eventually(CheckManifestIsInState). WithContext(ctx). WithArguments(kyma.GetName(), kyma.GetNamespace(), module.Name, controlPlaneClient, From af9df2c2f6b45f0dd4f9ed7a9a7f12b5a0785099 Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Fri, 21 Jun 2024 09:16:31 +0200 Subject: [PATCH 07/25] Fix Status decoupling Test --- ...tion_test.go => module_status_decoupling_test.go} | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) rename tests/e2e/{warning_status_propagation_test.go => module_status_decoupling_test.go} (91%) diff --git a/tests/e2e/warning_status_propagation_test.go b/tests/e2e/module_status_decoupling_test.go similarity index 91% rename from tests/e2e/warning_status_propagation_test.go rename to tests/e2e/module_status_decoupling_test.go index c993fb65e6..3675afa809 100644 --- a/tests/e2e/warning_status_propagation_test.go +++ b/tests/e2e/module_status_decoupling_test.go @@ -47,11 +47,11 @@ var _ = Describe("Module Status Decoupling", Ordered, func() { Expect(resource.GroupVersionKind().Kind).To(Equal(moduleCR.GroupVersionKind().Kind)) }).WithContext(ctx).Should(Succeed()) - By("And Module in KCP Kyma CR is in \"Warning\" State") + By("And Module in KCP Kyma CR is in \"Ready\" State") Eventually(CheckModuleState). WithContext(ctx). WithArguments(controlPlaneClient, kyma.GetName(), kyma.GetNamespace(), - module.Name, shared.StateWarning). + module.Name, shared.StateReady). Should(Succeed()) By("And KCP kyma CR is in \"Ready\" State") @@ -68,13 +68,7 @@ var _ = Describe("Module Status Decoupling", Ordered, func() { Should(Succeed()) }) - It("Then there is no Module in KCP Kyma CR spec", func() { - Eventually(NotContainsModuleInSpec, Timeout, Interval). - WithContext(ctx). - WithArguments(runtimeClient, defaultRemoteKymaName, RemoteNamespace, module.Name). - Should(Succeed()) - - By("And KCP Kyma CR is in \"Ready\" State") + It("Then KCP Kyma CR is in \"Ready\" State", func() { Eventually(KymaIsInState). WithContext(ctx). WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateReady). From 4cc4981fec7f255dc8387df4b56ef96d2deefd43 Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Mon, 24 Jun 2024 13:49:00 +0200 Subject: [PATCH 08/25] Fix Status decoupling Test --- tests/e2e/module_status_decoupling_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/e2e/module_status_decoupling_test.go b/tests/e2e/module_status_decoupling_test.go index 3675afa809..5333071036 100644 --- a/tests/e2e/module_status_decoupling_test.go +++ b/tests/e2e/module_status_decoupling_test.go @@ -82,13 +82,13 @@ var _ = Describe("Module Status Decoupling", Ordered, func() { Eventually(CheckManifestIsInState). WithContext(ctx). WithArguments(kyma.GetName(), kyma.GetNamespace(), module.Name, controlPlaneClient, - shared.StateWarning). + shared.StateReady). Should(Succeed()) Consistently(CheckManifestIsInState). WithContext(ctx). WithArguments(kyma.GetName(), kyma.GetNamespace(), module.Name, controlPlaneClient, - shared.StateWarning). + shared.StateReady). Should(Succeed()) }) From c6b62431de7345913b99f0f4ce8bfe1f1c37c127 Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Mon, 24 Jun 2024 17:11:31 +0200 Subject: [PATCH 09/25] Set deleting state if manifest has a deletion timestamp --- internal/declarative/v2/reconciler.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/declarative/v2/reconciler.go b/internal/declarative/v2/reconciler.go index 1f6f9e1388..dcc3e94d8f 100644 --- a/internal/declarative/v2/reconciler.go +++ b/internal/declarative/v2/reconciler.go @@ -406,6 +406,10 @@ func (r *Reconciler) checkTargetReadiness( return ErrInstallationConditionRequiresUpdate } + if !manifest.GetDeletionTimestamp().IsZero() { + crStateInfo.State = shared.StateDeleting + } + installationCondition := newInstallationCondition(manifest) if !meta.IsStatusConditionTrue(status.Conditions, installationCondition.Type) || status.State != crStateInfo.State { installationCondition.Status = apimetav1.ConditionTrue From c607683eba1b16acc6ae9dc25ab61922b32ab981 Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Mon, 24 Jun 2024 17:24:14 +0200 Subject: [PATCH 10/25] Adapt state decoupling test --- tests/e2e/module_status_decoupling_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/e2e/module_status_decoupling_test.go b/tests/e2e/module_status_decoupling_test.go index 5333071036..5827e57f5e 100644 --- a/tests/e2e/module_status_decoupling_test.go +++ b/tests/e2e/module_status_decoupling_test.go @@ -68,27 +68,27 @@ var _ = Describe("Module Status Decoupling", Ordered, func() { Should(Succeed()) }) - It("Then KCP Kyma CR is in \"Ready\" State", func() { + It("Then KCP Kyma CR is in a \"Processing\" State", func() { Eventually(KymaIsInState). WithContext(ctx). - WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateReady). + WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateProcessing). Should(Succeed()) Consistently(KymaIsInState). WithContext(ctx). - WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateReady). + WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateProcessing). Should(Succeed()) - By("And Module Manifest CR is in a \"Ready\" State") + By("And Module Manifest CR is in a \"Deleting\" State") Eventually(CheckManifestIsInState). WithContext(ctx). WithArguments(kyma.GetName(), kyma.GetNamespace(), module.Name, controlPlaneClient, - shared.StateReady). + shared.StateDeleting). Should(Succeed()) Consistently(CheckManifestIsInState). WithContext(ctx). WithArguments(kyma.GetName(), kyma.GetNamespace(), module.Name, controlPlaneClient, - shared.StateReady). + shared.StateDeleting). Should(Succeed()) }) From fcee820fda4c9d360e15441a693beb5752268490 Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Wed, 26 Jun 2024 16:55:55 +0200 Subject: [PATCH 11/25] Add new Testcase --- .../deploy-template-operator/action.yaml | 17 +++ tests/e2e/module_status_decoupling_test.go | 103 ++++++++++++++++++ 2 files changed, 120 insertions(+) diff --git a/.github/actions/deploy-template-operator/action.yaml b/.github/actions/deploy-template-operator/action.yaml index 741e153563..79631f1051 100644 --- a/.github/actions/deploy-template-operator/action.yaml +++ b/.github/actions/deploy-template-operator/action.yaml @@ -66,6 +66,23 @@ runs: sed -i 's/localhost:5111/k3d-kcp-registry.localhost:5000/g' ./template.yaml kubectl get crds kubectl apply -f template.yaml + - name: Create Template Operator Module with non-working image and apply + working-directory: template-operator + if: ${{ matrix.e2e-test == 'module-status-decoupling'}} + shell: bash + run: | + pushd config/default + echo \ + "- op: replace + path: /spec/template/spec/containers/0/image + value: non-working-path" >> image_patch.yaml + cat image_patch.yaml + kustomize edit add patch --path image_patch.yaml --kind Deployment + popd + kyma alpha create module --kubebuilder-project --channel=regular --name kyma.project.io/module/template-operator-misconfigured --version 1.1.1 --path . --registry localhost:5111 --insecure --module-archive-version-overwrite + sed -i 's/localhost:5111/k3d-kcp-registry.localhost:5000/g' ./template.yaml + kubectl get crds + kubectl apply -f template.yaml - name: Create Template Operator Module without default CR and apply working-directory: template-operator if: ${{ matrix.e2e-test == 'module-without-default-cr' }} diff --git a/tests/e2e/module_status_decoupling_test.go b/tests/e2e/module_status_decoupling_test.go index 5827e57f5e..fdfb4ae68c 100644 --- a/tests/e2e/module_status_decoupling_test.go +++ b/tests/e2e/module_status_decoupling_test.go @@ -84,7 +84,110 @@ var _ = Describe("Module Status Decoupling", Ordered, func() { WithArguments(kyma.GetName(), kyma.GetNamespace(), module.Name, controlPlaneClient, shared.StateDeleting). Should(Succeed()) + Consistently(CheckManifestIsInState). + WithContext(ctx). + WithArguments(kyma.GetName(), kyma.GetNamespace(), module.Name, controlPlaneClient, + shared.StateDeleting). + Should(Succeed()) + }) + + It("When blocking finalizers from Module CR get removed", func() { + var finalizers []string + Eventually(SetFinalizer). + WithContext(ctx). + WithArguments(TestModuleCRName, RemoteNamespace, "operator.kyma-project.io", "v1alpha1", + string(templatev1alpha1.SampleKind), finalizers, runtimeClient). + Should(Succeed()) + }) + + It("Then Module CR, Module Operator Deployment and Manifest CR are removed", func() { + Eventually(CheckIfExists). + WithContext(ctx). + WithArguments(TestModuleCRName, RemoteNamespace, + "operator.kyma-project.io", "v1alpha1", string(templatev1alpha1.SampleKind), runtimeClient). + Should(Equal(ErrNotFound)) + + Eventually(DeploymentIsReady). + WithContext(ctx). + WithArguments(runtimeClient, ModuleDeploymentName, TestModuleResourceNamespace). + Should(Equal(ErrNotFound)) + + Eventually(NoManifestExist). + WithContext(ctx). + WithArguments(controlPlaneClient). + Should(Succeed()) + + By("And KCP Kyma CR is in \"Ready\" State") + Eventually(KymaIsInState). + WithContext(ctx). + WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateReady). + Should(Succeed()) + }) + }) + + Context("Given SKR Cluster", func() { + It("When Kyma Module with wrong configured image is enabled", func() { + Eventually(EnableModule). + WithContext(ctx). + WithArguments(runtimeClient, defaultRemoteKymaName, RemoteNamespace, module). + Should(Succeed()) + }) + + It("Then Module CR exists", func() { + Eventually(ModuleCRExists). + WithContext(ctx). + WithArguments(runtimeClient, moduleCR). + Should(Succeed()) + By("And resource is defined in Manifest CR") + Eventually(func(g Gomega, ctx context.Context) { + resource, err := GetManifestResource(ctx, controlPlaneClient, + kyma.GetName(), kyma.GetNamespace(), module.Name) + Expect(err).ToNot(HaveOccurred()) + Expect(resource.GetName()).To(Equal(moduleCR.GetName())) + Expect(resource.GetNamespace()).To(Equal(moduleCR.GetNamespace())) + Expect(resource.GroupVersionKind().Version).To(Equal(moduleCR.GroupVersionKind().Version)) + Expect(resource.GroupVersionKind().Group).To(Equal(moduleCR.GroupVersionKind().Group)) + Expect(resource.GroupVersionKind().Kind).To(Equal(moduleCR.GroupVersionKind().Kind)) + }).WithContext(ctx).Should(Succeed()) + + By("And Module in KCP Kyma CR is in \"Error\" State") + Eventually(CheckModuleState). + WithContext(ctx). + WithArguments(controlPlaneClient, kyma.GetName(), kyma.GetNamespace(), + module.Name, shared.StateError). + Should(Succeed()) + + By("And KCP kyma CR is in \"Error\" State") + Eventually(KymaIsInState). + WithContext(ctx). + WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateError). + Should(Succeed()) + }) + + It("When Kyma Module is disabled", func() { + Eventually(DisableModule). + WithContext(ctx). + WithArguments(runtimeClient, defaultRemoteKymaName, RemoteNamespace, module.Name). + Should(Succeed()) + }) + + It("Then KCP Kyma CR is in a \"Processing\" State", func() { + Eventually(KymaIsInState). + WithContext(ctx). + WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateProcessing). + Should(Succeed()) + Consistently(KymaIsInState). + WithContext(ctx). + WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateProcessing). + Should(Succeed()) + + By("And Module Manifest CR is in a \"Deleting\" State") + Eventually(CheckManifestIsInState). + WithContext(ctx). + WithArguments(kyma.GetName(), kyma.GetNamespace(), module.Name, controlPlaneClient, + shared.StateDeleting). + Should(Succeed()) Consistently(CheckManifestIsInState). WithContext(ctx). WithArguments(kyma.GetName(), kyma.GetNamespace(), module.Name, controlPlaneClient, From 4a7a9bb2c7152993864ffa64024324d4a390a2cd Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Wed, 26 Jun 2024 17:15:14 +0200 Subject: [PATCH 12/25] Use wrong config module for test --- tests/e2e/module_status_decoupling_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/e2e/module_status_decoupling_test.go b/tests/e2e/module_status_decoupling_test.go index fdfb4ae68c..379550c0ec 100644 --- a/tests/e2e/module_status_decoupling_test.go +++ b/tests/e2e/module_status_decoupling_test.go @@ -17,6 +17,7 @@ import ( var _ = Describe("Module Status Decoupling", Ordered, func() { kyma := NewKymaWithSyncLabel("kyma-sample", ControlPlaneNamespace, v1beta2.DefaultChannel) module := NewTemplateOperator(v1beta2.DefaultChannel) + moduleWrongConfig := NewTestModuleWithFixName("template-operator-misconfigured", "regular") moduleCR := NewTestModuleCR(RemoteNamespace) InitEmptyKymaBeforeAll(kyma) CleanupKymaAfterAll(kyma) @@ -129,7 +130,7 @@ var _ = Describe("Module Status Decoupling", Ordered, func() { It("When Kyma Module with wrong configured image is enabled", func() { Eventually(EnableModule). WithContext(ctx). - WithArguments(runtimeClient, defaultRemoteKymaName, RemoteNamespace, module). + WithArguments(runtimeClient, defaultRemoteKymaName, RemoteNamespace, moduleWrongConfig). Should(Succeed()) }) From 675261f6107f3d2671596c92ce191d2a16add357 Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Wed, 26 Jun 2024 17:16:59 +0200 Subject: [PATCH 13/25] Use wrong config module for test --- tests/e2e/module_status_decoupling_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/e2e/module_status_decoupling_test.go b/tests/e2e/module_status_decoupling_test.go index 379550c0ec..cb5856b10d 100644 --- a/tests/e2e/module_status_decoupling_test.go +++ b/tests/e2e/module_status_decoupling_test.go @@ -143,7 +143,7 @@ var _ = Describe("Module Status Decoupling", Ordered, func() { By("And resource is defined in Manifest CR") Eventually(func(g Gomega, ctx context.Context) { resource, err := GetManifestResource(ctx, controlPlaneClient, - kyma.GetName(), kyma.GetNamespace(), module.Name) + kyma.GetName(), kyma.GetNamespace(), moduleWrongConfig.Name) Expect(err).ToNot(HaveOccurred()) Expect(resource.GetName()).To(Equal(moduleCR.GetName())) Expect(resource.GetNamespace()).To(Equal(moduleCR.GetNamespace())) @@ -156,7 +156,7 @@ var _ = Describe("Module Status Decoupling", Ordered, func() { Eventually(CheckModuleState). WithContext(ctx). WithArguments(controlPlaneClient, kyma.GetName(), kyma.GetNamespace(), - module.Name, shared.StateError). + moduleWrongConfig.Name, shared.StateError). Should(Succeed()) By("And KCP kyma CR is in \"Error\" State") @@ -169,7 +169,7 @@ var _ = Describe("Module Status Decoupling", Ordered, func() { It("When Kyma Module is disabled", func() { Eventually(DisableModule). WithContext(ctx). - WithArguments(runtimeClient, defaultRemoteKymaName, RemoteNamespace, module.Name). + WithArguments(runtimeClient, defaultRemoteKymaName, RemoteNamespace, moduleWrongConfig.Name). Should(Succeed()) }) @@ -186,12 +186,12 @@ var _ = Describe("Module Status Decoupling", Ordered, func() { By("And Module Manifest CR is in a \"Deleting\" State") Eventually(CheckManifestIsInState). WithContext(ctx). - WithArguments(kyma.GetName(), kyma.GetNamespace(), module.Name, controlPlaneClient, + WithArguments(kyma.GetName(), kyma.GetNamespace(), moduleWrongConfig.Name, controlPlaneClient, shared.StateDeleting). Should(Succeed()) Consistently(CheckManifestIsInState). WithContext(ctx). - WithArguments(kyma.GetName(), kyma.GetNamespace(), module.Name, controlPlaneClient, + WithArguments(kyma.GetName(), kyma.GetNamespace(), moduleWrongConfig.Name, controlPlaneClient, shared.StateDeleting). Should(Succeed()) }) From 3d0ce715ace070fe5cbc8fbcb37da92626bf2b39 Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Wed, 26 Jun 2024 17:37:46 +0200 Subject: [PATCH 14/25] Fix wrong config module for test --- tests/e2e/module_status_decoupling_test.go | 33 +--------------------- 1 file changed, 1 insertion(+), 32 deletions(-) diff --git a/tests/e2e/module_status_decoupling_test.go b/tests/e2e/module_status_decoupling_test.go index cb5856b10d..73c3cce068 100644 --- a/tests/e2e/module_status_decoupling_test.go +++ b/tests/e2e/module_status_decoupling_test.go @@ -173,38 +173,6 @@ var _ = Describe("Module Status Decoupling", Ordered, func() { Should(Succeed()) }) - It("Then KCP Kyma CR is in a \"Processing\" State", func() { - Eventually(KymaIsInState). - WithContext(ctx). - WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateProcessing). - Should(Succeed()) - Consistently(KymaIsInState). - WithContext(ctx). - WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateProcessing). - Should(Succeed()) - - By("And Module Manifest CR is in a \"Deleting\" State") - Eventually(CheckManifestIsInState). - WithContext(ctx). - WithArguments(kyma.GetName(), kyma.GetNamespace(), moduleWrongConfig.Name, controlPlaneClient, - shared.StateDeleting). - Should(Succeed()) - Consistently(CheckManifestIsInState). - WithContext(ctx). - WithArguments(kyma.GetName(), kyma.GetNamespace(), moduleWrongConfig.Name, controlPlaneClient, - shared.StateDeleting). - Should(Succeed()) - }) - - It("When blocking finalizers from Module CR get removed", func() { - var finalizers []string - Eventually(SetFinalizer). - WithContext(ctx). - WithArguments(TestModuleCRName, RemoteNamespace, "operator.kyma-project.io", "v1alpha1", - string(templatev1alpha1.SampleKind), finalizers, runtimeClient). - Should(Succeed()) - }) - It("Then Module CR, Module Operator Deployment and Manifest CR are removed", func() { Eventually(CheckIfExists). WithContext(ctx). @@ -229,4 +197,5 @@ var _ = Describe("Module Status Decoupling", Ordered, func() { Should(Succeed()) }) }) + }) From 05f5fe35daf8c9411283e187f14a1762c088b24f Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Thu, 27 Jun 2024 10:20:06 +0200 Subject: [PATCH 15/25] Fix go lint --- tests/e2e/module_status_decoupling_test.go | 102 ++++++++------------- 1 file changed, 39 insertions(+), 63 deletions(-) diff --git a/tests/e2e/module_status_decoupling_test.go b/tests/e2e/module_status_decoupling_test.go index 73c3cce068..b6890935fb 100644 --- a/tests/e2e/module_status_decoupling_test.go +++ b/tests/e2e/module_status_decoupling_test.go @@ -4,6 +4,7 @@ import ( "context" templatev1alpha1 "github.com/kyma-project/template-operator/api/v1alpha1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" @@ -30,37 +31,7 @@ var _ = Describe("Module Status Decoupling", Ordered, func() { Should(Succeed()) }) - It("Then Module CR exists", func() { - Eventually(ModuleCRExists). - WithContext(ctx). - WithArguments(runtimeClient, moduleCR). - Should(Succeed()) - - By("And resource is defined in Manifest CR") - Eventually(func(g Gomega, ctx context.Context) { - resource, err := GetManifestResource(ctx, controlPlaneClient, - kyma.GetName(), kyma.GetNamespace(), module.Name) - Expect(err).ToNot(HaveOccurred()) - Expect(resource.GetName()).To(Equal(moduleCR.GetName())) - Expect(resource.GetNamespace()).To(Equal(moduleCR.GetNamespace())) - Expect(resource.GroupVersionKind().Version).To(Equal(moduleCR.GroupVersionKind().Version)) - Expect(resource.GroupVersionKind().Group).To(Equal(moduleCR.GroupVersionKind().Group)) - Expect(resource.GroupVersionKind().Kind).To(Equal(moduleCR.GroupVersionKind().Kind)) - }).WithContext(ctx).Should(Succeed()) - - By("And Module in KCP Kyma CR is in \"Ready\" State") - Eventually(CheckModuleState). - WithContext(ctx). - WithArguments(controlPlaneClient, kyma.GetName(), kyma.GetNamespace(), - module.Name, shared.StateReady). - Should(Succeed()) - - By("And KCP kyma CR is in \"Ready\" State") - Eventually(KymaIsInState). - WithContext(ctx). - WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateReady). - Should(Succeed()) - }) + checkModuleStatus(module, moduleCR, kyma, shared.StateReady) It("When Kyma Module is disabled", func() { Eventually(DisableModule). @@ -134,37 +105,7 @@ var _ = Describe("Module Status Decoupling", Ordered, func() { Should(Succeed()) }) - It("Then Module CR exists", func() { - Eventually(ModuleCRExists). - WithContext(ctx). - WithArguments(runtimeClient, moduleCR). - Should(Succeed()) - - By("And resource is defined in Manifest CR") - Eventually(func(g Gomega, ctx context.Context) { - resource, err := GetManifestResource(ctx, controlPlaneClient, - kyma.GetName(), kyma.GetNamespace(), moduleWrongConfig.Name) - Expect(err).ToNot(HaveOccurred()) - Expect(resource.GetName()).To(Equal(moduleCR.GetName())) - Expect(resource.GetNamespace()).To(Equal(moduleCR.GetNamespace())) - Expect(resource.GroupVersionKind().Version).To(Equal(moduleCR.GroupVersionKind().Version)) - Expect(resource.GroupVersionKind().Group).To(Equal(moduleCR.GroupVersionKind().Group)) - Expect(resource.GroupVersionKind().Kind).To(Equal(moduleCR.GroupVersionKind().Kind)) - }).WithContext(ctx).Should(Succeed()) - - By("And Module in KCP Kyma CR is in \"Error\" State") - Eventually(CheckModuleState). - WithContext(ctx). - WithArguments(controlPlaneClient, kyma.GetName(), kyma.GetNamespace(), - moduleWrongConfig.Name, shared.StateError). - Should(Succeed()) - - By("And KCP kyma CR is in \"Error\" State") - Eventually(KymaIsInState). - WithContext(ctx). - WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, shared.StateError). - Should(Succeed()) - }) + checkModuleStatus(moduleWrongConfig, moduleCR, kyma, shared.StateError) It("When Kyma Module is disabled", func() { Eventually(DisableModule). @@ -197,5 +138,40 @@ var _ = Describe("Module Status Decoupling", Ordered, func() { Should(Succeed()) }) }) - }) + +func checkModuleStatus(module v1beta2.Module, moduleCR *unstructured.Unstructured, kyma *v1beta2.Kyma, + expectedState shared.State, +) { + It("Then Module CR exists", func() { + Eventually(ModuleCRExists). + WithContext(ctx). + WithArguments(runtimeClient, moduleCR). + Should(Succeed()) + + By("And resource is defined in Manifest CR") + Eventually(func(g Gomega, ctx context.Context) { + resource, err := GetManifestResource(ctx, controlPlaneClient, + kyma.GetName(), kyma.GetNamespace(), module.Name) + Expect(err).ToNot(HaveOccurred()) + Expect(resource.GetName()).To(Equal(moduleCR.GetName())) + Expect(resource.GetNamespace()).To(Equal(moduleCR.GetNamespace())) + Expect(resource.GroupVersionKind().Version).To(Equal(moduleCR.GroupVersionKind().Version)) + Expect(resource.GroupVersionKind().Group).To(Equal(moduleCR.GroupVersionKind().Group)) + Expect(resource.GroupVersionKind().Kind).To(Equal(moduleCR.GroupVersionKind().Kind)) + }).WithContext(ctx).Should(Succeed()) + + By("And Module in KCP Kyma CR is in " + string(expectedState) + " State") + Eventually(CheckModuleState). + WithContext(ctx). + WithArguments(controlPlaneClient, kyma.GetName(), kyma.GetNamespace(), + module.Name, expectedState). + Should(Succeed()) + + By("And KCP kyma CR is in " + string(expectedState) + " State") + Eventually(KymaIsInState). + WithContext(ctx). + WithArguments(kyma.GetName(), kyma.GetNamespace(), controlPlaneClient, expectedState). + Should(Succeed()) + }) +} From 172219cce9bf6d6b4bb9fdbe6672066dcabc4e5d Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Thu, 27 Jun 2024 14:45:00 +0200 Subject: [PATCH 16/25] Add docs --- docs/technical-reference/api/manifest-cr.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/technical-reference/api/manifest-cr.md b/docs/technical-reference/api/manifest-cr.md index 691efd2813..5ce226fe60 100644 --- a/docs/technical-reference/api/manifest-cr.md +++ b/docs/technical-reference/api/manifest-cr.md @@ -87,7 +87,16 @@ The resource is the default data that should be initialized for the module and i ### **.status** -The Manifest CR status is an unmodified version of the [declarative status](../../../internal/declarative/README.md#resource-tracking), so the tracking process of the library applies. There is no custom API for this. +The status of the Manifest Custom Resource (CR) is a direct, unaltered reflection of the [declarative status](../../../internal/declarative/README.md#resource-tracking), which means it follows the tracking process implemented by the internal library. There isn't a custom API specifically designed for this status tracking. Instead, the existing mechanisms provided by the library are utilized to monitor and update the status of the Manifest CR. + +The Manifest CR status is set based on the following logic, managed by the manifest reconciler: + +- If the Module defined in the Manifest CR is successfully applied and the deployed Module is up and running, the status of the Manifest CR is set to `Ready`. +- While the Manifest is being applied and the Deployment is still starting, the status of the Manifest CR is set to `Processing`. +- If the Deployment is unable to start (for example, due to an `ImagePullBackOff` error) or if the application of the Manifest fails, the status of the Manifest CR is set to `Error`. +- If the Manifest CR is marked for deletion, the status of the Manifest CR is set to `Deleted`. + +This status provides a reliable way to track the state of the Manifest CR and the associated Module, offering insights into the deployment process and any potential issues, while being decoupled of the Module business logic. ### **.metadata.labels** From ee7ba34d4c78089ce74b0fb42bf014875a405507 Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Fri, 28 Jun 2024 12:24:37 +0200 Subject: [PATCH 17/25] Adress review comments --- docs/technical-reference/api/manifest-cr.md | 4 +- internal/declarative/v2/reconciler.go | 47 ++++++++++++++------- internal/manifest/ready_check.go | 14 +++--- 3 files changed, 40 insertions(+), 25 deletions(-) diff --git a/docs/technical-reference/api/manifest-cr.md b/docs/technical-reference/api/manifest-cr.md index 5ce226fe60..7dcaa560c9 100644 --- a/docs/technical-reference/api/manifest-cr.md +++ b/docs/technical-reference/api/manifest-cr.md @@ -87,14 +87,12 @@ The resource is the default data that should be initialized for the module and i ### **.status** -The status of the Manifest Custom Resource (CR) is a direct, unaltered reflection of the [declarative status](../../../internal/declarative/README.md#resource-tracking), which means it follows the tracking process implemented by the internal library. There isn't a custom API specifically designed for this status tracking. Instead, the existing mechanisms provided by the library are utilized to monitor and update the status of the Manifest CR. - The Manifest CR status is set based on the following logic, managed by the manifest reconciler: - If the Module defined in the Manifest CR is successfully applied and the deployed Module is up and running, the status of the Manifest CR is set to `Ready`. - While the Manifest is being applied and the Deployment is still starting, the status of the Manifest CR is set to `Processing`. - If the Deployment is unable to start (for example, due to an `ImagePullBackOff` error) or if the application of the Manifest fails, the status of the Manifest CR is set to `Error`. -- If the Manifest CR is marked for deletion, the status of the Manifest CR is set to `Deleted`. +- If the Manifest CR is marked for deletion, the status of the Manifest CR is set to `Deleting`. This status provides a reliable way to track the state of the Manifest CR and the associated Module, offering insights into the deployment process and any potential issues, while being decoupled of the Module business logic. diff --git a/internal/declarative/v2/reconciler.go b/internal/declarative/v2/reconciler.go index dcc3e94d8f..0cf6cb7630 100644 --- a/internal/declarative/v2/reconciler.go +++ b/internal/declarative/v2/reconciler.go @@ -363,7 +363,12 @@ func (r *Reconciler) syncResources(ctx context.Context, clnt Client, obj Object, } } - return r.checkTargetReadiness(ctx, clnt, obj, target) + deploymentState, err := r.checkDeploymentState(ctx, clnt, obj, target) + if err != nil { + obj.SetStatus(status.WithState(shared.StateError).WithErr(err)) + return err + } + return r.setManifestState(obj, deploymentState) } func hasDiff(oldResources []shared.Resource, newResources []shared.Resource) bool { @@ -387,35 +392,47 @@ func hasDiff(oldResources []shared.Resource, newResources []shared.Resource) boo return false } -func (r *Reconciler) checkTargetReadiness( - ctx context.Context, clnt Client, manifest Object, target []*resource.Info, -) error { - status := manifest.GetStatus() - +func (r *Reconciler) checkDeploymentState( + ctx context.Context, clnt Client, obj Object, target []*resource.Info, +) (shared.State, error) { resourceReadyCheck := r.CustomReadyCheck - crStateInfo, err := resourceReadyCheck.Run(ctx, clnt, target) + deploymentStateInfo, err := resourceReadyCheck.Run(ctx, clnt, target) if err != nil { - manifest.SetStatus(status.WithState(shared.StateError).WithErr(err)) - return err + return "", err } - if crStateInfo.State == shared.StateProcessing { - waitingMsg := "waiting for resources to become ready: " + crStateInfo.Info + if deploymentStateInfo.State == shared.StateProcessing { + return shared.StateProcessing, nil + } + + if !obj.GetDeletionTimestamp().IsZero() { + return shared.StateDeleting, nil + } + + return deploymentStateInfo.State, nil +} + +func (r *Reconciler) setManifestState(manifest Object, state shared.State) error { + status := manifest.GetStatus() + + if state == shared.StateProcessing { + waitingMsg := "waiting for resources to become ready" manifest.SetStatus(status.WithState(shared.StateProcessing).WithOperation(waitingMsg)) return ErrInstallationConditionRequiresUpdate } if !manifest.GetDeletionTimestamp().IsZero() { - crStateInfo.State = shared.StateDeleting + state = shared.StateDeleting } installationCondition := newInstallationCondition(manifest) - if !meta.IsStatusConditionTrue(status.Conditions, installationCondition.Type) || status.State != crStateInfo.State { + if !meta.IsStatusConditionTrue(status.Conditions, + installationCondition.Type) || status.State != state { installationCondition.Status = apimetav1.ConditionTrue meta.SetStatusCondition(&status.Conditions, installationCondition) - manifest.SetStatus(status.WithState(crStateInfo.State). - WithOperation(generateOperationMessage(installationCondition, crStateInfo))) + manifest.SetStatus(status.WithState(state). + WithOperation(generateOperationMessage(installationCondition, StateInfo{State: state}))) return ErrInstallationConditionRequiresUpdate } diff --git a/internal/manifest/ready_check.go b/internal/manifest/ready_check.go index 31845f0eb9..583fee20dd 100644 --- a/internal/manifest/ready_check.go +++ b/internal/manifest/ready_check.go @@ -15,8 +15,7 @@ import ( declarativev2 "github.com/kyma-project/lifecycle-manager/internal/declarative/v2" ) -// NewDeploymentReadyCheck creates a readiness check that verifies that the Resource in the Manifest -// returns the ready state, if not it returns not ready. +// NewDeploymentReadyCheck creates a readiness check that verifies if a Deployment is ready. func NewDeploymentReadyCheck() *DeploymentReadyCheck { return &DeploymentReadyCheck{} } @@ -27,11 +26,11 @@ func (c *DeploymentReadyCheck) Run(ctx context.Context, clnt declarativev2.Client, resources []*resource.Info, ) (declarativev2.StateInfo, error) { - deploymentState := getDeploymentState(clnt, resources) + deploymentState := getDeploymentState(ctx, clnt, resources) return declarativev2.StateInfo{State: deploymentState}, nil } -func getDeploymentState(clt declarativev2.Client, resources []*resource.Info) shared.State { +func getDeploymentState(ctx context.Context, clt declarativev2.Client, resources []*resource.Info) shared.State { deploy, found := findDeployment(clt, resources) // Not every module operator use Deployment by default, e.g: StatefulSet also a valid approach if !found { @@ -44,7 +43,7 @@ func getDeploymentState(clt declarativev2.Client, resources []*resource.Info) sh // Since deployment is not ready check if pods are ready or in error state // Get all Pods associated with the Deployment - podList, err := getPodsForDeployment(clt, deploy) + podList, err := getPodsForDeployment(ctx, clt, deploy) if err != nil { return shared.StateError } @@ -73,13 +72,14 @@ func isDeploymentReady(deploy *apiappsv1.Deployment) bool { return false } -func getPodsForDeployment(clt declarativev2.Client, deploy *apiappsv1.Deployment) (*apicorev1.PodList, error) { +func getPodsForDeployment(ctx context.Context, clt declarativev2.Client, + deploy *apiappsv1.Deployment) (*apicorev1.PodList, error) { podList := &apicorev1.PodList{} listOptions := &client.ListOptions{ Namespace: deploy.Namespace, LabelSelector: k8slabels.SelectorFromSet(deploy.Spec.Selector.MatchLabels), } - if err := clt.List(context.TODO(), podList, listOptions); err != nil { + if err := clt.List(ctx, podList, listOptions); err != nil { return nil, fmt.Errorf("failed to list pods: %w", err) } return podList, nil From 591121c14de3ed8d090fb5f2219bf15ceaec37cd Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Fri, 28 Jun 2024 12:31:01 +0200 Subject: [PATCH 18/25] Gofumpt --- internal/manifest/ready_check.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/manifest/ready_check.go b/internal/manifest/ready_check.go index 583fee20dd..21f34b71ed 100644 --- a/internal/manifest/ready_check.go +++ b/internal/manifest/ready_check.go @@ -73,7 +73,8 @@ func isDeploymentReady(deploy *apiappsv1.Deployment) bool { } func getPodsForDeployment(ctx context.Context, clt declarativev2.Client, - deploy *apiappsv1.Deployment) (*apicorev1.PodList, error) { + deploy *apiappsv1.Deployment, +) (*apicorev1.PodList, error) { podList := &apicorev1.PodList{} listOptions := &client.ListOptions{ Namespace: deploy.Namespace, From a6999869ceb7dea3ac3c390e779723ebfb04f301 Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Fri, 28 Jun 2024 14:57:32 +0200 Subject: [PATCH 19/25] Adress second review comments --- internal/declarative/v2/reconciler.go | 2 +- internal/manifest/ready_check.go | 5 +- internal/manifest/ready_check_test.go | 106 ++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 internal/manifest/ready_check_test.go diff --git a/internal/declarative/v2/reconciler.go b/internal/declarative/v2/reconciler.go index 0cf6cb7630..58a8c46675 100644 --- a/internal/declarative/v2/reconciler.go +++ b/internal/declarative/v2/reconciler.go @@ -399,7 +399,7 @@ func (r *Reconciler) checkDeploymentState( deploymentStateInfo, err := resourceReadyCheck.Run(ctx, clnt, target) if err != nil { - return "", err + return shared.StateError, err } if deploymentStateInfo.State == shared.StateProcessing { diff --git a/internal/manifest/ready_check.go b/internal/manifest/ready_check.go index 21f34b71ed..65a943e642 100644 --- a/internal/manifest/ready_check.go +++ b/internal/manifest/ready_check.go @@ -89,12 +89,15 @@ func getPodsForDeployment(ctx context.Context, clt declarativev2.Client, func getPodsState(podList *apicorev1.PodList) shared.State { for _, pod := range podList.Items { for _, condition := range pod.Status.ContainerStatuses { + if condition.Started == nil { + return shared.StateError + } switch { case *condition.Started && condition.Ready: return shared.StateReady case *condition.Started && !condition.Ready: return shared.StateProcessing - case !*condition.Started && !condition.Ready: + default: return shared.StateError } } diff --git a/internal/manifest/ready_check_test.go b/internal/manifest/ready_check_test.go new file mode 100644 index 0000000000..f070d9d55c --- /dev/null +++ b/internal/manifest/ready_check_test.go @@ -0,0 +1,106 @@ +package manifest + +import ( + "testing" + + "github.com/kyma-project/lifecycle-manager/api/shared" + "github.com/stretchr/testify/require" + apicorev1 "k8s.io/api/core/v1" + "k8s.io/utils/pointer" +) + +func Test_getPodsState(t *testing.T) { + tests := []struct { + name string + podList *apicorev1.PodList + want shared.State + }{ + { + name: "Test Ready State", + podList: &apicorev1.PodList{ + Items: []apicorev1.Pod{ + { + Status: apicorev1.PodStatus{ + ContainerStatuses: []apicorev1.ContainerStatus{ + { + Ready: true, + Started: pointer.Bool(true), + }, + }, + }, + }, + }, + }, + want: shared.StateReady, + }, + { + name: "Test Processing State", + podList: &apicorev1.PodList{ + Items: []apicorev1.Pod{ + { + Status: apicorev1.PodStatus{ + ContainerStatuses: []apicorev1.ContainerStatus{ + { + Ready: false, + Started: pointer.Bool(true), + }, + }, + }, + }, + }, + }, + want: shared.StateProcessing, + }, + { + name: "Test Error State", + podList: &apicorev1.PodList{ + Items: []apicorev1.Pod{ + { + Status: apicorev1.PodStatus{ + ContainerStatuses: []apicorev1.ContainerStatus{ + { + Ready: false, + Started: pointer.Bool(false), + }, + }, + }, + }, + }, + }, + want: shared.StateError, + }, + { + name: "Test Empty State", + podList: &apicorev1.PodList{ + Items: []apicorev1.Pod{ + { + Status: apicorev1.PodStatus{}, + }, + }, + }, + want: shared.StateError, + }, + { + name: "Test Empty Started Condition", + podList: &apicorev1.PodList{ + Items: []apicorev1.Pod{ + { + Status: apicorev1.PodStatus{ + ContainerStatuses: []apicorev1.ContainerStatus{ + { + Ready: false, + }, + }, + }, + }, + }, + }, + want: shared.StateError, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, getPodsState(tt.podList)) + }) + } +} From d33dcc6b80b666e54434591db6106922b402ef9d Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Fri, 28 Jun 2024 15:20:43 +0200 Subject: [PATCH 20/25] Introduce more unit tests --- internal/manifest/ready_check.go | 8 +-- internal/manifest/ready_check_test.go | 80 ++++++++++++++++++++++++--- 2 files changed, 77 insertions(+), 11 deletions(-) diff --git a/internal/manifest/ready_check.go b/internal/manifest/ready_check.go index 65a943e642..3bf3dd48a0 100644 --- a/internal/manifest/ready_check.go +++ b/internal/manifest/ready_check.go @@ -37,7 +37,7 @@ func getDeploymentState(ctx context.Context, clt declarativev2.Client, resources return shared.StateReady } - if isDeploymentReady(deploy) { + if IsDeploymentReady(deploy) { return shared.StateReady } @@ -48,7 +48,7 @@ func getDeploymentState(ctx context.Context, clt declarativev2.Client, resources return shared.StateError } - return getPodsState(podList) + return GetPodsState(podList) } func findDeployment(clt declarativev2.Client, resources []*resource.Info) (*apiappsv1.Deployment, bool) { @@ -61,7 +61,7 @@ func findDeployment(clt declarativev2.Client, resources []*resource.Info) (*apia return nil, false } -func isDeploymentReady(deploy *apiappsv1.Deployment) bool { +func IsDeploymentReady(deploy *apiappsv1.Deployment) bool { availableCond := deployment.GetDeploymentCondition(deploy.Status, apiappsv1.DeploymentAvailable) if availableCond != nil && availableCond.Status == apicorev1.ConditionTrue { return true @@ -86,7 +86,7 @@ func getPodsForDeployment(ctx context.Context, clt declarativev2.Client, return podList, nil } -func getPodsState(podList *apicorev1.PodList) shared.State { +func GetPodsState(podList *apicorev1.PodList) shared.State { for _, pod := range podList.Items { for _, condition := range pod.Status.ContainerStatuses { if condition.Started == nil { diff --git a/internal/manifest/ready_check_test.go b/internal/manifest/ready_check_test.go index f070d9d55c..0c6f668aaa 100644 --- a/internal/manifest/ready_check_test.go +++ b/internal/manifest/ready_check_test.go @@ -1,12 +1,15 @@ -package manifest +package manifest_test import ( "testing" - "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/stretchr/testify/require" + apiappsv1 "k8s.io/api/apps/v1" apicorev1 "k8s.io/api/core/v1" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" + + "github.com/kyma-project/lifecycle-manager/api/shared" + "github.com/kyma-project/lifecycle-manager/internal/manifest" ) func Test_getPodsState(t *testing.T) { @@ -24,7 +27,7 @@ func Test_getPodsState(t *testing.T) { ContainerStatuses: []apicorev1.ContainerStatus{ { Ready: true, - Started: pointer.Bool(true), + Started: ptr.To(true), }, }, }, @@ -42,7 +45,7 @@ func Test_getPodsState(t *testing.T) { ContainerStatuses: []apicorev1.ContainerStatus{ { Ready: false, - Started: pointer.Bool(true), + Started: ptr.To(true), }, }, }, @@ -60,7 +63,7 @@ func Test_getPodsState(t *testing.T) { ContainerStatuses: []apicorev1.ContainerStatus{ { Ready: false, - Started: pointer.Bool(false), + Started: ptr.To(false), }, }, }, @@ -100,7 +103,70 @@ func Test_getPodsState(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - require.Equal(t, tt.want, getPodsState(tt.podList)) + require.Equal(t, tt.want, manifest.GetPodsState(tt.podList)) + }) + } +} + +func Test_IsDeploymentReady(t *testing.T) { + tests := []struct { + name string + deploy *apiappsv1.Deployment + expected bool + }{ + { + name: "Test Deployment Ready", + deploy: &apiappsv1.Deployment{ + Status: apiappsv1.DeploymentStatus{ + Conditions: []apiappsv1.DeploymentCondition{ + { + Type: apiappsv1.DeploymentAvailable, + Status: apicorev1.ConditionTrue, + }, + }, + ReadyReplicas: 1, + }, + Spec: apiappsv1.DeploymentSpec{ + Replicas: ptr.To(int32(1)), + }, + }, + expected: true, + }, + { + name: "Test Deployment Ready using Conditions", + deploy: &apiappsv1.Deployment{ + Status: apiappsv1.DeploymentStatus{ + ReadyReplicas: 1, + }, + Spec: apiappsv1.DeploymentSpec{ + Replicas: ptr.To(int32(1)), + }, + }, + expected: true, + }, + { + name: "Test Deployment Not Ready", + deploy: &apiappsv1.Deployment{ + Status: apiappsv1.DeploymentStatus{ + Conditions: []apiappsv1.DeploymentCondition{ + { + Type: apiappsv1.DeploymentAvailable, + Status: apicorev1.ConditionFalse, + }, + }, + ReadyReplicas: 0, + }, + Spec: apiappsv1.DeploymentSpec{ + Replicas: ptr.To(int32(1)), + }, + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.expected, manifest.IsDeploymentReady(tt.deploy)) }) } } From ab5c26e1144be5f046f684e0c15fe907b34ae894 Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Fri, 28 Jun 2024 15:55:00 +0200 Subject: [PATCH 21/25] Remove duplicated check --- internal/declarative/v2/reconciler.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/declarative/v2/reconciler.go b/internal/declarative/v2/reconciler.go index 58a8c46675..0aa4ad9439 100644 --- a/internal/declarative/v2/reconciler.go +++ b/internal/declarative/v2/reconciler.go @@ -406,10 +406,6 @@ func (r *Reconciler) checkDeploymentState( return shared.StateProcessing, nil } - if !obj.GetDeletionTimestamp().IsZero() { - return shared.StateDeleting, nil - } - return deploymentStateInfo.State, nil } From 0c58ac4d56e7c0b17aa0ee08cbf4dd56512612d0 Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Fri, 28 Jun 2024 15:56:27 +0200 Subject: [PATCH 22/25] Remove duplicated check --- internal/declarative/v2/reconciler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/declarative/v2/reconciler.go b/internal/declarative/v2/reconciler.go index 0aa4ad9439..8aee8ef058 100644 --- a/internal/declarative/v2/reconciler.go +++ b/internal/declarative/v2/reconciler.go @@ -363,7 +363,7 @@ func (r *Reconciler) syncResources(ctx context.Context, clnt Client, obj Object, } } - deploymentState, err := r.checkDeploymentState(ctx, clnt, obj, target) + deploymentState, err := r.checkDeploymentState(ctx, clnt, target) if err != nil { obj.SetStatus(status.WithState(shared.StateError).WithErr(err)) return err @@ -393,7 +393,7 @@ func hasDiff(oldResources []shared.Resource, newResources []shared.Resource) boo } func (r *Reconciler) checkDeploymentState( - ctx context.Context, clnt Client, obj Object, target []*resource.Info, + ctx context.Context, clnt Client, target []*resource.Info, ) (shared.State, error) { resourceReadyCheck := r.CustomReadyCheck From b496ecd3fd7d388801864eb80f7e4609568a0bfe Mon Sep 17 00:00:00 2001 From: Jeremy Harisch <48282931+jeremyharisch@users.noreply.github.com> Date: Mon, 1 Jul 2024 16:55:11 +0200 Subject: [PATCH 23/25] Apply suggestions from code review Co-authored-by: Grzegorz Karaluch --- docs/technical-reference/api/manifest-cr.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/technical-reference/api/manifest-cr.md b/docs/technical-reference/api/manifest-cr.md index 7dcaa560c9..b446e134b4 100644 --- a/docs/technical-reference/api/manifest-cr.md +++ b/docs/technical-reference/api/manifest-cr.md @@ -89,12 +89,12 @@ The resource is the default data that should be initialized for the module and i The Manifest CR status is set based on the following logic, managed by the manifest reconciler: -- If the Module defined in the Manifest CR is successfully applied and the deployed Module is up and running, the status of the Manifest CR is set to `Ready`. -- While the Manifest is being applied and the Deployment is still starting, the status of the Manifest CR is set to `Processing`. -- If the Deployment is unable to start (for example, due to an `ImagePullBackOff` error) or if the application of the Manifest fails, the status of the Manifest CR is set to `Error`. +- If the module defined in the Manifest CR is successfully applied and the deployed module is up and running, the status of the Manifest CR is set to `Ready`. +- While the manifest is being applied and the Deployment is still starting, the status of the Manifest CR is set to `Processing`. +- If the Deployment is unable to start (for example, due to an `ImagePullBackOff` error) or if the application of the manifest fails, the status of the Manifest CR is set to `Error`. - If the Manifest CR is marked for deletion, the status of the Manifest CR is set to `Deleting`. -This status provides a reliable way to track the state of the Manifest CR and the associated Module, offering insights into the deployment process and any potential issues, while being decoupled of the Module business logic. +This status provides a reliable way to track the state of the Manifest CR and the associated module, offering insights into the deployment process and any potential issues, while being decoupled from the module business logic. ### **.metadata.labels** From c093a0b9dae500dfbf2c3a97818f66b55d8a2be6 Mon Sep 17 00:00:00 2001 From: jeremyharisch Date: Mon, 1 Jul 2024 17:33:23 +0200 Subject: [PATCH 24/25] Remove Stateinfo --- internal/declarative/v2/ready_check.go | 15 +++++---------- internal/declarative/v2/reconciler.go | 15 ++++----------- internal/manifest/ready_check.go | 4 ++-- .../controller/manifest/ready_check_test.go | 4 ++-- 4 files changed, 13 insertions(+), 25 deletions(-) diff --git a/internal/declarative/v2/ready_check.go b/internal/declarative/v2/ready_check.go index dde1daa65f..04725eb6c8 100644 --- a/internal/declarative/v2/ready_check.go +++ b/internal/declarative/v2/ready_check.go @@ -13,13 +13,8 @@ import ( var ErrNotValidClientObject = errors.New("object in resource info is not a valid client object") -type StateInfo struct { - shared.State - Info string -} - type ReadyCheck interface { - Run(ctx context.Context, clnt Client, resources []*resource.Info) (StateInfo, error) + Run(ctx context.Context, clnt Client, resources []*resource.Info) (shared.State, error) } func NewExistsReadyCheck() *ExistsReadyCheck { @@ -32,15 +27,15 @@ func (c *ExistsReadyCheck) Run( ctx context.Context, clnt Client, resources []*resource.Info, -) (StateInfo, error) { +) (shared.State, error) { for i := range resources { obj, ok := resources[i].Object.(client.Object) if !ok { - return StateInfo{State: shared.StateError}, ErrNotValidClientObject + return shared.StateError, ErrNotValidClientObject } if err := clnt.Get(ctx, client.ObjectKeyFromObject(obj), obj); client.IgnoreNotFound(err) != nil { - return StateInfo{State: shared.StateError}, fmt.Errorf("failed to fetch object by key: %w", err) + return shared.StateError, fmt.Errorf("failed to fetch object by key: %w", err) } } - return StateInfo{State: shared.StateReady}, nil + return shared.StateReady, nil } diff --git a/internal/declarative/v2/reconciler.go b/internal/declarative/v2/reconciler.go index 8aee8ef058..c7cf54ce30 100644 --- a/internal/declarative/v2/reconciler.go +++ b/internal/declarative/v2/reconciler.go @@ -397,16 +397,16 @@ func (r *Reconciler) checkDeploymentState( ) (shared.State, error) { resourceReadyCheck := r.CustomReadyCheck - deploymentStateInfo, err := resourceReadyCheck.Run(ctx, clnt, target) + deploymentState, err := resourceReadyCheck.Run(ctx, clnt, target) if err != nil { return shared.StateError, err } - if deploymentStateInfo.State == shared.StateProcessing { + if deploymentState == shared.StateProcessing { return shared.StateProcessing, nil } - return deploymentStateInfo.State, nil + return deploymentState, nil } func (r *Reconciler) setManifestState(manifest Object, state shared.State) error { @@ -428,20 +428,13 @@ func (r *Reconciler) setManifestState(manifest Object, state shared.State) error installationCondition.Status = apimetav1.ConditionTrue meta.SetStatusCondition(&status.Conditions, installationCondition) manifest.SetStatus(status.WithState(state). - WithOperation(generateOperationMessage(installationCondition, StateInfo{State: state}))) + WithOperation(installationCondition.Message)) return ErrInstallationConditionRequiresUpdate } return nil } -func generateOperationMessage(installationCondition apimetav1.Condition, stateInfo StateInfo) string { - if stateInfo.Info != "" { - return stateInfo.Info - } - return installationCondition.Message -} - func (r *Reconciler) removeModuleCR(ctx context.Context, clnt Client, obj Object) error { if !obj.GetDeletionTimestamp().IsZero() { for _, preDelete := range r.PreDeletes { diff --git a/internal/manifest/ready_check.go b/internal/manifest/ready_check.go index 3bf3dd48a0..04e6f67877 100644 --- a/internal/manifest/ready_check.go +++ b/internal/manifest/ready_check.go @@ -25,9 +25,9 @@ type DeploymentReadyCheck struct{} func (c *DeploymentReadyCheck) Run(ctx context.Context, clnt declarativev2.Client, resources []*resource.Info, -) (declarativev2.StateInfo, error) { +) (shared.State, error) { deploymentState := getDeploymentState(ctx, clnt, resources) - return declarativev2.StateInfo{State: deploymentState}, nil + return deploymentState, nil } func getDeploymentState(ctx context.Context, clt declarativev2.Client, resources []*resource.Info) shared.State { diff --git a/tests/integration/controller/manifest/ready_check_test.go b/tests/integration/controller/manifest/ready_check_test.go index 734c954477..3a406e6b46 100644 --- a/tests/integration/controller/manifest/ready_check_test.go +++ b/tests/integration/controller/manifest/ready_check_test.go @@ -77,9 +77,9 @@ var _ = Describe("Manifest readiness check", Ordered, func() { By("Executing the CR readiness check") customReadyCheck := manifest.NewDeploymentReadyCheck() - stateInfo, err := customReadyCheck.Run(ctx, testClient, resources) + state, err := customReadyCheck.Run(ctx, testClient, resources) Expect(err).NotTo(HaveOccurred()) - Expect(stateInfo.State).To(Equal(shared.StateReady)) + Expect(state).To(Equal(shared.StateReady)) By("cleaning up the manifest") Eventually(verifyObjectExists(ctx, kcpClient, expectedDeployment.ToUnstructured()), standardTimeout, From 5be43d5191905250ec7fa83f2c8a636cd876f03c Mon Sep 17 00:00:00 2001 From: Jeremy Harisch <48282931+jeremyharisch@users.noreply.github.com> Date: Tue, 2 Jul 2024 10:59:18 +0200 Subject: [PATCH 25/25] Apply suggestions from code review Co-authored-by: Iwona Langer --- docs/technical-reference/api/manifest-cr.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/technical-reference/api/manifest-cr.md b/docs/technical-reference/api/manifest-cr.md index b446e134b4..e6ee00ef8c 100644 --- a/docs/technical-reference/api/manifest-cr.md +++ b/docs/technical-reference/api/manifest-cr.md @@ -91,10 +91,10 @@ The Manifest CR status is set based on the following logic, managed by the manif - If the module defined in the Manifest CR is successfully applied and the deployed module is up and running, the status of the Manifest CR is set to `Ready`. - While the manifest is being applied and the Deployment is still starting, the status of the Manifest CR is set to `Processing`. -- If the Deployment is unable to start (for example, due to an `ImagePullBackOff` error) or if the application of the manifest fails, the status of the Manifest CR is set to `Error`. +- If the Deployment cannot start (for example, due to an `ImagePullBackOff` error) or if the application of the manifest fails, the status of the Manifest CR is set to `Error`. - If the Manifest CR is marked for deletion, the status of the Manifest CR is set to `Deleting`. -This status provides a reliable way to track the state of the Manifest CR and the associated module, offering insights into the deployment process and any potential issues, while being decoupled from the module business logic. +This status provides a reliable way to track the state of the Manifest CR and the associated module. It offers insights into the deployment process and any potential issues while being decoupled from the module's business logic. ### **.metadata.labels**