Skip to content

Commit

Permalink
fix: update stack status only if Prometheus generation is different
Browse files Browse the repository at this point in the history 
This adds check comparing the Prometheus observedGeneration to the current
Prometheus generation. If there is a difference then the
MonitoringStack status is not updated.

JIRA: https://issues.redhat.com/browse/MON-2796
  • Loading branch information
@tremes
tremes authored Nov 1, 2022
1 parent 4625e3e commit 270ec28
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 35 deletions.
28 changes: 18 additions & 10 deletions pkg/controllers/monitoring/monitoring-stack/conditions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const (
NoReason = "None"
)

func updateConditions(msConditions []v1alpha1.Condition, promConditions []monv1.PrometheusCondition, generation int64, recError error) []v1alpha1.Condition {
func updateConditions(msConditions []v1alpha1.Condition, prom monv1.Prometheus, generation int64, recError error) []v1alpha1.Condition {
if len(msConditions) == 0 {
return []v1alpha1.Condition{
{
Expand All @@ -43,13 +43,13 @@ func updateConditions(msConditions []v1alpha1.Condition, promConditions []monv1.
for _, mc := range msConditions {
switch mc.Type {
case v1alpha1.AvailableCondition:
available := updateAvailable(mc, promConditions, generation)
available := updateAvailable(mc, prom, generation)
if !available.Equal(mc) {
available.LastTransitionTime = metav1.Now()
}
updatedConditions = append(updatedConditions, available)
case v1alpha1.ReconciledCondition:
reconciled := updateReconciled(mc, promConditions, generation, recError)
reconciled := updateReconciled(mc, prom, generation, recError)
if !reconciled.Equal(mc) {
reconciled.LastTransitionTime = metav1.Now()
}
Expand All @@ -62,17 +62,20 @@ func updateConditions(msConditions []v1alpha1.Condition, promConditions []monv1.

// updateAvailable gets existing "Available" condition and updates its parameters
// based on the Prometheus "Available" condition
func updateAvailable(ac v1alpha1.Condition, prometheusConditions []monv1.PrometheusCondition, generation int64) v1alpha1.Condition {
ac.ObservedGeneration = generation

prometheusAvailable, err := getPrometheusCondition(prometheusConditions, monv1.PrometheusAvailable)
func updateAvailable(ac v1alpha1.Condition, prom monv1.Prometheus, generation int64) v1alpha1.Condition {
prometheusAvailable, err := getPrometheusCondition(prom.Status.Conditions, monv1.PrometheusAvailable)

if err != nil {
ac.Status = v1alpha1.ConditionUnknown
ac.Reason = PrometheusNotAvailable
ac.Message = CannotReadPrometheusConditions
return ac
}
// MonitoringStack status will not be updated if there is a difference between the Prometheus generation
// and the Prometheus ObservedGeneration. This can occur, for example, in the case of an invalid Prometheus configuration.
if prometheusAvailable.ObservedGeneration != prom.Generation {
return ac
}

if prometheusAvailable.Status != monv1.PrometheusConditionTrue {
ac.Status = prometheusStatusToMSStatus(prometheusAvailable.Status)
Expand All @@ -87,21 +90,21 @@ func updateAvailable(ac v1alpha1.Condition, prometheusConditions []monv1.Prometh
ac.Status = v1alpha1.ConditionTrue
ac.Reason = AvailableReason
ac.Message = AvailableMessage
ac.ObservedGeneration = generation
return ac
}

// updateReconciled updates "Reconciled" conditions based on the provided error value and
// Prometheus "Reconciled" condition
func updateReconciled(rc v1alpha1.Condition, prometheusConditions []monv1.PrometheusCondition, generation int64, err error) v1alpha1.Condition {
rc.ObservedGeneration = generation
func updateReconciled(rc v1alpha1.Condition, prom monv1.Prometheus, generation int64, err error) v1alpha1.Condition {

if err != nil {
rc.Status = v1alpha1.ConditionFalse
rc.Message = err.Error()
rc.Reason = FailedToReconcileReason
return rc
}
prometheusReconciled, err := getPrometheusCondition(prometheusConditions, monv1.PrometheusReconciled)
prometheusReconciled, err := getPrometheusCondition(prom.Status.Conditions, monv1.PrometheusReconciled)

if err != nil {
rc.Status = v1alpha1.ConditionUnknown
Expand All @@ -110,6 +113,10 @@ func updateReconciled(rc v1alpha1.Condition, prometheusConditions []monv1.Promet
return rc
}

if prometheusReconciled.ObservedGeneration != prom.Generation {
return rc
}

if prometheusReconciled.Status != monv1.PrometheusConditionTrue {
rc.Status = prometheusStatusToMSStatus(prometheusReconciled.Status)
rc.Reason = PrometheusNotReconciled
Expand All @@ -119,6 +126,7 @@ func updateReconciled(rc v1alpha1.Condition, prometheusConditions []monv1.Promet
rc.Status = v1alpha1.ConditionTrue
rc.Reason = ReconciledReason
rc.Message = SuccessfullyReconciledMessage
rc.ObservedGeneration = generation
return rc
}

Expand Down
120 changes: 96 additions & 24 deletions pkg/controllers/monitoring/monitoring-stack/conditions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func TestUpdateConditions(t *testing.T) {
tt := []struct {
name string
originalMSConditions []v1alpha1.Condition
prometheusConditions []monv1.PrometheusCondition
prometheus monv1.Prometheus
generation int64
recError error
expectedResults []v1alpha1.Condition
Expand All @@ -27,7 +27,7 @@ func TestUpdateConditions(t *testing.T) {
originalMSConditions: []v1alpha1.Condition{},
generation: 1,
recError: nil,
prometheusConditions: []monv1.PrometheusCondition{},
prometheus: monv1.Prometheus{},
expectedResults: []v1alpha1.Condition{
{
Type: v1alpha1.AvailableCondition,
Expand Down Expand Up @@ -62,16 +62,23 @@ func TestUpdateConditions(t *testing.T) {
},
generation: 1,
recError: nil,
prometheusConditions: []monv1.PrometheusCondition{
{
Type: monv1.PrometheusAvailable,
Status: monv1.PrometheusConditionTrue,
},
{
Type: monv1.PrometheusReconciled,
Status: monv1.PrometheusConditionTrue,
prometheus: monv1.Prometheus{
ObjectMeta: metav1.ObjectMeta{
Generation: 1,
},
},
Status: monv1.PrometheusStatus{
Conditions: []monv1.PrometheusCondition{
{
Type: monv1.PrometheusAvailable,
Status: monv1.PrometheusConditionTrue,
ObservedGeneration: 1,
},
{
Type: monv1.PrometheusReconciled,
Status: monv1.PrometheusConditionTrue,
ObservedGeneration: 1,
},
}}},
expectedResults: []v1alpha1.Condition{
{
Type: v1alpha1.AvailableCondition,
Expand Down Expand Up @@ -111,9 +118,9 @@ func TestUpdateConditions(t *testing.T) {
LastTransitionTime: transitionTime,
},
},
generation: 1,
recError: nil,
prometheusConditions: []monv1.PrometheusCondition{},
generation: 1,
recError: nil,
prometheus: monv1.Prometheus{},
expectedResults: []v1alpha1.Condition{
{
Type: v1alpha1.AvailableCondition,
Expand Down Expand Up @@ -152,16 +159,23 @@ func TestUpdateConditions(t *testing.T) {
},
generation: 1,
recError: nil,
prometheusConditions: []monv1.PrometheusCondition{
{
Type: monv1.PrometheusAvailable,
Status: monv1.PrometheusConditionDegraded,
prometheus: monv1.Prometheus{
ObjectMeta: metav1.ObjectMeta{
Generation: 1,
},
{
Type: monv1.PrometheusReconciled,
Status: monv1.PrometheusConditionDegraded,
},
},
Status: monv1.PrometheusStatus{
Conditions: []monv1.PrometheusCondition{
{
Type: monv1.PrometheusAvailable,
Status: monv1.PrometheusConditionDegraded,
ObservedGeneration: 1,
},
{
Type: monv1.PrometheusReconciled,
Status: monv1.PrometheusConditionDegraded,
ObservedGeneration: 1,
},
}}},
expectedResults: []v1alpha1.Condition{
{
Type: v1alpha1.AvailableCondition,
Expand All @@ -176,10 +190,68 @@ func TestUpdateConditions(t *testing.T) {
Reason: PrometheusNotReconciled,
}},
},
{
name: "Prometheus observed generation is different from the Prometheus generation",
originalMSConditions: []v1alpha1.Condition{
{
Type: v1alpha1.AvailableCondition,
Status: v1alpha1.ConditionTrue,
ObservedGeneration: 2,
Reason: AvailableReason,
Message: AvailableMessage,
LastTransitionTime: transitionTime,
},
{
Type: v1alpha1.ReconciledCondition,
Status: v1alpha1.ConditionTrue,
ObservedGeneration: 2,
Reason: ReconciledReason,
Message: SuccessfullyReconciledMessage,
LastTransitionTime: transitionTime,
},
},
generation: 2,
recError: nil,
prometheus: monv1.Prometheus{
ObjectMeta: metav1.ObjectMeta{
Generation: 3,
},
Status: monv1.PrometheusStatus{
Conditions: []monv1.PrometheusCondition{
{
Type: monv1.PrometheusAvailable,
Status: monv1.PrometheusConditionFalse,
ObservedGeneration: 2,
},
{
Type: monv1.PrometheusReconciled,
Status: monv1.PrometheusConditionFalse,
ObservedGeneration: 2,
},
}}},
expectedResults: []v1alpha1.Condition{
{
Type: v1alpha1.AvailableCondition,
Status: v1alpha1.ConditionTrue,
ObservedGeneration: 2,
Reason: AvailableReason,
Message: AvailableMessage,
LastTransitionTime: transitionTime,
},
{
Type: v1alpha1.ReconciledCondition,
Status: v1alpha1.ConditionTrue,
ObservedGeneration: 2,
Reason: ReconciledReason,
Message: SuccessfullyReconciledMessage,
LastTransitionTime: transitionTime,
}},
sameTransitionTimes: true,
},
}

for _, test := range tt {
res := updateConditions(test.originalMSConditions, test.prometheusConditions, test.generation, test.recError)
res := updateConditions(test.originalMSConditions, test.prometheus, test.generation, test.recError)
for _, c := range res {
expectedC := getConditionByType(test.expectedResults, c.Type)
assert.Check(t, expectedC.Equal(c), "%s - expected:\n %v\n and got:\n %v\n", test.name, expectedC, c)
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/monitoring/monitoring-stack/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func (rm resourceManager) updateStatus(ctx context.Context, req ctrl.Request, ms
logger.Info("Failed to get prometheus object", "err", err)
return ctrl.Result{RequeueAfter: 2 * time.Second}
}
ms.Status.Conditions = updateConditions(ms.Status.Conditions, prom.Status.Conditions, ms.Generation, recError)
ms.Status.Conditions = updateConditions(ms.Status.Conditions, prom, ms.Generation, recError)
err = rm.k8sClient.Status().Update(ctx, ms)
if err != nil {
logger.Info("Failed to update status", "err", err)
Expand Down

0 comments on commit 270ec28

Please sign in to comment.