diff --git a/controllers/istio_controller.go b/controllers/istio_controller.go index 3e5a41ec79..56e8b1fe91 100644 --- a/controllers/istio_controller.go +++ b/controllers/istio_controller.go @@ -182,6 +182,19 @@ func (r *IstioReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl return ctrl.Result{}, nil } + updateIstioTagErr := r.updateIstioTag(ctx, client.ObjectKeyFromObject(&istioCR), istioImageVersion.Tag()) + if updateIstioTagErr != nil { + r.log.Error(err, "Could not update Istio tag in annotation") + result, requeueReconcileErr := r.requeueReconciliation(ctx, + &istioCR, + describederrors.NewDescribedError(updateIstioTagErr, "Could not update Istio tag in annotation"), + operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonIstioInstallUninstallFailed), + reconciliationRequeueTimeError) + if requeueReconcileErr != nil { + return result, err + } + } + resourcesErr := r.istioResources.Reconcile(ctx, istioCR) if resourcesErr != nil { return r.requeueReconciliation(ctx, &istioCR, resourcesErr, @@ -388,6 +401,20 @@ func (r *IstioReconciler) updateLastAppliedConfiguration(ctx context.Context, ob }) } +func (r *IstioReconciler) updateIstioTag(ctx context.Context, objectKey types.NamespacedName, istioTag string) error { + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + lacIstioCR := operatorv1alpha2.Istio{} + if err := r.Get(ctx, objectKey, &lacIstioCR); err != nil { + return err + } + err := configuration.UpdateIstioTag(&lacIstioCR, istioTag) + if err != nil { + return err + } + return r.Update(ctx, &lacIstioCR) + }) +} + func (r *IstioReconciler) setConditionForError(istioCR *operatorv1alpha2.Istio, reason operatorv1alpha2.ReasonWithMessage) { if !operatorv1alpha2.IsReadyTypeCondition(reason) { r.statusHandler.SetCondition(istioCR, operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonReconcileFailed)) diff --git a/controllers/istio_controller_test.go b/controllers/istio_controller_test.go index 90ae9e8049..6b1cf4be07 100644 --- a/controllers/istio_controller_test.go +++ b/controllers/istio_controller_test.go @@ -12,6 +12,7 @@ import ( "github.com/kyma-project/istio/operator/internal/istiooperator" "github.com/kyma-project/istio/operator/internal/restarter" + "github.com/kyma-project/istio/operator/pkg/labels" "github.com/kyma-project/istio/operator/internal/reconciliations/istioresources" "github.com/kyma-project/istio/operator/internal/status" @@ -388,6 +389,7 @@ var _ = Describe("Istio Controller", func() { result, err := sut.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{Namespace: testNamespace, Name: istioCrName}}) // then + Expect(err).Should(HaveOccurred()) Expect(err.Error()).To(Equal("Update to ready error")) Expect(result).Should(Equal(reconcile.Result{})) Expect(statusMock.updatedToReadyCalled).Should(BeTrue()) @@ -1109,6 +1111,45 @@ var _ = Describe("Istio Controller", func() { Expect((*updatedIstioCR.Status.Conditions)[0].Status).To(Equal(metav1.ConditionFalse)) }) }) + Context("LastAppliedConfiguration", func() { + It("should update LastAppliedConfiguration with istioTag version even if restarter is blocked", func() { + //given + istioCR := &operatorv1alpha2.Istio{ + ObjectMeta: metav1.ObjectMeta{ + Name: istioCrName, + Namespace: testNamespace, + UID: "1", + CreationTimestamp: metav1.Unix(1494505756, 0), + Finalizers: []string{ + "istios.operator.kyma-project.io/istio-installation", + }, + }, + } + + fakeClient := createFakeClient(istioCR) + proxySidecarsRestarter := &restarterMock{restarted: false, requeue: true, err: describederrors.NewDescribedError(errors.New("test described error"), "test error description")} + sut := &IstioReconciler{ + Client: fakeClient, + Scheme: getTestScheme(), + istioInstallation: &istioInstallationReconciliationMock{}, + istioResources: &istioResourcesReconciliationMock{}, + userResources: &UserResourcesMock{}, + restarters: []restarter.Restarter{proxySidecarsRestarter}, + log: logr.Discard(), + statusHandler: status.NewStatusHandler(fakeClient), + reconciliationInterval: testReconciliationInterval, + } + + //when + _, err := sut.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{Namespace: testNamespace, Name: istioCrName}}) + + //then + _ = fakeClient.Get(context.Background(), client.ObjectKeyFromObject(istioCR), istioCR) + + Expect(err).To(HaveOccurred()) + Expect(istioCR.Annotations[labels.LastAppliedConfiguration]).To(ContainSubstring(`"IstioTag":"1.16.0-distroless"`)) + }) + }) }) }) diff --git a/docs/release-notes/1.23.0.md b/docs/release-notes/1.23.0.md index f5bb50376f..ac84e79ad7 100644 --- a/docs/release-notes/1.23.0.md +++ b/docs/release-notes/1.23.0.md @@ -2,4 +2,6 @@ - We've fixed a bug that caused restarts during each reconciliation of an Istio-injected Pod with custom proxy resource settings defined using annotations. -See [#1649](https://github.com/kyma-project/istio/pull/1649). \ No newline at end of file +See [#1649](https://github.com/kyma-project/istio/pull/1649). + +- We've fixed a bug where the `IstioVersion` annotation was not updated in the Istio CR when reconciliation failed to reach the last step, which could lead to blocked Istio upgrades. See [#1644](https://github.com/kyma-project/istio/issues/1644). \ No newline at end of file diff --git a/internal/reconciliations/istio/configuration/configuration.go b/internal/reconciliations/istio/configuration/configuration.go index 3851a66434..39d7eedc4f 100644 --- a/internal/reconciliations/istio/configuration/configuration.go +++ b/internal/reconciliations/istio/configuration/configuration.go @@ -93,3 +93,26 @@ func CheckIstioVersionUpdate(currentIstioVersionString, targetIstioVersionString func amongOneMinor(current, target semver.Version) bool { return current.Minor == target.Minor || current.Minor-target.Minor == -1 || current.Minor-target.Minor == 1 } + +func UpdateIstioTag(istioCR *v1alpha2.Istio, istioTag string) error { + if len(istioCR.Annotations) == 0 { + istioCR.Annotations = map[string]string{} + } + appliedConfig := AppliedConfig{} + if lastAppliedConfiguration, ok := istioCR.Annotations[labels.LastAppliedConfiguration]; ok { + err := json.Unmarshal([]byte(lastAppliedConfiguration), &appliedConfig) + if err != nil { + return err + } + } + + appliedConfig.IstioTag = istioTag + + config, err := json.Marshal(appliedConfig) + if err != nil { + return err + } + + istioCR.Annotations[labels.LastAppliedConfiguration] = string(config) + return nil +} diff --git a/internal/reconciliations/istio/configuration/configuration_test.go b/internal/reconciliations/istio/configuration/configuration_test.go index 126e008630..743ec7de7e 100644 --- a/internal/reconciliations/istio/configuration/configuration_test.go +++ b/internal/reconciliations/istio/configuration/configuration_test.go @@ -10,8 +10,9 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "github.com/kyma-project/istio/operator/internal/tests" "github.com/onsi/ginkgo/v2/types" + + "github.com/kyma-project/istio/operator/internal/tests" ) const ( @@ -85,4 +86,32 @@ var _ = Describe("Istio Configuration", func() { Expect(err).ShouldNot(HaveOccurred()) }) }) + Context("UpdateIstioTagVersion", func() { + It("should create Istio tag version when it is first installation", func() { + // given + istioCR := operatorv1alpha2.Istio{Spec: operatorv1alpha2.IstioSpec{}} + installedIstioTagVersion := "1.16.1-distroless" + // when + err := configuration.UpdateIstioTag(&istioCR, installedIstioTagVersion) + // then + Expect(err).ShouldNot(HaveOccurred()) + Expect(istioCR.Annotations).To(Not(BeEmpty())) + Expect(istioCR.Annotations[lastAppliedConfiguration]).To(Equal(`{"config":{"telemetry":{"metrics":{}}},"IstioTag":"1.16.1-distroless"}`)) + }) + + It("should update Istio tag version for existing upgraded Istio", func() { + // given + istioCR := operatorv1alpha2.Istio{Spec: operatorv1alpha2.IstioSpec{}} + istioCR.Annotations = map[string]string{} + istioCR.Annotations[lastAppliedConfiguration] = `{"config":{"numTrustedProxies":1,"telemetry":{"metrics":{}}},"IstioTag":"1.15.0-distroless"}` + installedIstioTagVersion := "1.16.1-distroless" + // when + err := configuration.UpdateIstioTag(&istioCR, installedIstioTagVersion) + // then + Expect(err).ShouldNot(HaveOccurred()) + Expect(istioCR.Annotations).To(Not(BeEmpty())) + Expect(istioCR.Annotations[lastAppliedConfiguration]).To(Equal(`{"config":{"numTrustedProxies":1,"telemetry":{"metrics":{}}},"IstioTag":"1.16.1-distroless"}`)) + }) + }) + })