Proxy reset with pagination and reconciliation requeue

api/v1alpha2/conditions.go

@@ -23,6 +23,7 @@ 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},
+	ConditionReasonReconcileRequeued:  {Type: ConditionTypeReady, Status: metav1.ConditionFalse, Message: ConditionReasonReconcileRequeuedMessage},
 	ConditionReasonValidationFailed:   {Type: ConditionTypeReady, Status: metav1.ConditionFalse, Message: ConditionReasonValidationFailedMessage},
 	ConditionReasonOlderCRExists:      {Type: ConditionTypeReady, Status: metav1.ConditionFalse, Message: ConditionReasonOlderCRExistsMessage},
 
@@ -37,9 +38,10 @@ var conditionReasons = map[ConditionReason]conditionMeta{
 	ConditionReasonCRsReconcileSucceeded: {Type: ConditionTypeReady, Status: metav1.ConditionFalse, Message: ConditionReasonCRsReconcileSucceededMessage},
 	ConditionReasonCRsReconcileFailed:    {Type: ConditionTypeReady, Status: metav1.ConditionFalse, Message: ConditionReasonCRsReconcileFailedMessage},
 
-	ConditionReasonProxySidecarRestartSucceeded:      {Type: ConditionTypeProxySidecarRestartSucceeded, Status: metav1.ConditionTrue, Message: ConditionReasonProxySidecarRestartSucceededMessage},
-	ConditionReasonProxySidecarRestartFailed:         {Type: ConditionTypeProxySidecarRestartSucceeded, Status: metav1.ConditionFalse, Message: ConditionReasonProxySidecarRestartFailedMessage},
-	ConditionReasonProxySidecarManualRestartRequired: {Type: ConditionTypeProxySidecarRestartSucceeded, Status: metav1.ConditionFalse, Message: ConditionReasonProxySidecarManualRestartRequiredMessage},
+	ConditionReasonProxySidecarRestartSucceeded:          {Type: ConditionTypeProxySidecarRestartSucceeded, Status: metav1.ConditionTrue, Message: ConditionReasonProxySidecarRestartSucceededMessage},
+	ConditionReasonProxySidecarRestartFailed:             {Type: ConditionTypeProxySidecarRestartSucceeded, Status: metav1.ConditionFalse, Message: ConditionReasonProxySidecarRestartFailedMessage},
+	ConditionReasonProxySidecarRestartPartiallySucceeded: {Type: ConditionTypeProxySidecarRestartSucceeded, Status: metav1.ConditionFalse, Message: ConditionReasonProxySidecarRestartPartiallySucceededMessage},
+	ConditionReasonProxySidecarManualRestartRequired:     {Type: ConditionTypeProxySidecarRestartSucceeded, Status: metav1.ConditionFalse, Message: ConditionReasonProxySidecarManualRestartRequiredMessage},
 
 	ConditionReasonIngressGatewayRestartSucceeded: {Type: ConditionTypeReady, Status: metav1.ConditionFalse, Message: ConditionReasonIngressGatewayRestartSucceededMessage},
 	ConditionReasonIngressGatewayRestartFailed:    {Type: ConditionTypeReady, Status: metav1.ConditionFalse, Message: ConditionReasonIngressGatewayRestartFailedMessage},

api/v1alpha2/istio_types.go

@@ -40,6 +40,8 @@ const (
 	ConditionReasonReconcileSucceededMessage                 = "Reconciliation succeeded"
 	ConditionReasonReconcileUnknown          ConditionReason = "ReconcileUnknown"
 	ConditionReasonReconcileUnknownMessage                   = "Module readiness is unknown. Either a reconciliation is progressing, or failed previously. Check status of other conditions"
+	ConditionReasonReconcileRequeued         ConditionReason = "ReconcileRequeued"
+	ConditionReasonReconcileRequeuedMessage                  = "Proxy reset is still ongoing. Reconciliation requeued"
 	ConditionReasonReconcileFailed           ConditionReason = "ReconcileFailed"
 	ConditionReasonReconcileFailedMessage                    = "Reconciliation failed"
 	ConditionReasonValidationFailed          ConditionReason = "ValidationFailed"
@@ -70,12 +72,14 @@ const (
 	ConditionReasonCRsReconcileFailedMessage                    = "Custom resources reconciliation failed"
 
 	// proxy reset
-	ConditionReasonProxySidecarRestartSucceeded             ConditionReason = "ProxySidecarRestartSucceeded"
-	ConditionReasonProxySidecarRestartSucceededMessage                      = "Proxy sidecar restart succeeded"
-	ConditionReasonProxySidecarRestartFailed                ConditionReason = "ProxySidecarRestartFailed"
-	ConditionReasonProxySidecarRestartFailedMessage                         = "Proxy sidecar restart failed"
-	ConditionReasonProxySidecarManualRestartRequired        ConditionReason = "ProxySidecarManualRestartRequired"
-	ConditionReasonProxySidecarManualRestartRequiredMessage                 = "Proxy sidecar manual restart is required for some workloads"
+	ConditionReasonProxySidecarRestartSucceeded                 ConditionReason = "ProxySidecarRestartSucceeded"
+	ConditionReasonProxySidecarRestartSucceededMessage                          = "Proxy sidecar restart succeeded"
+	ConditionReasonProxySidecarRestartFailed                    ConditionReason = "ProxySidecarRestartFailed"
+	ConditionReasonProxySidecarRestartFailedMessage                             = "Proxy sidecar restart failed"
+	ConditionReasonProxySidecarRestartPartiallySucceeded        ConditionReason = "ProxySidecarRestartPartiallySucceeded"
+	ConditionReasonProxySidecarRestartPartiallySucceededMessage                 = "Proxy sidecar restart partially succeeded"
+	ConditionReasonProxySidecarManualRestartRequired            ConditionReason = "ProxySidecarManualRestartRequired"
+	ConditionReasonProxySidecarManualRestartRequiredMessage                     = "Proxy sidecar manual restart is required for some workloads"
 
 	// ingress gateway
 	ConditionReasonIngressGatewayRestartSucceeded        ConditionReason = "IngressGatewayRestartSucceeded"

controllers/istio_controller.go

@@ -146,14 +146,18 @@ func (r *IstioReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 		return r.requeueReconciliation(ctx, &istioCR, resourcesErr, operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonCRsReconcileFailed))
 	}
 
-	if err := restarter.Restart(ctx, &istioCR, r.restarters); err != nil {
+	err, requeue := restarter.Restart(ctx, &istioCR, r.restarters)
+	if err != nil {
 		// We don't want to use the requeueReconciliation function here, since there is condition handling in this function, and we
 		// need to clean this up, before we can use it here as conditions are already handled in the restarters.
 		statusUpdateErr := r.statusHandler.UpdateToError(ctx, &istioCR, err)
 		if statusUpdateErr != nil {
 			r.log.Error(statusUpdateErr, "Error during updating status to error")
 		}
 		return ctrl.Result{}, err
+	} else if requeue {
+		r.statusHandler.SetCondition(&istioCR, operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonReconcileRequeued))
+		return r.requeueReconciliationWithoutError(ctx, &istioCR)
 	}
 
 	return r.finishReconcile(ctx, &istioCR, istioImageVersion.Tag())
@@ -168,11 +172,19 @@ func (r *IstioReconciler) requeueReconciliation(ctx context.Context, istioCR *op
 	if statusUpdateErr != nil {
 		r.log.Error(statusUpdateErr, "Error during updating status to error")
 	}
-
 	r.log.Error(err, "Reconcile failed")
 	return ctrl.Result{}, err
 }
 
+func (r *IstioReconciler) requeueReconciliationWithoutError(ctx context.Context, istioCR *operatorv1alpha2.Istio) (ctrl.Result, error) {
+	statusUpdateErr := r.statusHandler.UpdateToProcessing(ctx, istioCR)
+	if statusUpdateErr != nil {
+		r.log.Error(statusUpdateErr, "Error during updating status to error")
+	}
+	r.log.Info("Reconcile requeued")
+	return ctrl.Result{Requeue: true}, nil
+}
+
 // terminateReconciliation stops the reconciliation and does not requeue the request.
 func (r *IstioReconciler) terminateReconciliation(ctx context.Context, istioCR *operatorv1alpha2.Istio, err described_errors.DescribedError, reason operatorv1alpha2.ReasonWithMessage) (ctrl.Result, error) {
 	if err.ShouldSetCondition() {

controllers/istio_controller_test.go

@@ -322,7 +322,6 @@ var _ = Describe("Istio Controller", func() {
 
 			Expect(updatedIstioCR.Status.Conditions).ToNot(BeNil())
 			Expect(*updatedIstioCR.Status.Conditions).To(HaveLen(1))
-
 			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))
@@ -413,7 +412,6 @@ var _ = Describe("Istio Controller", func() {
 
 			Expect(updatedIstioCR.Status.Conditions).ToNot(BeNil())
 			Expect(*updatedIstioCR.Status.Conditions).To(HaveLen(1))
-
 			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))
@@ -519,7 +517,6 @@ var _ = Describe("Istio Controller", func() {
 
 			Expect(updatedIstioCR.Status.Conditions).ToNot(BeNil())
 			Expect(*updatedIstioCR.Status.Conditions).To(HaveLen(1))
-
 			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))
@@ -574,7 +571,6 @@ var _ = Describe("Istio Controller", func() {
 
 			Expect(updatedIstioCR.Status.Conditions).ToNot(BeNil())
 			Expect(*updatedIstioCR.Status.Conditions).To(HaveLen(1))
-
 			Expect((*updatedIstioCR.Status.Conditions)[0].Type).To(Equal(string(operatorv1alpha2.ConditionTypeReady)))
 			Expect((*updatedIstioCR.Status.Conditions)[0].Reason).To(Equal(string(operatorv1alpha2.ConditionReasonOlderCRExists)))
 			Expect((*updatedIstioCR.Status.Conditions)[0].Status).To(Equal(metav1.ConditionFalse))
@@ -618,7 +614,6 @@ var _ = Describe("Istio Controller", func() {
 
 			Expect(updatedIstioCR.Status.Conditions).ToNot(BeNil())
 			Expect(*updatedIstioCR.Status.Conditions).To(HaveLen(1))
-
 			Expect((*updatedIstioCR.Status.Conditions)[0].Type).To(Equal(string(operatorv1alpha2.ConditionTypeReady)))
 			Expect((*updatedIstioCR.Status.Conditions)[0].Reason).To(Equal(string(operatorv1alpha2.ConditionReasonReconcileFailed)))
 			Expect((*updatedIstioCR.Status.Conditions)[0].Status).To(Equal(metav1.ConditionFalse))
@@ -680,7 +675,6 @@ var _ = Describe("Istio Controller", func() {
 
 			Expect(updatedIstioCR.Status.Conditions).ToNot(BeNil())
 			Expect(*updatedIstioCR.Status.Conditions).To(HaveLen(1))
-
 			Expect((*updatedIstioCR.Status.Conditions)[0].Type).To(Equal(string(operatorv1alpha2.ConditionTypeReady)))
 			Expect((*updatedIstioCR.Status.Conditions)[0].Reason).To(Equal(string(operatorv1alpha2.ConditionReasonValidationFailed)))
 			Expect((*updatedIstioCR.Status.Conditions)[0].Status).To(Equal(metav1.ConditionFalse))
@@ -721,10 +715,9 @@ var _ = Describe("Istio Controller", func() {
 			updatedIstioCR := operatorv1alpha2.Istio{}
 			Expect(fakeClient.Get(context.Background(), client.ObjectKeyFromObject(istioCR), &updatedIstioCR)).Should(Succeed())
 
+			By("Verifying that Istio CR has Condition Ready with False")
 			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))
@@ -753,10 +746,9 @@ var _ = Describe("Istio Controller", func() {
 			updatedIstioCR = operatorv1alpha2.Istio{}
 			Expect(fakeClient.Get(context.Background(), client.ObjectKeyFromObject(istioCR), &updatedIstioCR)).Should(Succeed())
 
+			By("Verifying that the condition lastTransitionTime is also updated when only the reason has changed")
 			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))
@@ -766,7 +758,6 @@ var _ = Describe("Istio Controller", func() {
 		})
 
 		Context("Restarters", func() {
-
 			It("should restart if reconciliations are successful", func() {
 				//given
 				istioCR := &operatorv1alpha2.Istio{
@@ -960,22 +951,76 @@ var _ = Describe("Istio Controller", func() {
 				Expect(updatedIstioCR.Status.State).Should(Equal(operatorv1alpha2.Warning))
 				Expect(updatedIstioCR.Status.Description).To(ContainSubstring("Restart Warning description"))
 			})
+
+			It("should requeue reconcile request when a restarter needs to finish work on the next reconcile", 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)
+				ingressGatewayRestarter := &restarterMock{restarted: false}
+				proxySidecarsRestarter := &restarterMock{restarted: false, requeue: true}
+				sut := &IstioReconciler{
+					Client:                 fakeClient,
+					Scheme:                 getTestScheme(),
+					istioInstallation:      &istioInstallationReconciliationMock{},
+					istioResources:         &istioResourcesReconciliationMock{},
+					restarters:             []restarter.Restarter{ingressGatewayRestarter, proxySidecarsRestarter},
+					log:                    logr.Discard(),
+					statusHandler:          status.NewStatusHandler(fakeClient),
+					reconciliationInterval: testReconciliationInterval,
+				}
+
+				//when
+				reconcileResult, err := sut.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{Namespace: testNamespace, Name: istioCrName}})
+
+				//then
+				Expect(err).ToNot(HaveOccurred())
+				Expect(reconcileResult.Requeue).To(BeTrue())
+
+				Expect(ingressGatewayRestarter.RestartCalled()).To(BeTrue())
+				Expect(proxySidecarsRestarter.RestartCalled()).To(BeTrue())
+
+				updatedIstioCR := operatorv1alpha2.Istio{}
+				err = fakeClient.Get(context.Background(), client.ObjectKeyFromObject(istioCR), &updatedIstioCR)
+				Expect(err).To(Not(HaveOccurred()))
+
+				Expect(updatedIstioCR.Status.State).To(Equal(operatorv1alpha2.Processing))
+
+				By("Verifying that Istio CR has Condition Ready status with Requeued reason")
+				Expect(updatedIstioCR.Status.Conditions).ToNot(BeNil())
+				Expect(*updatedIstioCR.Status.Conditions).To(HaveLen(1))
+				Expect((*updatedIstioCR.Status.Conditions)[0].Type).To(Equal(string(operatorv1alpha2.ConditionTypeReady)))
+				Expect((*updatedIstioCR.Status.Conditions)[0].Reason).To(Equal(string(operatorv1alpha2.ConditionReasonReconcileRequeued)))
+				Expect((*updatedIstioCR.Status.Conditions)[0].Message).To(Equal(operatorv1alpha2.ConditionReasonReconcileRequeuedMessage))
+				Expect((*updatedIstioCR.Status.Conditions)[0].Status).To(Equal(metav1.ConditionFalse))
+			})
 		})
 	})
 })
 
 type restarterMock struct {
 	err       described_errors.DescribedError
+	requeue   bool
 	restarted bool
 }
 
 func (i *restarterMock) RestartCalled() bool {
 	return i.restarted
 }
 
-func (i *restarterMock) Restart(_ context.Context, _ *operatorv1alpha2.Istio) described_errors.DescribedError {
+func (i *restarterMock) Restart(_ context.Context, _ *operatorv1alpha2.Istio) (described_errors.DescribedError, bool) {
 	i.restarted = true
-	return i.err
+	return i.err, i.requeue
 }
 
 type istioResourcesReconciliationMock struct {

docs/contributor/04-10-technical-design.md

@@ -103,17 +103,20 @@ Additionally, clear status reporting and the ability to provide detailed informa
 
 ### Restart Predicates
 
-The [SidecarRestarter](#sidecarsrestarter) and [IngressGatewayRestarter](#ingressgatewayrestarter)  components use Restart Predicates.
+The [SidecarRestarter](#sidecarsrestarter) and [IngressGatewayRestarter](#ingressgatewayrestarter) components use Restart Predicates.
 Depending on the implemented interfaces, a predicate can trigger a restart of Ingress Gateways, Proxy Sidecars, or both Ingress Gateways and Proxy Sidecars.
 
 For cases where it isn't trivial to check whether the configuration has been applied to the cluster state, Restart Predicates use a timestamp-based approach. For example, the `envoy_filter_allow_partial_referer` resource has the `istios.operator.kyma-project.io/updatedAt` annotation, which includes the timestamp of its last update.
 The predicate initiates a restart of the sidecar and Ingress Gateway if the target was created before this timestamp.
 
 ### SidecarsRestarter
 
-The SidecarsRestarter is responsible for keeping the proxy sidecars in the desired state. It restarts Pods that are part of the service mesh or
-that must be added to the service mesh.
-The Istio CR and [Istio Version](#istio-version) represent the desired state.
+The SidecarsRestarter is responsible for keeping the proxy sidecars in the desired state. It restarts Pods that are in running state and are part of the service mesh having the annotation `sidecar.istio.io/status`.
+The Istio CR and [Istio Version](#istio-version) represent the desired state. Pods will be restarted in chunks with limits on how many to restart with one reconciliation and how many to list when requesting from Kubernetes API Server. If more Pods need to be restarted this will happen in next reconciliation. This happens with requeuing the reconciliation request, which will be imediately rescheduled.
+
+During proxy sidecars restarting phase, Istio CR will be kept in `Processing` state having following status conditions:
+- `Ready` condition set to `false` with `ReconcileRequeued` reason.
+- `ProxySidecarRestartSucceeded` condition set to `false` with `ProxySidecarPartiallySucceeded` reason.
 
 This component covers the following restart triggers: