Skip to content

Commit

Permalink
Change OlderCRExists state from error to warning (#725)
Browse files Browse the repository at this point in the history 
* Change OlderCRExists state from error to warning

* Change error message

* Improve message
  • Loading branch information
@MarekMichali
MarekMichali authored Jun 14, 2024
1 parent 144711e commit 5012b23
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 12 deletions.
6 changes: 3 additions & 3 deletions controllers/btpoperator_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ func (r *BtpOperatorReconciler) setNewLeader(ctx context.Context, existingBtpOpe
logger := log.FromContext(ctx)
logger.Info(fmt.Sprintf("Found %d existing BtpOperators", len(existingBtpOperators.Items)))
oldestCr := r.getOldestCR(existingBtpOperators)
if oldestCr.GetStatus().State == v1alpha1.StateError && oldestCr.IsReasonStringEqual(string(conditions.OlderCRExists)) {
if oldestCr.GetStatus().State == v1alpha1.StateWarning && oldestCr.IsReasonStringEqual(string(conditions.OlderCRExists)) {
if err := r.UpdateBtpOperatorStatus(ctx, oldestCr, v1alpha1.StateProcessing, conditions.Processing,
fmt.Sprintf("%s is the new leader", oldestCr.GetName())); err != nil {
logger.Error(err, fmt.Sprintf("unable to set %s BtpOperator CR as the new leader", oldestCr.GetName()))
Expand Down Expand Up @@ -283,8 +283,8 @@ func (r *BtpOperatorReconciler) getOldestCR(existingBtpOperators *v1alpha1.BtpOp
func (r *BtpOperatorReconciler) HandleRedundantCR(ctx context.Context, oldestCr *v1alpha1.BtpOperator, cr *v1alpha1.BtpOperator) error {
logger := log.FromContext(ctx)
logger.Info("Handling redundant BtpOperator CR")
return r.UpdateBtpOperatorStatus(ctx, cr, v1alpha1.StateError, conditions.OlderCRExists, fmt.Sprintf("'%s' BtpOperator CR in '%s' namespace reconciles the module",
oldestCr.GetName(), oldestCr.GetNamespace()))
return r.UpdateBtpOperatorStatus(ctx, cr, v1alpha1.StateWarning, conditions.OlderCRExists, fmt.Sprintf("The '%s' BtpOperator CR in '%s' namespace reconciles the module. To use this CR to reconcile the module, delete the '%s' BtpOperator CR in the '%s' namespace and remove finalizers from the '%s' BtpOperator CR in the '%s' namespace.",
oldestCr.GetName(), oldestCr.GetNamespace(), oldestCr.GetName(), oldestCr.GetNamespace(), oldestCr.GetName(), oldestCr.GetNamespace()))
}

func (r *BtpOperatorReconciler) UpdateBtpOperatorStatus(ctx context.Context, cr *v1alpha1.BtpOperator, newState v1alpha1.State, reason conditions.Reason, message string) error {
Expand Down
14 changes: 7 additions & 7 deletions controllers/btpoperator_controller_cr_rotation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@ var _ = Describe("BTP Operator CR leader replacement", func() {
time.Sleep(1 * time.Second)
btpOperator2 := createBtpOperator(secondBtpOperator)
Eventually(func() error { return k8sClient.Create(ctx, btpOperator2) }).WithTimeout(k8sOpsTimeout).WithPolling(k8sOpsPollingInterval).Should(Succeed())
Eventually(updateCh).Should(Receive(matchState(v1alpha1.StateError)))
Eventually(updateCh).Should(Receive(matchState(v1alpha1.StateWarning)))

// required for later CreationTimestamp than btpOperator2
time.Sleep(1 * time.Second)
btpOperator3 := createBtpOperator(thirdBtpOperator)
Eventually(func() error { return k8sClient.Create(ctx, btpOperator3) }).WithTimeout(k8sOpsTimeout).WithPolling(k8sOpsPollingInterval).Should(Succeed())
Eventually(updateCh).Should(Receive(matchState(v1alpha1.StateError)))
Eventually(updateCh).Should(Receive(matchState(v1alpha1.StateWarning)))

btpOperators := &v1alpha1.BtpOperatorList{}
Expect(k8sClient.List(ctx, btpOperators)).To(Succeed())
Expand All @@ -76,11 +76,11 @@ var _ = Describe("BTP Operator CR leader replacement", func() {
}).WithTimeout(k8sOpsTimeout).WithPolling(k8sOpsPollingInterval).Should(BeTrue())
Eventually(func() (bool, error) {
err := k8sClient.Get(ctx, client.ObjectKey{Namespace: defaultNamespace, Name: secondBtpOperator}, btpOperatorWithCurrentState)
return btpOperatorWithCurrentState.Status.State == v1alpha1.StateError && btpOperatorWithCurrentState.Status.Conditions[0].Reason == string(conditions.OlderCRExists), err
return btpOperatorWithCurrentState.Status.State == v1alpha1.StateWarning && btpOperatorWithCurrentState.Status.Conditions[0].Reason == string(conditions.OlderCRExists), err
}).WithTimeout(k8sOpsTimeout).WithPolling(k8sOpsPollingInterval).Should(BeTrue())
Eventually(func() (bool, error) {
err := k8sClient.Get(ctx, client.ObjectKey{Namespace: defaultNamespace, Name: thirdBtpOperator}, btpOperatorWithCurrentState)
return btpOperatorWithCurrentState.Status.State == v1alpha1.StateError && btpOperatorWithCurrentState.Status.Conditions[0].Reason == string(conditions.OlderCRExists), err
return btpOperatorWithCurrentState.Status.State == v1alpha1.StateWarning && btpOperatorWithCurrentState.Status.Conditions[0].Reason == string(conditions.OlderCRExists), err
}).WithTimeout(k8sOpsTimeout).WithPolling(k8sOpsPollingInterval).Should(BeTrue())

Expect(k8sClient.Delete(ctx, btpOperator1)).To(Succeed())
Expand All @@ -97,7 +97,7 @@ var _ = Describe("BTP Operator CR leader replacement", func() {

Eventually(func() (bool, error) {
err := k8sClient.Get(ctx, client.ObjectKey{Namespace: defaultNamespace, Name: thirdBtpOperator}, btpOperatorWithCurrentState)
return btpOperatorWithCurrentState.Status.State == v1alpha1.StateError && btpOperatorWithCurrentState.Status.Conditions[0].Reason == string(conditions.OlderCRExists), err
return btpOperatorWithCurrentState.Status.State == v1alpha1.StateWarning && btpOperatorWithCurrentState.Status.Conditions[0].Reason == string(conditions.OlderCRExists), err
}).WithTimeout(k8sOpsTimeout).WithPolling(k8sOpsPollingInterval).Should(BeTrue())
})
})
Expand All @@ -120,7 +120,7 @@ var _ = Describe("BTP Operator CR leader replacement", func() {
time.Sleep(1 * time.Second)
btpOperator2 := createBtpOperator(secondBtpOperator)
Eventually(func() error { return k8sClient.Create(ctx, btpOperator2) }).WithTimeout(k8sOpsTimeout).WithPolling(k8sOpsPollingInterval).Should(Succeed())
Eventually(updateCh).Should(Receive(matchState(v1alpha1.StateError)))
Eventually(updateCh).Should(Receive(matchState(v1alpha1.StateWarning)))

btpOperators := &v1alpha1.BtpOperatorList{}
Expect(k8sClient.List(ctx, btpOperators)).To(Succeed())
Expand All @@ -133,7 +133,7 @@ var _ = Describe("BTP Operator CR leader replacement", func() {
}).WithTimeout(k8sOpsTimeout).WithPolling(k8sOpsPollingInterval).Should(BeTrue())
Eventually(func() (bool, error) {
err := k8sClient.Get(ctx, client.ObjectKey{Namespace: defaultNamespace, Name: secondBtpOperator}, btpOperatorWithCurrentState)
return btpOperatorWithCurrentState.Status.State == v1alpha1.StateError && btpOperatorWithCurrentState.Status.Conditions[0].Reason == string(conditions.OlderCRExists), err
return btpOperatorWithCurrentState.Status.State == v1alpha1.StateWarning && btpOperatorWithCurrentState.Status.Conditions[0].Reason == string(conditions.OlderCRExists), err
}).WithTimeout(k8sOpsTimeout).WithPolling(k8sOpsPollingInterval).Should(BeTrue())

Expect(k8sClient.Delete(ctx, btpOperator1)).To(Succeed())
Expand Down
2 changes: 1 addition & 1 deletion docs/contributor/02-10-operations.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ Only one Condition of type `Ready` is used.
| 15 | Error | Ready | false | GettingConfigMapFailed | Getting Config Map failed |
| 16 | Error | Ready | false | InconsistentChart | Chart is inconsistent. Reconciliation initialized |
| 17 | Error | Ready | false | InvalidSecret | sap-btp-manager secret does not contain required data - create proper secret |
| 18 | Error | Ready | false | OlderCRExists | This CR is not the oldest one so does not represent the module State |
| 18 | Warning | Ready | false | OlderCRExists | This CR is not the oldest one so does not represent the module State |
| 19 | Error | Ready | false | PreparingInstallInfoFailed | Error while preparing installation information |
| 20 | Error | Ready | false | ProvisioningFailed | Provisioning failed |
| 21 | Error | Ready | false | ReconcileFailed | Reconciliation failed |
Expand Down
2 changes: 1 addition & 1 deletion internal/conditions/conditions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ var Reasons = map[Reason]Metadata{
ChartInstallFailed: {Status: metav1.ConditionFalse, State: v1alpha1.StateError}, //Error;Failure during chart installation
ConsistencyCheckFailed: {Status: metav1.ConditionFalse, State: v1alpha1.StateError}, //Error;Failure during consistency check
Processing: {Status: metav1.ConditionFalse, State: v1alpha1.StateProcessing}, //Processing;Final State after deprovisioning
OlderCRExists: {Status: metav1.ConditionFalse, State: v1alpha1.StateError}, //Error;This CR is not the oldest one so does not represent the module State
OlderCRExists: {Status: metav1.ConditionFalse, State: v1alpha1.StateWarning}, //Warning;This CR is not the oldest one so does not represent the module State
MissingSecret: {Status: metav1.ConditionFalse, State: v1alpha1.StateWarning}, //Warning;sap-btp-manager secret was not found - create proper secret
InvalidSecret: {Status: metav1.ConditionFalse, State: v1alpha1.StateError}, //Error;sap-btp-manager secret does not contain required data - create proper secret
HardDeleting: {Status: metav1.ConditionFalse, State: v1alpha1.StateDeleting}, //Deleting;Trying to hard delete
Expand Down

0 comments on commit 5012b23

Please sign in to comment.