Skip to content

Commit

Permalink
chore: Make set manifest state more understandable (#2074)
Browse files Browse the repository at this point in the history
* make set manifest state more understandable

* fix test

* add unit test

* fix lint
  • Loading branch information
ruanxin authored Dec 3, 2024
1 parent c6c57ea commit 1158247
Show file tree
Hide file tree
Showing 7 changed files with 144 additions and 29 deletions.
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
11 changes: 9 additions & 2 deletions internal/declarative/v2/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -334,15 +335,21 @@ 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)
if err != nil {
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 {
Expand Down
11 changes: 11 additions & 0 deletions internal/manifest/status/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
40 changes: 40 additions & 0 deletions internal/manifest/status/condition_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
40 changes: 16 additions & 24 deletions internal/manifest/status/state.go
Original file line number Diff line number Diff line change
@@ -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
}
66 changes: 66 additions & 0 deletions internal/manifest/status/state_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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())
})

Expand Down

0 comments on commit 1158247

Please sign in to comment.