diff --git a/internal/declarative/v2/reconciler.go b/internal/declarative/v2/reconciler.go index 880730c9dd..d85bd32b27 100644 --- a/internal/declarative/v2/reconciler.go +++ b/internal/declarative/v2/reconciler.go @@ -69,6 +69,8 @@ type Reconciler struct { specResolver SpecResolver } +const waitingForResourcesMsg = "waiting for resources to become ready" + type ConditionType string const ( @@ -366,6 +368,10 @@ func (r *Reconciler) syncResources(ctx context.Context, clnt Client, manifest *v } } + if !manifest.GetDeletionTimestamp().IsZero() { + return r.setManifestState(manifest, shared.StateDeleting) + } + managerState, err := r.checkManagerState(ctx, clnt, target) if err != nil { manifest.SetStatus(status.WithState(shared.StateError).WithErr(err)) @@ -399,39 +405,28 @@ func (r *Reconciler) checkManagerState(ctx context.Context, clnt Client, target error, ) { managerReadyCheck := r.CustomStateCheck - managerState, err := managerReadyCheck.GetState(ctx, clnt, target) if err != nil { return shared.StateError, err } - if managerState == shared.StateProcessing { - return shared.StateProcessing, nil - } - return managerState, nil } -func (r *Reconciler) setManifestState(manifest *v1beta2.Manifest, state shared.State) error { +func (r *Reconciler) setManifestState(manifest *v1beta2.Manifest, newState 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)) + if newState == shared.StateProcessing { + manifest.SetStatus(status.WithState(shared.StateProcessing).WithOperation(waitingForResourcesMsg)) return ErrInstallationConditionRequiresUpdate } - if !manifest.GetDeletionTimestamp().IsZero() { - state = shared.StateDeleting - } - installationCondition := newInstallationCondition(manifest) - if !meta.IsStatusConditionTrue(status.Conditions, - installationCondition.Type) || status.State != state { + if newState != status.State || !meta.IsStatusConditionTrue(status.Conditions, installationCondition.Type) { installationCondition.Status = apimetav1.ConditionTrue meta.SetStatusCondition(&status.Conditions, installationCondition) - manifest.SetStatus(status.WithState(state). - WithOperation(installationCondition.Message)) + + manifest.SetStatus(status.WithState(newState).WithOperation(installationCondition.Message)) return ErrInstallationConditionRequiresUpdate } diff --git a/internal/manifest/statecheck/state_check.go b/internal/manifest/statecheck/state_check.go index 29ac1d577c..e37a9d446a 100644 --- a/internal/manifest/statecheck/state_check.go +++ b/internal/manifest/statecheck/state_check.go @@ -2,6 +2,7 @@ package statecheck import ( "context" + "errors" apiappsv1 "k8s.io/api/apps/v1" "k8s.io/cli-runtime/pkg/resource" @@ -30,6 +31,11 @@ const ( StatefulSetKind ManagerKind = "StatefulSet" ) +var ( + ErrNoManagerProvided = errors.New("failed to find manager in provided resources") + ErrNoStateDetermined = errors.New("failed to determine state for manager") +) + type Manager struct { kind ManagerKind deployment *apiappsv1.Deployment @@ -45,13 +51,16 @@ func NewManagerStateCheck(statefulSetChecker StatefulSetStateChecker, } } +// Determines the state based on the manager. The manager may either be a Deployment or a StatefulSet and +// must be included in the provided resources. +// Will be refactored with https://github.com/kyma-project/lifecycle-manager/issues/1831 func (m *ManagerStateCheck) GetState(ctx context.Context, clnt client.Client, resources []*resource.Info, ) (shared.State, error) { mgr := findManager(clnt, resources) if mgr == nil { - return shared.StateReady, nil + return shared.StateError, ErrNoManagerProvided } switch mgr.kind { @@ -61,7 +70,8 @@ func (m *ManagerStateCheck) GetState(ctx context.Context, return m.deploymentStateChecker.GetState(mgr.deployment) } - return shared.StateReady, nil + // fall through that should not be reached + return shared.StateError, ErrNoStateDetermined } func findManager(clt client.Client, resources []*resource.Info) *Manager { diff --git a/internal/manifest/statecheck/state_check_test.go b/internal/manifest/statecheck/state_check_test.go index f199b3999c..088bec6c45 100644 --- a/internal/manifest/statecheck/state_check_test.go +++ b/internal/manifest/statecheck/state_check_test.go @@ -18,9 +18,11 @@ import ( func TestManagerStateCheck_GetState(t *testing.T) { tests := []struct { - name string - resources []*resource.Info - isDeployment bool + name string + resources []*resource.Info + isDeployment bool + isStateFulSet bool + expectedError error }{ { name: "Test Deployment State Checker", @@ -31,7 +33,9 @@ func TestManagerStateCheck_GetState(t *testing.T) { }, }, }, - isDeployment: true, + isDeployment: true, + isStateFulSet: false, + expectedError: nil, }, { name: "Test StatefulSet State Checker", @@ -42,7 +46,16 @@ func TestManagerStateCheck_GetState(t *testing.T) { }, }, }, - isDeployment: false, + isDeployment: false, + isStateFulSet: true, + expectedError: nil, + }, + { + name: "Test no manager found", + resources: []*resource.Info{}, + isDeployment: false, + isStateFulSet: false, + expectedError: statecheck.ErrNoManagerProvided, }, } for _, testCase := range tests { @@ -55,12 +68,20 @@ func TestManagerStateCheck_GetState(t *testing.T) { deploymentChecker := &DeploymentStateCheckerStub{} m := statecheck.NewManagerStateCheck(statefulsetChecker, deploymentChecker) got, err := m.GetState(context.Background(), clnt, testCase.resources) - require.NoError(t, err) + + if testCase.expectedError == nil { + require.NoError(t, err) + } else { + require.Equal(t, testCase.expectedError, err) + } + if testCase.isDeployment { require.True(t, deploymentChecker.called) require.False(t, statefulsetChecker.called) require.Equal(t, shared.StateProcessing, got) - } else { + } + + if testCase.isStateFulSet { require.True(t, statefulsetChecker.called) require.False(t, deploymentChecker.called) require.Equal(t, shared.StateReady, got)