From 626bfd20f2e5df5abc23b36517d6222e4f64da9e Mon Sep 17 00:00:00 2001 From: Marco Bebway Date: Mon, 25 Mar 2024 12:41:16 +0100 Subject: [PATCH] fix: update the Eventing CR status accordingly after switching from an empty backend (#538) --- api/operator/v1alpha1/eventing_types.go | 12 +- api/operator/v1alpha1/eventing_types_test.go | 50 ++++++ api/operator/v1alpha1/status.go | 6 + api/operator/v1alpha1/status_test.go | 166 ++++++++++++++++++ .../operator/eventing/controller.go | 9 +- .../controller/integration_test.go | 103 +++++++++++ test/matchers/matchers.go | 9 + test/utils/options.go | 16 +- test/utils/utils.go | 12 +- 9 files changed, 370 insertions(+), 13 deletions(-) diff --git a/api/operator/v1alpha1/eventing_types.go b/api/operator/v1alpha1/eventing_types.go index aa454928b..f05654bf8 100644 --- a/api/operator/v1alpha1/eventing_types.go +++ b/api/operator/v1alpha1/eventing_types.go @@ -18,6 +18,8 @@ limitations under the License. package v1alpha1 import ( + "strings" + kcorev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -39,12 +41,10 @@ const ( ConditionSubscriptionManagerReady ConditionType = "SubscriptionManagerReady" ConditionDeleted ConditionType = "Deleted" - // common reasons. ConditionReasonProcessing ConditionReason = "Processing" ConditionReasonDeleted ConditionReason = "Deleted" ConditionReasonStopped ConditionReason = "Stopped" - // publisher proxy reasons. ConditionReasonDeployed ConditionReason = "Deployed" ConditionReasonDeployedFailed ConditionReason = "DeployFailed" ConditionReasonDeploymentStatusSyncFailed ConditionReason = "DeploymentStatusSyncFailed" @@ -55,8 +55,8 @@ const ( ConditionReasonWebhookFailed ConditionReason = "WebhookFailed" ConditionReasonWebhookReady ConditionReason = "Ready" ConditionReasonDeletionError ConditionReason = "DeletionError" + ConditionReasonEventMeshConfigAvailable ConditionReason = "EventMeshConfigAvailable" - // message for conditions. ConditionPublisherProxyReadyMessage = "Publisher proxy is deployed" ConditionPublisherProxyDeletedMessage = "Publisher proxy is deleted" ConditionNATSAvailableMessage = "NATS is available" @@ -65,8 +65,8 @@ const ( ConditionSubscriptionManagerReadyMessage = "Subscription manager is ready" ConditionSubscriptionManagerStoppedMessage = "Subscription manager is stopped" ConditionBackendNotSpecifiedMessage = "Backend config is not provided. Please specify a backend." + ConditionEventMeshConfigAvailableMessage = "EventMesh config is available" - // subscription manager reasons. ConditionReasonEventMeshSubManagerReady ConditionReason = "EventMeshSubscriptionManagerReady" ConditionReasonEventMeshSubManagerFailed ConditionReason = "EventMeshSubscriptionManagerFailed" ConditionReasonEventMeshSubManagerStopFailed ConditionReason = "EventMeshSubscriptionManagerStopFailed" @@ -243,3 +243,7 @@ func (e *Eventing) IsPreviousBackendEmpty() bool { func (e *Eventing) IsSpecBackendTypeChanged() bool { return e.Status.ActiveBackend != e.Spec.Backend.Type } + +func (es EventingSpec) HasEmptyBackend() bool { + return es.Backend == nil || len(strings.TrimSpace(string(es.Backend.Type))) == 0 +} diff --git a/api/operator/v1alpha1/eventing_types_test.go b/api/operator/v1alpha1/eventing_types_test.go index 6f0dcde30..c69c78ba6 100644 --- a/api/operator/v1alpha1/eventing_types_test.go +++ b/api/operator/v1alpha1/eventing_types_test.go @@ -1,6 +1,7 @@ package v1alpha1 import ( + "github.com/stretchr/testify/assert" "testing" "github.com/stretchr/testify/require" @@ -109,3 +110,52 @@ func Test_getSupportedConditionsTypes(t *testing.T) { got := getSupportedConditionsTypes() require.Equal(t, want, got) } + +func TestHasEmptyBackend(t *testing.T) { + tests := []struct { + name string + givenEventingSpec EventingSpec + wantHasEmptyBackend bool + }{ + { + name: "with nil backend", + givenEventingSpec: EventingSpec{ + Backend: nil, + }, + wantHasEmptyBackend: true, + }, + { + name: "with empty backend type", + givenEventingSpec: EventingSpec{ + Backend: &Backend{ + Type: "", + }, + }, + wantHasEmptyBackend: true, + }, + { + name: "with non-empty backend type all whitespaces", + givenEventingSpec: EventingSpec{ + Backend: &Backend{ + Type: " ", + }, + }, + wantHasEmptyBackend: true, + }, + { + name: "with non-empty backend type", + givenEventingSpec: EventingSpec{ + Backend: &Backend{ + Type: "any", + }, + }, + wantHasEmptyBackend: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.givenEventingSpec.HasEmptyBackend() + assert.Equal(t, tt.wantHasEmptyBackend, got) + }) + } +} diff --git a/api/operator/v1alpha1/status.go b/api/operator/v1alpha1/status.go index e63e0667f..f1d74d18c 100644 --- a/api/operator/v1alpha1/status.go +++ b/api/operator/v1alpha1/status.go @@ -93,6 +93,12 @@ func (es *EventingStatus) SetNATSAvailableConditionToTrue() { es.UpdateConditionBackendAvailable(kmetav1.ConditionTrue, ConditionReasonNATSAvailable, ConditionNATSAvailableMessage) } +func (es *EventingStatus) SetEventMeshAvailableConditionToTrue() { + es.UpdateConditionBackendAvailable( + kmetav1.ConditionTrue, ConditionReasonEventMeshConfigAvailable, ConditionEventMeshConfigAvailableMessage, + ) +} + func (es *EventingStatus) SetSubscriptionManagerReadyConditionToFalse(reason ConditionReason, message string) { es.UpdateConditionSubscriptionManagerReady(kmetav1.ConditionFalse, reason, message) diff --git a/api/operator/v1alpha1/status_test.go b/api/operator/v1alpha1/status_test.go index 5efe4a90d..bf240ffe1 100644 --- a/api/operator/v1alpha1/status_test.go +++ b/api/operator/v1alpha1/status_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -285,3 +286,168 @@ func TestRemoveUnsupportedConditions(t *testing.T) { }) } } + +func TestSetEventMeshAvailableConditionToTrue(t *testing.T) { + var ( + anyCondition0 = kmetav1.Condition{ + Type: "any-type-0", + Status: kmetav1.ConditionStatus("any-status-0"), + Reason: "any-reason-0", + Message: "any-message-0", + } + + anyCondition1 = kmetav1.Condition{ + Type: "any-type-1", + Status: kmetav1.ConditionStatus("any-status-1"), + Reason: "any-reason-1", + Message: "any-message-1", + } + + anyCondition2 = kmetav1.Condition{ + Type: "any-type-2", + Status: kmetav1.ConditionStatus("any-status-2"), + Reason: "any-reason-2", + Message: "any-message-2", + } + + backendAvailableConditionFalse = kmetav1.Condition{ + Type: string(ConditionBackendAvailable), + Status: kmetav1.ConditionFalse, + Reason: string(ConditionReasonBackendNotSpecified), + Message: ConditionBackendNotSpecifiedMessage, + } + + backendAvailableConditionTrue = kmetav1.Condition{ + Type: string(ConditionBackendAvailable), + Status: kmetav1.ConditionTrue, + Reason: string(ConditionReasonEventMeshConfigAvailable), + Message: ConditionEventMeshConfigAvailableMessage, + } + ) + + tests := []struct { + name string + givenEventingStatus EventingStatus + wantEventingStatus EventingStatus + }{ + // add new condition + { + name: "should add a new condition in case of nil conditions", + givenEventingStatus: EventingStatus{ + Conditions: nil, + }, + wantEventingStatus: EventingStatus{ + Conditions: []kmetav1.Condition{ + backendAvailableConditionTrue, + }, + }, + }, + { + name: "should add a new condition in case of empty conditions", + givenEventingStatus: EventingStatus{ + Conditions: []kmetav1.Condition{}, + }, + wantEventingStatus: EventingStatus{ + Conditions: []kmetav1.Condition{ + backendAvailableConditionTrue, + }, + }, + }, + { + name: "should add a new condition and preserve existing ones", + givenEventingStatus: EventingStatus{ + Conditions: []kmetav1.Condition{ + anyCondition0, + anyCondition1, + anyCondition2, + }, + }, + wantEventingStatus: EventingStatus{ + Conditions: []kmetav1.Condition{ + anyCondition0, + anyCondition1, + anyCondition2, + backendAvailableConditionTrue, + }, + }, + }, + // update existing condition + { + name: "should update existing condition", + givenEventingStatus: EventingStatus{ + Conditions: []kmetav1.Condition{ + backendAvailableConditionFalse, + }, + }, + wantEventingStatus: EventingStatus{ + Conditions: []kmetav1.Condition{ + backendAvailableConditionTrue, + }, + }, + }, + { + name: "should update condition and preserve existing ones", + givenEventingStatus: EventingStatus{ + Conditions: []kmetav1.Condition{ + anyCondition0, + anyCondition1, + backendAvailableConditionTrue, + anyCondition2, + }, + }, + wantEventingStatus: EventingStatus{ + Conditions: []kmetav1.Condition{ + anyCondition0, + anyCondition1, + backendAvailableConditionTrue, + anyCondition2, + }, + }, + }, + { + name: "should update condition from false to true and preserve existing ones", + givenEventingStatus: EventingStatus{ + Conditions: []kmetav1.Condition{ + anyCondition0, + backendAvailableConditionFalse, + anyCondition1, + anyCondition2, + }, + }, + wantEventingStatus: EventingStatus{ + Conditions: []kmetav1.Condition{ + anyCondition0, + backendAvailableConditionTrue, + anyCondition1, + anyCondition2, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.givenEventingStatus.SetEventMeshAvailableConditionToTrue() + assertConditionsEqual(t, tt.wantEventingStatus.Conditions, tt.givenEventingStatus.Conditions) + }) + } +} + +// assertConditionsEqual ensures conditions are equal. +// Note: This function takes into consideration the order of conditions while doing the equality check. +func assertConditionsEqual(t *testing.T, expected, actual []kmetav1.Condition) { + t.Helper() + + assert.Equal(t, len(expected), len(actual)) + for i := 0; i < len(expected); i++ { + assertConditionEqual(t, expected[i], actual[i]) + } +} + +func assertConditionEqual(t *testing.T, expected, actual kmetav1.Condition) { + t.Helper() + + assert.Equal(t, expected.Type, actual.Type) + assert.Equal(t, expected.Status, actual.Status) + assert.Equal(t, expected.Reason, actual.Reason) + assert.Equal(t, expected.Message, actual.Message) +} diff --git a/internal/controller/operator/eventing/controller.go b/internal/controller/operator/eventing/controller.go index 316d4befd..6d514a60d 100644 --- a/internal/controller/operator/eventing/controller.go +++ b/internal/controller/operator/eventing/controller.go @@ -464,8 +464,10 @@ func (r *Reconciler) handleEventingReconcile(ctx context.Context, // set state processing if not set yet r.InitStateProcessing(eventing) - if eventing.Spec.Backend == nil { - return kctrl.Result{Requeue: true}, r.syncStatusForEmptyBackend(ctx, + + // Handle empty backend. + if eventing.Spec.HasEmptyBackend() { + return kctrl.Result{Requeue: false}, r.syncStatusForEmptyBackend(ctx, operatorv1alpha1.ConditionBackendNotSpecifiedMessage, eventing, log) } @@ -677,6 +679,9 @@ func (r *Reconciler) reconcileEventMeshBackend(ctx context.Context, eventing *op } return kctrl.Result{}, r.syncStatusWithSubscriptionManagerErr(ctx, eventing, err, log) } + + // Set the Eventing CR status accordingly. + eventing.Status.SetEventMeshAvailableConditionToTrue() eventing.Status.SetSubscriptionManagerReadyConditionToTrue() deployment, err := r.handlePublisherProxy(ctx, eventing, eventing.Spec.Backend.Type) diff --git a/internal/controller/operator/eventing/integrationtests/controller/integration_test.go b/internal/controller/operator/eventing/integrationtests/controller/integration_test.go index c59e07bdd..cd52182d3 100644 --- a/internal/controller/operator/eventing/integrationtests/controller/integration_test.go +++ b/internal/controller/operator/eventing/integrationtests/controller/integration_test.go @@ -721,6 +721,109 @@ func Test_CreateEventingCR_EventMesh(t *testing.T) { } } +func TestUpdateEventingCRFromEmptyToNonEmptyBackend(t *testing.T) { + tests := []struct { + name string + givenBackendTypeToUse operatorv1alpha1.BackendType + wantMatchesBeforeUpdate gomegatypes.GomegaMatcher + wantMatchesAfterUpdate gomegatypes.GomegaMatcher + }{ + { + name: "update Eventing CR from empty backend to NATS", + givenBackendTypeToUse: operatorv1alpha1.NatsBackendType, + wantMatchesBeforeUpdate: gomega.And( + matchers.HaveBackendNotAvailableConditionWith( + operatorv1alpha1.ConditionBackendNotSpecifiedMessage, + operatorv1alpha1.ConditionReasonBackendNotSpecified, + ), + ), + wantMatchesAfterUpdate: gomega.And( + matchers.HaveFinalizer(), + matchers.HaveBackendAvailableConditionWith( + operatorv1alpha1.ConditionNATSAvailableMessage, + operatorv1alpha1.ConditionReasonNATSAvailable, + ), + ), + }, + { + name: "update Eventing CR from empty backend to EventMesh", + givenBackendTypeToUse: operatorv1alpha1.EventMeshBackendType, + wantMatchesBeforeUpdate: gomega.And( + matchers.HaveBackendNotAvailableConditionWith( + operatorv1alpha1.ConditionBackendNotSpecifiedMessage, + operatorv1alpha1.ConditionReasonBackendNotSpecified, + ), + ), + wantMatchesAfterUpdate: gomega.And( + matchers.HaveFinalizer(), + matchers.HaveBackendAvailableConditionWith( + operatorv1alpha1.ConditionEventMeshConfigAvailableMessage, + operatorv1alpha1.ConditionReasonEventMeshConfigAvailable, + ), + ), + }, + } + + for _, test := range tests { + test := test + + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := gomega.NewWithT(t) + + ///// + // Create the Eventing CR with an empty backend + ///// + + eventingCR := utils.NewEventingCR(utils.WithEmptyBackend()) + namespace := eventingCR.Namespace + testEnvironment.EnsureNamespaceCreation(t, namespace) + testEnvironment.EnsureK8sResourceCreated(t, eventingCR) + defer func() { + testEnvironment.EnsureEventingResourceDeletion(t, eventingCR.Name, namespace) + testEnvironment.EnsureNamespaceDeleted(t, namespace) + }() + + testEnvironment.GetEventingAssert(g, eventingCR).Should(test.wantMatchesBeforeUpdate) + + ///// + // Update the Eventing CR with a non-empty backend + ///// + + eventingCR, err := testEnvironment.GetEventingFromK8s(eventingCR.Name, namespace) + g.Expect(err).ShouldNot(gomega.HaveOccurred()) + eventingCR = eventingCR.DeepCopy() + + switch test.givenBackendTypeToUse { + case operatorv1alpha1.NatsBackendType: + { + natsCR := natstestutils.NewNATSCR( + natstestutils.WithNATSCRNamespace(namespace), + natstestutils.WithNATSCRDefaults(), + natstestutils.WithNATSStateReady(), + ) + testEnvironment.EnsureK8sResourceCreated(t, natsCR) + defer func() { testEnvironment.EnsureK8sResourceDeleted(t, natsCR) }() + + g.Expect(utils.WithNATSBackend()(eventingCR)).ShouldNot(gomega.HaveOccurred()) + testEnvironment.EnsureK8sResourceUpdated(t, eventingCR) + } + case operatorv1alpha1.EventMeshBackendType: + { + g.Expect(utils.WithEventingDomain(utils.Domain)(eventingCR)).ShouldNot(gomega.HaveOccurred()) + g.Expect(utils.WithEventMeshBackend("test-eventmesh-secret")(eventingCR)).ShouldNot(gomega.HaveOccurred()) + + testEnvironment.EnsureEventMeshSecretCreated(t, eventingCR) + testEnvironment.EnsureOAuthSecretCreated(t, eventingCR) + testEnvironment.EnsureK8sResourceUpdated(t, eventingCR) + } + } + + testEnvironment.GetEventingAssert(g, eventingCR).Should(test.wantMatchesAfterUpdate) + }) + } +} + func Test_DeleteEventingCR(t *testing.T) { t.Parallel() testCases := []struct { diff --git a/test/matchers/matchers.go b/test/matchers/matchers.go index e7cd7fed0..a3c761f3e 100644 --- a/test/matchers/matchers.go +++ b/test/matchers/matchers.go @@ -106,6 +106,15 @@ func HaveBackendNotAvailableConditionWith(message string, reason v1alpha1.Condit }) } +func HaveBackendAvailableConditionWith(message string, reason v1alpha1.ConditionReason) gomegatypes.GomegaMatcher { + return HaveCondition(kmetav1.Condition{ + Type: string(v1alpha1.ConditionBackendAvailable), + Status: kmetav1.ConditionTrue, + Reason: string(reason), + Message: message, + }) +} + func HaveNATSNotAvailableConditionWith(message string) gomegatypes.GomegaMatcher { return HaveCondition(kmetav1.Condition{ Type: string(v1alpha1.ConditionBackendAvailable), diff --git a/test/utils/options.go b/test/utils/options.go index be8ae7fb7..5f3cb7d81 100644 --- a/test/utils/options.go +++ b/test/utils/options.go @@ -116,12 +116,18 @@ func WithEventingLogLevel(logLevel string) EventingOption { func WithEventMeshBackend(eventMeshSecretName string) EventingOption { return func(e *v1alpha1.Eventing) error { - e.Spec.Backend = &v1alpha1.Backend{ - Type: v1alpha1.EventMeshBackendType, - Config: v1alpha1.BackendConfig{ - EventMeshSecret: e.Namespace + "/" + eventMeshSecretName, - }, + if e.Spec.Backend == nil { + e.Spec.Backend = &v1alpha1.Backend{} } + e.Spec.Backend.Type = v1alpha1.EventMeshBackendType + e.Spec.Backend.Config.EventMeshSecret = e.Namespace + "/" + eventMeshSecretName + return nil + } +} + +func WithEmptyBackend() EventingOption { + return func(e *v1alpha1.Eventing) error { + e.Spec.Backend = &v1alpha1.Backend{} return nil } } diff --git a/test/utils/utils.go b/test/utils/utils.go index c47c8879d..cb2aa60de 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -109,9 +109,17 @@ func NewAPIRuleCRD() *kapiextensionsv1.CustomResourceDefinition { return result } +func GetRandomName() string { + return fmt.Sprintf(NameFormat, GetRandString(randomNameLen)) +} + +func GetRandomNamespaceName() string { + return fmt.Sprintf(NamespaceFormat, GetRandString(randomNameLen)) +} + func NewEventingCR(opts ...EventingOption) *v1alpha1.Eventing { - name := fmt.Sprintf(NameFormat, GetRandString(randomNameLen)) - namespace := fmt.Sprintf(NamespaceFormat, GetRandString(randomNameLen)) + name := GetRandomName() + namespace := GetRandomNamespaceName() eventing := &v1alpha1.Eventing{ TypeMeta: kmetav1.TypeMeta{