fix: do not infinitely reconcile in case of exit code > 0 (#284)

DoodleScheduling · Sep 23, 2024 · 5f5481e · 5f5481e
1 parent 3af65a0
commit 5f5481e
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 151 deletions.
diff --git a/README.md b/README.md
@@ -46,7 +46,6 @@ spec:
   authSecret:
     name: keycloak-admin
   interval: 1h
-  timeout: 5m0s
   realm:
     accessCodeLifespan: 60
     accessCodeLifespanLogin: 1800
@@ -170,7 +169,6 @@ spec:
     passwordField: password
     userField: username
   interval: 1h
-  timeout: 5m0s
   suspend: false
   realm:
     accessCodeLifespan: 60
@@ -211,7 +209,6 @@ spec:
   authSecret:
     name: keycloak-admin
   interval: 1h
-  timeout: 5m0s
   realm:
     accessCodeLifespan: 60
     accessCodeLifespanLogin: 1800
@@ -268,7 +265,6 @@ spec:
   authSecret:
     name: keycloak-admin
   interval: 1h
-  timeout: 5m0s
   realm:
     accessCodeLifespan: 60
     accessCodeLifespanLogin: 1800

diff --git a/api/v1beta1/keycloakrealm_types.go b/api/v1beta1/keycloakrealm_types.go
@@ -64,6 +64,7 @@ type KeycloakRealmSpec struct {
 
 	// Timeout
 	// +optional
+	// +deprecated
 	Timeout *metav1.Duration `json:"timeout,omitempty"`
 
 	// Suspend reconciliation

diff --git a/internal/controllers/keycloakrealm_controller.go b/internal/controllers/keycloakrealm_controller.go
@@ -324,13 +324,13 @@ func (r *KeycloakRealmReconciler) reconcile(ctx context.Context, realm infrav1be
 	// cleanup reconciler pod if stale
 	if needUpdate {
 		logger.V(1).Info("realm checksum changed, delete stale reconciler", "pod-name", realm.Status.Reconciler)
-		return realm, ctrl.Result{}, cleanup()
+		return realm, ctrl.Result{Requeue: true}, cleanup()
 	}
 
 	// garbage collect reconciler pod
-	if progressingCondition != nil && readyCondition != nil && readyCondition.Status != metav1.ConditionUnknown && podErr == nil && realm.Status.Reconciler != "" {
+	if progressingCondition != nil && readyCondition != nil && readyCondition.Status == metav1.ConditionTrue && podErr == nil && realm.Status.Reconciler != "" {
 		logger.V(1).Info("garbage collect reconciler pod", "pod-name", realm.Status.Reconciler)
-		return realm, ctrl.Result{}, cleanup()
+		return realm, ctrl.Result{Requeue: true}, cleanup()
 	}
 
 	// rate limiter
@@ -374,12 +374,9 @@ func (r *KeycloakRealmReconciler) handlerReconcilerState(realm infrav1beta1.Keyc
 		return realm, ctrl.Result{Requeue: true}, nil
 
 	case containerStatus.State.Terminated != nil:
-		realm = infrav1beta1.KeycloakRealmReady(realm, metav1.ConditionFalse, "ReconciliationFailed", fmt.Sprintf("reconciler terminated with code %d", containerStatus.State.Terminated.ExitCode))
-		return realm, ctrl.Result{Requeue: true}, nil
-
-	case containerStatus.State.Running != nil && realm.Spec.Timeout != nil && time.Since(containerStatus.State.Running.StartedAt.Time) >= realm.Spec.Timeout.Duration:
-		conditions.Delete(&realm, infrav1beta1.ConditionReconciling)
-		return realm, reconcile.Result{}, errors.New("reconciler timeout reached")
+		err := fmt.Errorf("reconciler terminated with code %d", containerStatus.State.Terminated.ExitCode)
+		realm = infrav1beta1.KeycloakRealmReady(realm, metav1.ConditionFalse, "ReconciliationFailed", err.Error())
+		return realm, ctrl.Result{}, nil
 	}
 
 	return realm, ctrl.Result{}, nil
@@ -522,7 +519,7 @@ func (r *KeycloakRealmReconciler) createReconciler(ctx context.Context, realm in
 		return realm, ctrl.Result{}, err
 	}
 
-	template.Spec.RestartPolicy = corev1.RestartPolicyNever
+	template.Spec.RestartPolicy = corev1.RestartPolicyOnFailure
 	template.Spec.Containers = containers
 	template.Spec.Volumes = append(template.Spec.Volumes, corev1.Volume{
 		Name: "realm",
@@ -799,8 +796,6 @@ func (r *KeycloakRealmReconciler) patchStatus(ctx context.Context, realm *infrav
 		return err
 	}
 
-	//	logger.V(1).Info("update .status", "new", realm.Status, "current", latest.Status)
-
 	return r.Client.Status().Patch(ctx, realm, client.MergeFrom(latest))
 }
 

diff --git a/internal/controllers/keycloakrealm_controller_test.go b/internal/controllers/keycloakrealm_controller_test.go
@@ -1703,139 +1703,4 @@ var _ = Describe("KeycloakRealm controller", func() {
 			Expect(string(secret.Data["realm.json"])).Should(Equal(fmt.Sprintf(`{"realm":"%s","users":[{"username":"%s","enabled":true}],"components":null,"requiredActions":null}`, realm.Name, userName)))
 		})
 	})
-	/*
-		TODO: this test is flaky as the controller already progresses into another reconcile if the timeout occurs.
-
-			When("a realm reconciler runs into spec.timeout", func() {
-				realmName := fmt.Sprintf("realm-%s", rand.String(5))
-
-				It("recreates the reconciler with a new secret", func() {
-					By("creating a new KeycloakRealm")
-					ctx := context.Background()
-
-					authSecret := &corev1.Secret{
-						ObjectMeta: metav1.ObjectMeta{
-							Name:      fmt.Sprintf("auth-%s", rand.String(5)),
-							Namespace: "default",
-						},
-						StringData: map[string]string{
-							"username": "kc-user",
-							"password": "kc-password",
-						},
-					}
-
-					Expect(k8sClient.Create(ctx, authSecret)).Should(Succeed())
-
-					realm := &v1beta1.KeycloakRealm{
-						ObjectMeta: metav1.ObjectMeta{
-							Name:      realmName,
-							Namespace: "default",
-						},
-						Spec: v1beta1.KeycloakRealmSpec{
-							Interval: &metav1.Duration{Duration: time.Second * 100},
-							Timeout:  &metav1.Duration{Duration: time.Second * 100},
-							Version:  "22.0.1",
-							AuthSecret: v1beta1.SecretReference{
-								Name: authSecret.Name,
-							},
-						},
-					}
-					Expect(k8sClient.Create(ctx, realm)).Should(Succeed())
-
-					By("waiting for the reconciliation")
-					instanceLookupKey := types.NamespacedName{Name: realmName, Namespace: "default"}
-					reconciledInstance := &v1beta1.KeycloakRealm{}
-
-					expectedStatus := &v1beta1.KeycloakRealmStatus{
-						ObservedGeneration: 1,
-						Conditions: []metav1.Condition{
-							{
-								Type:    v1beta1.ConditionReady,
-								Status:  metav1.ConditionUnknown,
-								Reason:  "Progressing",
-								Message: "Reconciliation in progress",
-							},
-							{
-								Type:   v1beta1.ConditionReconciling,
-								Status: metav1.ConditionTrue,
-								Reason: "Progressing",
-							},
-						},
-					}
-
-					Eventually(func() bool {
-						err := k8sClient.Get(ctx, instanceLookupKey, reconciledInstance)
-						if err != nil {
-							return false
-						}
-
-						return needStatus(reconciledInstance, expectedStatus)
-					}, timeout, interval).Should(BeTrue())
-
-				})
-
-				It("transitions into unready once the reconciler pod reached the timeout", func() {
-					reconciledInstance := &v1beta1.KeycloakRealm{}
-					instanceLookupKey := types.NamespacedName{Name: realmName, Namespace: "default"}
-					Expect(k8sClient.Get(ctx, instanceLookupKey, reconciledInstance)).Should(Succeed())
-
-					By("container shall be running for 500s")
-					pod := &corev1.Pod{}
-					Expect(k8sClient.Get(ctx, types.NamespacedName{
-						Name:      reconciledInstance.Status.Reconciler,
-						Namespace: reconciledInstance.Namespace,
-					}, pod)).Should(Succeed())
-
-					pod.Status.ContainerStatuses = []corev1.ContainerStatus{
-						{
-							Name: "keycloak-config-cli",
-							State: corev1.ContainerState{
-								Running: &corev1.ContainerStateRunning{
-									StartedAt: metav1.NewTime(time.Now().Add(time.Second * -500)),
-								},
-							},
-						},
-					}
-
-					Expect(k8sClient.Status().Update(ctx, pod)).Should(Succeed())
-
-					By("waiting for the reconciliation")
-					expectedStatus := &v1beta1.KeycloakRealmStatus{
-						ObservedGeneration: 1,
-						Conditions: []metav1.Condition{
-							{
-								Type:    v1beta1.ConditionReady,
-								Status:  metav1.ConditionFalse,
-								Reason:  "ReconciliationFailed",
-								Message: "reconciler timeout reached",
-							},
-						},
-					}
-
-					Eventually(func() bool {
-						err := k8sClient.Get(ctx, instanceLookupKey, reconciledInstance)
-						if err != nil {
-							return false
-						}
-
-						return needStatus(reconciledInstance, expectedStatus)
-					}, timeout, interval).Should(BeTrue())
-
-					By("making sure the reconciler pod is gone")
-					Expect(k8sClient.Get(ctx, types.NamespacedName{
-						Name:      reconciledInstance.Status.Reconciler,
-						Namespace: reconciledInstance.Namespace,
-					}, pod)).Should(Not(BeNil()))
-
-					Expect(reconciledInstance.Status.Reconciler).Should(Equal(""))
-
-					By("making sure the realm secret is gone")
-					var secret *corev1.Secret
-					Expect(k8sClient.Get(ctx, types.NamespacedName{
-						Name:      reconciledInstance.Status.Reconciler,
-						Namespace: reconciledInstance.Namespace,
-					}, secret)).Should(Not(BeNil()))
-				})
-			})
-	*/
 })