diff --git a/go.sum b/go.sum index dbc183f47b..c257e2da23 100644 --- a/go.sum +++ b/go.sum @@ -1231,8 +1231,6 @@ google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQ google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= google.golang.org/grpc v1.31.0/go.mod h1:N36X2cJ7JwdamYAgDz+s+rVMFjt3numwzf/HckM8pak= google.golang.org/grpc v1.33.2/go.mod h1:JMHMWHQWaTccqQQlmk3MJZS+GWXOdAesneDmEnv2fbc= -google.golang.org/grpc v1.67.1 h1:zWnc1Vrcno+lHZCOofnIMvycFcc0QRGIzm9dhnDX68E= -google.golang.org/grpc v1.67.1/go.mod h1:1gLDyUQU7CTLJI90u3nXZ9ekeghjeM7pTDZlqFNg2AA= google.golang.org/grpc v1.68.0 h1:aHQeeJbo8zAkAa3pRzrVjZlbz6uSfeOXlJNQM0RAbz0= google.golang.org/grpc v1.68.0/go.mod h1:fmSPC5AsjSBCK54MyHRx48kpOti1/jRfOlwEWywNjWA= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= diff --git a/internal/declarative/v2/reconciler.go b/internal/declarative/v2/reconciler.go index cf1b1b4584..aa771d8149 100644 --- a/internal/declarative/v2/reconciler.go +++ b/internal/declarative/v2/reconciler.go @@ -34,6 +34,7 @@ var ( ErrManagerInErrorState = errors.New("manager is in error state") ErrResourceSyncDiffInSameOCILayer = errors.New("resource syncTarget diff detected but in " + "same oci layer, prevent sync resource to be deleted") + errStateRequireUpdate = errors.New("manifest state requires update") ) const ( @@ -334,7 +335,10 @@ func (r *Reconciler) syncManifestState(ctx context.Context, skrClient Client, ma } status.ConfirmModuleCRCondition(manifest) } - return status.SetManifestState(manifest, shared.StateDeleting) + if status.RequireManifestStateUpdateAfterSyncResource(manifest, shared.StateDeleting) { + return errStateRequireUpdate + } + return nil } managerState, err := r.checkManagerState(ctx, skrClient, target) @@ -342,7 +346,10 @@ func (r *Reconciler) syncManifestState(ctx context.Context, skrClient Client, ma manifest.SetStatus(manifestStatus.WithState(shared.StateError).WithErr(err)) return err } - return status.SetManifestState(manifest, managerState) + if status.RequireManifestStateUpdateAfterSyncResource(manifest, managerState) { + return errStateRequireUpdate + } + return nil } func (r *Reconciler) RecordModuleCRWarningCondition(manifest *v1beta2.Manifest) error { diff --git a/internal/manifest/status/condition.go b/internal/manifest/status/condition.go index 221b5dfb04..06a20172df 100644 --- a/internal/manifest/status/condition.go +++ b/internal/manifest/status/condition.go @@ -74,3 +74,14 @@ func ConfirmResourcesCondition(manifest *v1beta2.Manifest) { manifest.SetStatus(status.WithOperation(resourceCondition.Message)) } } + +func ConfirmInstallationCondition(manifest *v1beta2.Manifest) { + status := manifest.GetStatus() + installationCondition := initInstallationCondition(manifest) + + if !meta.IsStatusConditionTrue(status.Conditions, installationCondition.Type) { + installationCondition.Status = apimetav1.ConditionTrue + meta.SetStatusCondition(&status.Conditions, installationCondition) + manifest.SetStatus(status.WithOperation(installationCondition.Message)) + } +} diff --git a/internal/manifest/status/condition_test.go b/internal/manifest/status/condition_test.go new file mode 100644 index 0000000000..17a0d570c9 --- /dev/null +++ b/internal/manifest/status/condition_test.go @@ -0,0 +1,40 @@ +package status_test + +import ( + "testing" + + apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/internal/manifest/status" +) + +func TestConfirmInstallationCondition(t *testing.T) { + manifest := &v1beta2.Manifest{} + manifest.SetGeneration(1) + + status.ConfirmInstallationCondition(manifest) + + conditions := manifest.GetStatus().Conditions + if len(conditions) != 1 { + t.Fatalf("expected 1 condition, got %d", len(conditions)) + } + + condition := conditions[0] + if condition.Type != string(status.ConditionTypeInstallation) { + t.Errorf("expected condition type %s, got %s", status.ConditionTypeInstallation, condition.Type) + } + if condition.Reason != string(status.ConditionReasonReady) { + t.Errorf("expected condition reason %s, got %s", status.ConditionReasonReady, condition.Reason) + } + if condition.Status != apimetav1.ConditionTrue { + t.Errorf("expected condition status %s, got %s", apimetav1.ConditionTrue, condition.Status) + } + if condition.Message != "installation is ready and resources can be used" { + t.Errorf("expected condition message %s, got %s", "installation is ready and resources can be used", + condition.Message) + } + if condition.ObservedGeneration != manifest.GetGeneration() { + t.Errorf("expected observed generation %d, got %d", manifest.GetGeneration(), condition.ObservedGeneration) + } +} diff --git a/internal/manifest/status/state.go b/internal/manifest/status/state.go index cc41d544f7..f51fa00ec6 100644 --- a/internal/manifest/status/state.go +++ b/internal/manifest/status/state.go @@ -1,35 +1,27 @@ package status import ( - "errors" - - "k8s.io/apimachinery/pkg/api/meta" - apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" ) -const waitingForResourcesMsg = "waiting for resources to become ready" - -var ErrInstallationConditionRequiresUpdate = errors.New("installation condition needs an update") - -func SetManifestState(manifest *v1beta2.Manifest, newState shared.State) error { - status := manifest.GetStatus() - - if newState == shared.StateProcessing { - manifest.SetStatus(status.WithState(shared.StateProcessing).WithOperation(waitingForResourcesMsg)) - return ErrInstallationConditionRequiresUpdate - } - - installationCondition := initInstallationCondition(manifest) - if newState != status.State || !meta.IsStatusConditionTrue(status.Conditions, installationCondition.Type) { - installationCondition.Status = apimetav1.ConditionTrue - meta.SetStatusCondition(&status.Conditions, installationCondition) +const ( + ResourcesAreReadyMsg = "resources are ready" + WaitingForResourcesMsg = "waiting for resources to become ready" +) - manifest.SetStatus(status.WithState(newState).WithOperation(installationCondition.Message)) - return ErrInstallationConditionRequiresUpdate +func RequireManifestStateUpdateAfterSyncResource(manifest *v1beta2.Manifest, newState shared.State) bool { + manifestStatus := manifest.GetStatus() + + if newState != manifestStatus.State { + if newState == shared.StateProcessing || newState == shared.StateError { + manifest.SetStatus(manifestStatus.WithState(newState).WithOperation(WaitingForResourcesMsg)) + } else { + ConfirmInstallationCondition(manifest) + manifest.SetStatus(manifestStatus.WithState(newState).WithOperation(ResourcesAreReadyMsg)) + } + return true } - return nil + return false } diff --git a/internal/manifest/status/state_test.go b/internal/manifest/status/state_test.go new file mode 100644 index 0000000000..fa43fad807 --- /dev/null +++ b/internal/manifest/status/state_test.go @@ -0,0 +1,66 @@ +package status_test + +import ( + "testing" + + "github.com/kyma-project/lifecycle-manager/api/shared" + "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/internal/manifest/status" +) + +func TestRequireManifestStateUpdateAfterSyncResource(t *testing.T) { + tests := []struct { + name string + newState shared.State + expectedState shared.State + expectedOp string + expectUpdate bool + }{ + { + name: "State changes to Processing", + newState: shared.StateProcessing, + expectedState: shared.StateProcessing, + expectedOp: status.WaitingForResourcesMsg, + expectUpdate: true, + }, + { + name: "State changes to Error", + newState: shared.StateError, + expectedState: shared.StateError, + expectedOp: status.WaitingForResourcesMsg, + expectUpdate: true, + }, + { + name: "State changes to Ready", + newState: shared.StateReady, + expectedState: shared.StateReady, + expectedOp: status.ResourcesAreReadyMsg, + expectUpdate: false, + }, + } + + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + manifest := &v1beta2.Manifest{} + manifest.SetGeneration(1) + manifestStatus := shared.Status{ + State: shared.StateReady, + } + manifestStatus = manifestStatus.WithOperation(status.ResourcesAreReadyMsg) + manifest.SetStatus(manifestStatus) + + updated := status.RequireManifestStateUpdateAfterSyncResource(manifest, testCase.newState) + if updated != testCase.expectUpdate { + t.Errorf("expected update to be %v, got %v", testCase.expectUpdate, updated) + } + + if manifest.GetStatus().State != testCase.expectedState { + t.Errorf("expected state to be %v, got %v", testCase.expectedState, manifest.GetStatus().State) + } + + if manifest.GetStatus().Operation != testCase.expectedOp { + t.Errorf("expected operation to be %v, got %v", testCase.expectedOp, manifest.GetStatus().Operation) + } + }) + } +} diff --git a/tests/integration/controller/manifest/manifest_controller_test.go b/tests/integration/controller/manifest/manifest_controller_test.go index cbd3ff9261..7bc52fad78 100644 --- a/tests/integration/controller/manifest/manifest_controller_test.go +++ b/tests/integration/controller/manifest/manifest_controller_test.go @@ -9,6 +9,7 @@ import ( "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/internal/manifest/status" "github.com/kyma-project/lifecycle-manager/pkg/ocmextensions" . "github.com/kyma-project/lifecycle-manager/pkg/testutils" @@ -150,7 +151,7 @@ var _ = Describe( standardInterval). WithContext(ctx). WithArguments(kcpClient, manifest.GetName(), - "installation is ready and resources can be used"). + status.ResourcesAreReadyMsg). Should(Succeed()) })