diff --git a/api/v1alpha2/conditions.go b/api/v1alpha2/conditions.go index 42b34eb5c..2113f8719 100644 --- a/api/v1alpha2/conditions.go +++ b/api/v1alpha2/conditions.go @@ -22,6 +22,7 @@ func ConditionFromReason(reason ReasonWithMessage) *metav1.Condition { var conditionReasons = map[ConditionReason]conditionMeta{ ConditionReasonReconcileSucceeded: {Type: ConditionTypeReady, Status: metav1.ConditionTrue, Message: ConditionReasonReconcileSucceededMessage}, ConditionReasonReconcileFailed: {Type: ConditionTypeReady, Status: metav1.ConditionFalse, Message: ConditionReasonReconcileFailedMessage}, + ConditionReasonReconcileUnknown: {Type: ConditionTypeReady, Status: metav1.ConditionUnknown, Message: ConditionReasonReconcileUnknownMessage}, ConditionReasonValidationFailed: {Type: ConditionTypeReady, Status: metav1.ConditionFalse, Message: ConditionReasonValidationFailedMessage}, ConditionReasonOlderCRExists: {Type: ConditionTypeReady, Status: metav1.ConditionFalse, Message: ConditionReasonOlderCRExistsMessage}, diff --git a/api/v1alpha2/istio_types.go b/api/v1alpha2/istio_types.go index 040ba24c8..c7ceb29d1 100644 --- a/api/v1alpha2/istio_types.go +++ b/api/v1alpha2/istio_types.go @@ -38,6 +38,8 @@ const ( // general ConditionReasonReconcileSucceeded ConditionReason = "ReconcileSucceeded" ConditionReasonReconcileSucceededMessage = "Reconciliation succeeded" + ConditionReasonReconcileUnknown ConditionReason = "ReconcileUnknown" + ConditionReasonReconcileUnknownMessage = "Module readiness is unknown. Either a reconciliation is progressing, or failed previously. Check status of other conditions" ConditionReasonReconcileFailed ConditionReason = "ReconcileFailed" ConditionReasonReconcileFailedMessage = "Reconciliation failed" ConditionReasonValidationFailed ConditionReason = "ValidationFailed" diff --git a/controllers/istio_controller.go b/controllers/istio_controller.go index a77b3d394..abe5d83b6 100644 --- a/controllers/istio_controller.go +++ b/controllers/istio_controller.go @@ -93,6 +93,8 @@ func (r *IstioReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl return ctrl.Result{}, err } + r.statusHandler.SetCondition(&istioCR, operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonReconcileUnknown)) + err := validation.ValidateAuthorizers(istioCR) if err != nil { return r.terminateReconciliation(ctx, &istioCR, err, operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonValidationFailed)) diff --git a/controllers/istio_controller_test.go b/controllers/istio_controller_test.go index c4395c63a..4b0cfdbf3 100644 --- a/controllers/istio_controller_test.go +++ b/controllers/istio_controller_test.go @@ -330,13 +330,14 @@ var _ = Describe("Istio Controller", func() { Expect(updatedIstioCR.Status.Conditions).ToNot(BeNil()) Expect(*updatedIstioCR.Status.Conditions).To(HaveLen(2)) - Expect((*updatedIstioCR.Status.Conditions)[0].Type).To(Equal(string(operatorv1alpha2.ConditionTypeProxySidecarRestartSucceeded))) - Expect((*updatedIstioCR.Status.Conditions)[0].Reason).To(Equal(string(operatorv1alpha2.ConditionReasonProxySidecarRestartSucceeded))) + Expect((*updatedIstioCR.Status.Conditions)[0].Type).To(Equal(string(operatorv1alpha2.ConditionTypeReady))) + Expect((*updatedIstioCR.Status.Conditions)[0].Reason).To(Equal(string(operatorv1alpha2.ConditionReasonReconcileSucceeded))) Expect((*updatedIstioCR.Status.Conditions)[0].Status).To(Equal(metav1.ConditionTrue)) - Expect((*updatedIstioCR.Status.Conditions)[1].Type).To(Equal(string(operatorv1alpha2.ConditionTypeReady))) - Expect((*updatedIstioCR.Status.Conditions)[1].Reason).To(Equal(string(operatorv1alpha2.ConditionReasonReconcileSucceeded))) + Expect((*updatedIstioCR.Status.Conditions)[1].Type).To(Equal(string(operatorv1alpha2.ConditionTypeProxySidecarRestartSucceeded))) + Expect((*updatedIstioCR.Status.Conditions)[1].Reason).To(Equal(string(operatorv1alpha2.ConditionReasonProxySidecarRestartSucceeded))) Expect((*updatedIstioCR.Status.Conditions)[1].Status).To(Equal(metav1.ConditionTrue)) + }) It("Should return an error when update status to ready failed", func() { @@ -376,6 +377,7 @@ var _ = Describe("Istio Controller", func() { Expect(statusMock.updatedToReadyCalled).Should(BeTrue()) Expect(statusMock.setConditionCalled).Should(BeTrue()) Expect(statusMock.GetConditions()).Should(Equal([]operatorv1alpha2.ReasonWithMessage{ + operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonReconcileUnknown), operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonProxySidecarRestartSucceeded), operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonIngressGatewayReconcileSucceeded), operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonReconcileSucceeded), @@ -545,13 +547,14 @@ var _ = Describe("Istio Controller", func() { Expect(updatedIstioCR.Status.Conditions).ToNot(BeNil()) Expect(*updatedIstioCR.Status.Conditions).To(HaveLen(2)) - Expect((*updatedIstioCR.Status.Conditions)[0].Type).To(Equal(string(operatorv1alpha2.ConditionTypeProxySidecarRestartSucceeded))) - Expect((*updatedIstioCR.Status.Conditions)[0].Reason).To(Equal(string(operatorv1alpha2.ConditionReasonProxySidecarRestartSucceeded))) + Expect((*updatedIstioCR.Status.Conditions)[0].Type).To(Equal(string(operatorv1alpha2.ConditionTypeReady))) + Expect((*updatedIstioCR.Status.Conditions)[0].Reason).To(Equal(string(operatorv1alpha2.ConditionReasonReconcileSucceeded))) Expect((*updatedIstioCR.Status.Conditions)[0].Status).To(Equal(metav1.ConditionTrue)) - Expect((*updatedIstioCR.Status.Conditions)[1].Type).To(Equal(string(operatorv1alpha2.ConditionTypeReady))) - Expect((*updatedIstioCR.Status.Conditions)[1].Reason).To(Equal(string(operatorv1alpha2.ConditionReasonReconcileSucceeded))) + Expect((*updatedIstioCR.Status.Conditions)[1].Type).To(Equal(string(operatorv1alpha2.ConditionTypeProxySidecarRestartSucceeded))) + Expect((*updatedIstioCR.Status.Conditions)[1].Reason).To(Equal(string(operatorv1alpha2.ConditionReasonProxySidecarRestartSucceeded))) Expect((*updatedIstioCR.Status.Conditions)[1].Status).To(Equal(metav1.ConditionTrue)) + }) It("Should set an error status and do not requeue an Istio CR when an older Istio CR is present", func() { @@ -770,6 +773,87 @@ var _ = Describe("Istio Controller", func() { Expect((*updatedIstioCR.Status.Conditions)[0].Reason).To(Equal(string(operatorv1alpha2.ConditionReasonValidationFailed))) Expect((*updatedIstioCR.Status.Conditions)[0].Status).To(Equal(metav1.ConditionFalse)) }) + + It("should update lastTransitionTime of Ready condition when reason changed", func() { + // given + istioCR := &operatorv1alpha2.Istio{ + ObjectMeta: metav1.ObjectMeta{ + Name: istioCrName, + Namespace: testNamespace, + Finalizers: []string{ + "istios.operator.kyma-project.io/istio-installation", + }, + }, + } + + fakeClient := createFakeClient(istioCR) + + By("Mocking Istio install reconciliation to fail") + reconcilerFailingOnIstioInstall := &IstioReconciler{ + Client: fakeClient, + Scheme: getTestScheme(), + istioInstallation: &istioInstallationReconciliationMock{ + err: described_errors.NewDescribedError(errors.New("test error"), "test error description"), + }, + proxySidecars: &proxySidecarsReconciliationMock{}, + istioResources: &istioResourcesReconciliationMock{}, + ingressGateway: &ingressGatewayReconciliationMock{}, + log: logr.Discard(), + statusHandler: status.NewStatusHandler(fakeClient), + reconciliationInterval: testReconciliationInterval, + } + + _, err := reconcilerFailingOnIstioInstall.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{Namespace: testNamespace, Name: istioCrName}}) + + Expect(err).To(HaveOccurred()) + + updatedIstioCR := operatorv1alpha2.Istio{} + Expect(fakeClient.Get(context.Background(), client.ObjectKeyFromObject(istioCR), &updatedIstioCR)).Should(Succeed()) + + Expect(updatedIstioCR.Status.Conditions).ToNot(BeNil()) + Expect(*updatedIstioCR.Status.Conditions).To(HaveLen(1)) + + By("Verifying that Istio CR has Condition Ready with False") + Expect((*updatedIstioCR.Status.Conditions)[0].Type).To(Equal(string(operatorv1alpha2.ConditionTypeReady))) + Expect((*updatedIstioCR.Status.Conditions)[0].Reason).To(Equal(string(operatorv1alpha2.ConditionReasonIstioInstallUninstallFailed))) + Expect((*updatedIstioCR.Status.Conditions)[0].Status).To(Equal(metav1.ConditionFalse)) + + firstNotReadyTransitionTime := (*updatedIstioCR.Status.Conditions)[0].LastTransitionTime + + By("Mocking Istio resources reconciliation to fail") + reconcilerFailingOnIstioResources := &IstioReconciler{ + Client: fakeClient, + Scheme: getTestScheme(), + istioInstallation: &istioInstallationReconciliationMock{}, + proxySidecars: &proxySidecarsReconciliationMock{}, + istioResources: &istioResourcesReconciliationMock{ + err: described_errors.NewDescribedError(errors.New("test error"), "test error description"), + }, + ingressGateway: &ingressGatewayReconciliationMock{}, + log: logr.Discard(), + statusHandler: status.NewStatusHandler(fakeClient), + reconciliationInterval: testReconciliationInterval, + } + + // when + _, err = reconcilerFailingOnIstioResources.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{Namespace: testNamespace, Name: istioCrName}}) + + // then + Expect(err).To(HaveOccurred()) + updatedIstioCR = operatorv1alpha2.Istio{} + Expect(fakeClient.Get(context.Background(), client.ObjectKeyFromObject(istioCR), &updatedIstioCR)).Should(Succeed()) + + Expect(updatedIstioCR.Status.Conditions).ToNot(BeNil()) + Expect(*updatedIstioCR.Status.Conditions).To(HaveLen(1)) + + By("Verifying that the condition lastTransitionTime is also updated when only the reason has changed") + Expect((*updatedIstioCR.Status.Conditions)[0].Type).To(Equal(string(operatorv1alpha2.ConditionTypeReady))) + Expect((*updatedIstioCR.Status.Conditions)[0].Reason).To(Equal(string(operatorv1alpha2.ConditionReasonCRsReconcileFailed))) + Expect((*updatedIstioCR.Status.Conditions)[0].Status).To(Equal(metav1.ConditionFalse)) + + secondNotReadyTransitionTime := (*updatedIstioCR.Status.Conditions)[0].LastTransitionTime + Expect(secondNotReadyTransitionTime.Compare(firstNotReadyTransitionTime.Time) >= 0).To(BeTrue()) + }) }) }) @@ -785,6 +869,7 @@ func (i *ingressGatewayReconciliationMock) Reconcile(_ context.Context) describe } type istioResourcesReconciliationMock struct { + err described_errors.DescribedError } func (i *istioResourcesReconciliationMock) AddReconcileResource(_ istio_resources.Resource) istio_resources.ResourcesReconciliation { @@ -792,7 +877,7 @@ func (i *istioResourcesReconciliationMock) AddReconcileResource(_ istio_resource } func (i *istioResourcesReconciliationMock) Reconcile(_ context.Context, _ operatorv1alpha2.Istio) described_errors.DescribedError { - return nil + return i.err } type shouldFailClient struct { diff --git a/tests/integration/features/istio/production/main-suite/istio_installation.feature b/tests/integration/features/istio/production/main-suite/istio_installation.feature index dc72f7ed2..2e7e92532 100644 --- a/tests/integration/features/istio/production/main-suite/istio_installation.feature +++ b/tests/integration/features/istio/production/main-suite/istio_installation.feature @@ -9,7 +9,8 @@ Feature: Installing and uninstalling Istio module Scenario: Installation of Istio module with default values Given Istio CR "istio-sample" from "istio_cr_template" is applied in namespace "kyma-system" - Then Istio CR "istio-sample" in namespace "kyma-system" has status "Ready" + Then Istio CR "istio-sample" in namespace "kyma-system" has condition with reason "ReconcileUnknown" of type "Ready" and status "Unknown" + And Istio CR "istio-sample" in namespace "kyma-system" has status "Ready" And Istio CR "istio-sample" in namespace "kyma-system" has condition with reason "ReconcileSucceeded" of type "Ready" and status "True" And "proxy" has "requests" set to cpu - "10m" and memory - "192Mi" And "proxy" has "limits" set to cpu - "1000m" and memory - "1024Mi" diff --git a/tests/integration/steps/istio_cr.go b/tests/integration/steps/istio_cr.go index 30fa72843..afd6145a7 100644 --- a/tests/integration/steps/istio_cr.go +++ b/tests/integration/steps/istio_cr.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "github.com/pkg/errors" "os" "path" "text/template" @@ -70,6 +71,9 @@ func IstioCRInNamespaceHasStatusCondition(ctx context.Context, name, namespace, if err != nil { return err } + if cr.Status.Conditions == nil { + return errors.New("No condition was present on Istio CR") + } for _, condition := range *cr.Status.Conditions { if condition.Reason == reason && condition.Type == conditionType && string(condition.Status) == status { return nil