From bc9358b5be17de2ab1ea69f8e4f8aa9d34eaa90b Mon Sep 17 00:00:00 2001 From: Vladimir Andjelkoski Date: Mon, 14 Oct 2024 09:46:39 +0200 Subject: [PATCH 1/2] chore: setExclusiveConditions --- pkg/composed/errors.go | 10 -- pkg/composed/setExclusiveConditions.go | 25 +++++ pkg/composed/setExclusiveConditions_test.go | 105 ++++++++++++++++++ .../provider/aws/vpcpeering/loadRemoteVpc.go | 14 +-- pkg/skr/awsvpcpeering/updateStatus.go | 3 +- pkg/skr/azurevpcpeering/updateStatus.go | 3 +- 6 files changed, 136 insertions(+), 24 deletions(-) create mode 100644 pkg/composed/setExclusiveConditions.go create mode 100644 pkg/composed/setExclusiveConditions_test.go diff --git a/pkg/composed/errors.go b/pkg/composed/errors.go index 1306f2918..fd233a7ca 100644 --- a/pkg/composed/errors.go +++ b/pkg/composed/errors.go @@ -3,9 +3,6 @@ package composed import ( "errors" "fmt" - "github.com/elliotchance/pie/v2" - "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/reconcile" "time" ) @@ -96,10 +93,3 @@ func (e *stopWithRequeueDelay) Delay() time.Duration { func StopWithRequeueDelay(d time.Duration) error { return &stopWithRequeueDelay{delay: d} } - -func AnyConditionChanged(obj ObjWithConditions, conditionsToSet ...metav1.Condition) bool { - return pie.All(conditionsToSet, func(x metav1.Condition) bool { - c := meta.FindStatusCondition(*obj.Conditions(), x.Type) - return c == nil || c.Reason != x.Reason || c.Message != x.Message || c.Status != x.Status - }) -} diff --git a/pkg/composed/setExclusiveConditions.go b/pkg/composed/setExclusiveConditions.go new file mode 100644 index 000000000..29d15fe5c --- /dev/null +++ b/pkg/composed/setExclusiveConditions.go @@ -0,0 +1,25 @@ +package composed + +import ( + "github.com/elliotchance/pie/v2" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func SetExclusiveConditions(conditions *[]metav1.Condition, conditionsToSet ...metav1.Condition) bool { + changed := false + + conditionsToRemove := pie.Filter(*conditions, func(condition metav1.Condition) bool { + return meta.FindStatusCondition(conditionsToSet, condition.Type) == nil + }) + + pie.Each(conditionsToRemove, func(condition metav1.Condition) { + changed = changed || meta.RemoveStatusCondition(conditions, condition.Type) + }) + + pie.Each(conditionsToSet, func(condition metav1.Condition) { + changed = changed || meta.SetStatusCondition(conditions, condition) + }) + + return changed +} diff --git a/pkg/composed/setExclusiveConditions_test.go b/pkg/composed/setExclusiveConditions_test.go new file mode 100644 index 000000000..754e509f8 --- /dev/null +++ b/pkg/composed/setExclusiveConditions_test.go @@ -0,0 +1,105 @@ +package composed + +import ( + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "testing" +) + +type SetStatusConditionsSuite struct { + suite.Suite +} + +const ( + conditionTypeReady = "Ready" + conditionTypeError = "Error" + conditionTypeWarning = "Warning" +) + +func (me *SetStatusConditionsSuite) TestConditionRemoved() { + conditions := &[]metav1.Condition{ + { + Type: conditionTypeReady, + Status: metav1.ConditionTrue, + }, + { + Type: conditionTypeError, + Status: metav1.ConditionTrue, + }, + } + + assert.True(me.T(), SetExclusiveConditions(conditions, metav1.Condition{ + Type: conditionTypeReady, + Status: metav1.ConditionTrue, + }), "Conditions changes should be detected, but didn't") + + assert.Equal(me.T(), 1, len(*conditions)) +} + +func (me *SetStatusConditionsSuite) TestConditionAdded() { + conditions := &[]metav1.Condition{ + { + Type: conditionTypeReady, + Status: metav1.ConditionTrue, + }, + { + Type: conditionTypeError, + Status: metav1.ConditionTrue, + }, + } + + assert.True(me.T(), SetExclusiveConditions(conditions, + []metav1.Condition{ + { + Type: conditionTypeReady, + Status: metav1.ConditionTrue, + }, + { + Type: conditionTypeError, + Status: metav1.ConditionTrue, + }, + { + Type: conditionTypeWarning, + Status: metav1.ConditionTrue, + }, + }..., + ), "Conditions changes should be detected, but didn't") + + assert.Equal(me.T(), 3, len(*conditions)) + +} + +func (me *SetStatusConditionsSuite) TestConditionChanged() { + + conditions := &[]metav1.Condition{ + { + Type: conditionTypeReady, + Status: metav1.ConditionTrue, + }, + { + Type: conditionTypeError, + Status: metav1.ConditionTrue, + }, + } + + assert.True(me.T(), SetExclusiveConditions(conditions, + []metav1.Condition{ + { + Type: conditionTypeReady, + Status: metav1.ConditionFalse, + }, + { + Type: conditionTypeError, + Status: metav1.ConditionTrue, + }, + }..., + ), "Conditions changes should be detected, but didn't") + + assert.Equal(me.T(), 2, len(*conditions)) + +} + +func TestSetStatusConditionsSuite(t *testing.T) { + suite.Run(t, new(SetStatusConditionsSuite)) +} diff --git a/pkg/kcp/provider/aws/vpcpeering/loadRemoteVpc.go b/pkg/kcp/provider/aws/vpcpeering/loadRemoteVpc.go index 730221e53..d7ea481db 100644 --- a/pkg/kcp/provider/aws/vpcpeering/loadRemoteVpc.go +++ b/pkg/kcp/provider/aws/vpcpeering/loadRemoteVpc.go @@ -30,19 +30,16 @@ func loadRemoteVpc(ctx context.Context, st composed.State) (error, context.Conte if err != nil { logger.Error(err, "Error loading remote AWS VPC Networks") - condition := metav1.Condition{ + if !composed.SetExclusiveConditions(obj.Conditions(), metav1.Condition{ Type: cloudcontrolv1beta1.ConditionTypeError, Status: metav1.ConditionTrue, Reason: cloudcontrolv1beta1.ReasonVpcNotFound, Message: err.Error(), - } - - if !composed.AnyConditionChanged(obj, condition) { + }) { return composed.StopAndForget, nil } return composed.PatchStatus(obj). - SetExclusiveConditions(condition). ErrorLogMessage("Error updating VpcPeering status when loading vpc"). SuccessError(composed.StopAndForget). Run(ctx, st) @@ -55,19 +52,16 @@ func loadRemoteVpc(ctx context.Context, st composed.State) (error, context.Conte ). Info("VPC not found") - condition := metav1.Condition{ + if !composed.SetExclusiveConditions(obj.Conditions(), metav1.Condition{ Type: cloudcontrolv1beta1.ConditionTypeError, Status: metav1.ConditionTrue, Reason: cloudcontrolv1beta1.ReasonVpcNotFound, Message: fmt.Sprintf("AWS VPC ID %s not found", remoteVpcId), - } - - if !composed.AnyConditionChanged(obj, condition) { + }) { return composed.StopAndForget, nil } return composed.PatchStatus(obj). - SetExclusiveConditions(condition). ErrorLogMessage("Error updating VpcPeering status when loading vpc"). SuccessError(composed.StopAndForget). Run(ctx, st) diff --git a/pkg/skr/awsvpcpeering/updateStatus.go b/pkg/skr/awsvpcpeering/updateStatus.go index f17cdc723..80f4353d3 100644 --- a/pkg/skr/awsvpcpeering/updateStatus.go +++ b/pkg/skr/awsvpcpeering/updateStatus.go @@ -15,7 +15,7 @@ func updateStatus(ctx context.Context, st composed.State) (error, context.Contex changed := false - if composed.AnyConditionChanged(state.ObjAsAwsVpcPeering(), *state.KcpVpcPeering.Conditions()...) { + if composed.SetExclusiveConditions(state.ObjAsAwsVpcPeering().Conditions(), *state.KcpVpcPeering.Conditions()...) { changed = true } @@ -26,7 +26,6 @@ func updateStatus(ctx context.Context, st composed.State) (error, context.Contex if changed { state.ObjAsAwsVpcPeering().Status.State = state.KcpVpcPeering.Status.State return composed.UpdateStatus(state.ObjAsAwsVpcPeering()). - SetExclusiveConditions(*state.KcpVpcPeering.Conditions()...). ErrorLogMessage("Error updating SKR AwsVpcPeering status"). SuccessLogMsg("Updated and forgot SKR AwsVpcPeering status"). SuccessErrorNil(). diff --git a/pkg/skr/azurevpcpeering/updateStatus.go b/pkg/skr/azurevpcpeering/updateStatus.go index 19127fcd0..84ba4fde0 100644 --- a/pkg/skr/azurevpcpeering/updateStatus.go +++ b/pkg/skr/azurevpcpeering/updateStatus.go @@ -16,7 +16,7 @@ func updateStatus(ctx context.Context, st composed.State) (error, context.Contex changed := false - if composed.AnyConditionChanged(obj, *state.KcpVpcPeering.Conditions()...) { + if composed.SetExclusiveConditions(obj.Conditions(), *state.KcpVpcPeering.Conditions()...) { changed = true } @@ -27,7 +27,6 @@ func updateStatus(ctx context.Context, st composed.State) (error, context.Contex if changed { obj.Status.State = state.KcpVpcPeering.Status.State return composed.UpdateStatus(obj). - SetExclusiveConditions(*state.KcpVpcPeering.Conditions()...). ErrorLogMessage("Error updating SKR AzureVpcPeering status"). SuccessLogMsg("Updated and forgot SKR AzureVpcPeering status"). SuccessErrorNil(). From eb3b3eb4e04a8d91d3bc41231bcd6dfb9fb39d18 Mon Sep 17 00:00:00 2001 From: Vladimir Andjelkoski Date: Tue, 5 Nov 2024 09:53:17 +0100 Subject: [PATCH 2/2] chore: setExclusiveConditions --- pkg/composed/setExclusiveConditions_test.go | 30 +++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/pkg/composed/setExclusiveConditions_test.go b/pkg/composed/setExclusiveConditions_test.go index 754e509f8..f40a26ac3 100644 --- a/pkg/composed/setExclusiveConditions_test.go +++ b/pkg/composed/setExclusiveConditions_test.go @@ -100,6 +100,36 @@ func (me *SetStatusConditionsSuite) TestConditionChanged() { } +func (me *SetStatusConditionsSuite) TestConditionNotChanged() { + + conditions := &[]metav1.Condition{ + { + Type: conditionTypeReady, + Status: metav1.ConditionTrue, + }, + { + Type: conditionTypeError, + Status: metav1.ConditionTrue, + }, + } + + assert.False(me.T(), SetExclusiveConditions(conditions, + []metav1.Condition{ + { + Type: conditionTypeReady, + Status: metav1.ConditionTrue, + }, + { + Type: conditionTypeError, + Status: metav1.ConditionTrue, + }, + }..., + ), "Conditions changes should not be detected, but did") + + assert.Equal(me.T(), 2, len(*conditions)) + +} + func TestSetStatusConditionsSuite(t *testing.T) { suite.Run(t, new(SetStatusConditionsSuite)) }