From 40a8422001e84a3382e594f45753029b4a8d8732 Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Wed, 13 Mar 2024 15:15:53 +0100 Subject: [PATCH 1/6] ensure pending reason is preserved --- internal/conditions/conditions.go | 35 ++++++++++++--------- internal/reconciler/logparser/status.go | 18 +++++++++-- internal/reconciler/logpipeline/status.go | 18 +++++++++-- internal/reconciler/tracepipeline/status.go | 20 +++++++++--- 4 files changed, 67 insertions(+), 24 deletions(-) diff --git a/internal/conditions/conditions.go b/internal/conditions/conditions.go index e8a914c85..533ddf1d2 100644 --- a/internal/conditions/conditions.go +++ b/internal/conditions/conditions.go @@ -127,23 +127,31 @@ func SetPendingCondition(ctx context.Context, conditions *[]metav1.Condition, ge meta.SetStatusCondition(conditions, pending) } -func SetRunningCondition(ctx context.Context, conditions *[]metav1.Condition, generation int64, reason, resourceName string, messageMap map[string]string) { +func SetRunningCondition(ctx context.Context, conditions *[]metav1.Condition, generation int64, reason, resourceName string, messageMap map[string]string, preUpgradePendingCondition *metav1.Condition, pendingFallbackReason string) { log := logf.FromContext(ctx) - existingPending := meta.FindStatusCondition(*conditions, TypePending) - if existingPending != nil { - newPending := New( - TypePending, - existingPending.Reason, - metav1.ConditionFalse, - generation, - messageMap, - ) - newPending.Message = PendingTypeDeprecationMsg + newPending.Message - log.V(1).Info(fmt.Sprintf("Updating the status of %s: Setting the Pending condition to False", resourceName)) - meta.SetStatusCondition(conditions, newPending) + // Set Pending condition to False + var pendingReason string + existingPendingCondition := meta.FindStatusCondition(*conditions, TypePending) + if existingPendingCondition != nil { + pendingReason = existingPendingCondition.Reason + } else if preUpgradePendingCondition != nil { + pendingReason = preUpgradePendingCondition.Reason + } else { + pendingReason = pendingFallbackReason } + newPending := New( + TypePending, + pendingReason, + metav1.ConditionFalse, + generation, + messageMap, + ) + newPending.Message = PendingTypeDeprecationMsg + newPending.Message + log.V(1).Info(fmt.Sprintf("Updating the status of %s: Setting the Pending condition to False", resourceName)) + meta.SetStatusCondition(conditions, newPending) + // Set Running condition to True running := New( TypeRunning, reason, @@ -152,7 +160,6 @@ func SetRunningCondition(ctx context.Context, conditions *[]metav1.Condition, ge messageMap, ) running.Message = RunningTypeDeprecationMsg + running.Message - log.V(1).Info(fmt.Sprintf("Updating the status of %s: Setting the Running condition to True", resourceName)) meta.SetStatusCondition(conditions, running) } diff --git a/internal/reconciler/logparser/status.go b/internal/reconciler/logparser/status.go index a167b60a9..ae90cd99f 100644 --- a/internal/reconciler/logparser/status.go +++ b/internal/reconciler/logparser/status.go @@ -31,12 +31,15 @@ func (r *Reconciler) updateStatus(ctx context.Context, parserName string) error // If the "AgentHealthy" type doesn't exist in the conditions, // then we need to reset the conditions list to ensure that the "Pending" and "Running" conditions are appended to the end of the conditions list // Check step 3 in https://github.com/kyma-project/telemetry-manager/blob/main/docs/contributor/arch/004-consolidate-pipeline-statuses.md#decision + // At the same time, we need to store the "Pending" condition to preserve the pending reason + var preUpgradePendingCondition *metav1.Condition if meta.FindStatusCondition(parser.Status.Conditions, conditions.TypeAgentHealthy) == nil { + preUpgradePendingCondition = meta.FindStatusCondition(parser.Status.Conditions, conditions.TypePending) parser.Status.Conditions = []metav1.Condition{} } r.setAgentHealthyCondition(ctx, &parser) - r.setPendingAndRunningConditions(ctx, &parser) + r.setPendingAndRunningConditions(ctx, &parser, preUpgradePendingCondition) if err := r.Status().Update(ctx, &parser); err != nil { return fmt.Errorf("failed to update LogParser status: %w", err) @@ -62,7 +65,7 @@ func (r *Reconciler) setAgentHealthyCondition(ctx context.Context, parser *telem meta.SetStatusCondition(&parser.Status.Conditions, conditions.New(conditions.TypeAgentHealthy, reason, status, parser.Generation, conditions.LogsMessage)) } -func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, parser *telemetryv1alpha1.LogParser) { +func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, parser *telemetryv1alpha1.LogParser, preUpgradePendingCondition *metav1.Condition) { fluentBitReady, err := r.prober.IsReady(ctx, r.config.DaemonSet) if err != nil { logf.FromContext(ctx).V(1).Error(err, "Failed to probe fluent bit daemonset") @@ -75,5 +78,14 @@ func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, parser } - conditions.SetRunningCondition(ctx, &parser.Status.Conditions, parser.Generation, conditions.ReasonFluentBitDSReady, parser.Name, conditions.LogsMessage) + conditions.SetRunningCondition( + ctx, + &parser.Status.Conditions, + parser.Generation, + conditions.ReasonFluentBitDSReady, + parser.Name, + conditions.LogsMessage, + preUpgradePendingCondition, + conditions.ReasonFluentBitDSNotReady, + ) } diff --git a/internal/reconciler/logpipeline/status.go b/internal/reconciler/logpipeline/status.go index 8d1498a66..d9d6b3bdb 100644 --- a/internal/reconciler/logpipeline/status.go +++ b/internal/reconciler/logpipeline/status.go @@ -38,13 +38,16 @@ func (r *Reconciler) updateStatus(ctx context.Context, pipelineName string) erro // If the "AgentHealthy" type doesn't exist in the conditions, // then we need to reset the conditions list to ensure that the "Pending" and "Running" conditions are appended to the end of the conditions list // Check step 3 in https://github.com/kyma-project/telemetry-manager/blob/main/docs/contributor/arch/004-consolidate-pipeline-statuses.md#decision + // At the same time, we need to store the "Pending" condition to preserve the pending reason + var preUpgradePendingCondition *metav1.Condition if meta.FindStatusCondition(pipeline.Status.Conditions, conditions.TypeAgentHealthy) == nil { + preUpgradePendingCondition = meta.FindStatusCondition(pipeline.Status.Conditions, conditions.TypePending) pipeline.Status.Conditions = []metav1.Condition{} } r.setAgentHealthyCondition(ctx, &pipeline) r.setFluentBitConfigGeneratedCondition(ctx, &pipeline) - r.setPendingAndRunningConditions(ctx, &pipeline) + r.setPendingAndRunningConditions(ctx, &pipeline, preUpgradePendingCondition) if err := r.Status().Update(ctx, &pipeline); err != nil { return fmt.Errorf("failed to update LogPipeline status: %w", err) @@ -99,7 +102,7 @@ func (r *Reconciler) setFluentBitConfigGeneratedCondition(ctx context.Context, p meta.SetStatusCondition(&pipeline.Status.Conditions, conditions.New(conditions.TypeConfigurationGenerated, reason, status, pipeline.Generation, conditions.LogsMessage)) } -func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipeline *telemetryv1alpha1.LogPipeline) { +func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipeline *telemetryv1alpha1.LogPipeline, preUpgradePendingCondition *metav1.Condition) { if pipeline.Spec.Output.IsLokiDefined() { conditions.SetPendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonUnsupportedLokiOutput, pipeline.Name, conditions.LogsMessage) @@ -123,5 +126,14 @@ func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipelin return } - conditions.SetRunningCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonFluentBitDSReady, pipeline.Name, conditions.LogsMessage) + conditions.SetRunningCondition( + ctx, + &pipeline.Status.Conditions, + pipeline.Generation, + conditions.ReasonFluentBitDSReady, + pipeline.Name, + conditions.LogsMessage, + preUpgradePendingCondition, + conditions.ReasonFluentBitDSNotReady, + ) } diff --git a/internal/reconciler/tracepipeline/status.go b/internal/reconciler/tracepipeline/status.go index 84e5f7ea4..9e63854f4 100644 --- a/internal/reconciler/tracepipeline/status.go +++ b/internal/reconciler/tracepipeline/status.go @@ -31,16 +31,19 @@ func (r *Reconciler) updateStatus(ctx context.Context, pipelineName string, with return nil } - // If the "GatewayHealthy" type doesn't exist in the conditions, + // If the "GatewayHealthy" type doesn't exist in the conditions // then we need to reset the conditions list to ensure that the "Pending" and "Running" conditions are appended to the end of the conditions list // Check step 3 in https://github.com/kyma-project/telemetry-manager/blob/main/docs/contributor/arch/004-consolidate-pipeline-statuses.md#decision + // At the same time, we need to store the "Pending" condition to preserve the pending reason + var preUpgradePendingCondition *metav1.Condition if meta.FindStatusCondition(pipeline.Status.Conditions, conditions.TypeGatewayHealthy) == nil { + preUpgradePendingCondition = meta.FindStatusCondition(pipeline.Status.Conditions, conditions.TypePending) pipeline.Status.Conditions = []metav1.Condition{} } r.setGatewayHealthyCondition(ctx, &pipeline) r.setGatewayConfigGeneratedCondition(ctx, &pipeline, withinPipelineCountLimit) - r.setPendingAndRunningConditions(ctx, &pipeline, withinPipelineCountLimit) + r.setPendingAndRunningConditions(ctx, &pipeline, withinPipelineCountLimit, preUpgradePendingCondition) if err := r.Status().Update(ctx, &pipeline); err != nil { return fmt.Errorf("failed to update TracePipeline status: %w", err) @@ -83,7 +86,7 @@ func (r *Reconciler) setGatewayConfigGeneratedCondition(ctx context.Context, pip meta.SetStatusCondition(&pipeline.Status.Conditions, conditions.New(conditions.TypeConfigurationGenerated, reason, status, pipeline.Generation, conditions.TracesMessage)) } -func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipeline *telemetryv1alpha1.TracePipeline, withinPipelineCountLimit bool) { +func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipeline *telemetryv1alpha1.TracePipeline, withinPipelineCountLimit bool, preUpgradePendingCondition *metav1.Condition) { if !withinPipelineCountLimit { conditions.SetPendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonMaxPipelinesExceeded, pipeline.Name, conditions.TracesMessage) return @@ -106,5 +109,14 @@ func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipelin return } - conditions.SetRunningCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonTraceGatewayDeploymentReady, pipeline.Name, conditions.TracesMessage) + conditions.SetRunningCondition( + ctx, + &pipeline.Status.Conditions, + pipeline.Generation, + conditions.ReasonTraceGatewayDeploymentReady, + pipeline.Name, + conditions.TracesMessage, + preUpgradePendingCondition, + conditions.ReasonTraceGatewayDeploymentNotReady, + ) } From ce7d3101b69927ef5525a2ca8df637a8d2024b37 Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Thu, 14 Mar 2024 01:37:08 +0100 Subject: [PATCH 2/6] fix previous pending reason for a running pipeline --- internal/conditions/conditions.go | 19 +++++-------------- internal/reconciler/logparser/status.go | 10 +++------- internal/reconciler/logpipeline/status.go | 10 +++------- internal/reconciler/tracepipeline/status.go | 10 +++------- 4 files changed, 14 insertions(+), 35 deletions(-) diff --git a/internal/conditions/conditions.go b/internal/conditions/conditions.go index 533ddf1d2..0578b1bb6 100644 --- a/internal/conditions/conditions.go +++ b/internal/conditions/conditions.go @@ -127,34 +127,25 @@ func SetPendingCondition(ctx context.Context, conditions *[]metav1.Condition, ge meta.SetStatusCondition(conditions, pending) } -func SetRunningCondition(ctx context.Context, conditions *[]metav1.Condition, generation int64, reason, resourceName string, messageMap map[string]string, preUpgradePendingCondition *metav1.Condition, pendingFallbackReason string) { +func SetRunningCondition(ctx context.Context, conditions *[]metav1.Condition, generation int64, runningReason, pendingReason, resourceName string, messageMap map[string]string) { log := logf.FromContext(ctx) // Set Pending condition to False - var pendingReason string - existingPendingCondition := meta.FindStatusCondition(*conditions, TypePending) - if existingPendingCondition != nil { - pendingReason = existingPendingCondition.Reason - } else if preUpgradePendingCondition != nil { - pendingReason = preUpgradePendingCondition.Reason - } else { - pendingReason = pendingFallbackReason - } - newPending := New( + pending := New( TypePending, pendingReason, metav1.ConditionFalse, generation, messageMap, ) - newPending.Message = PendingTypeDeprecationMsg + newPending.Message + pending.Message = PendingTypeDeprecationMsg + pending.Message log.V(1).Info(fmt.Sprintf("Updating the status of %s: Setting the Pending condition to False", resourceName)) - meta.SetStatusCondition(conditions, newPending) + meta.SetStatusCondition(conditions, pending) // Set Running condition to True running := New( TypeRunning, - reason, + runningReason, metav1.ConditionTrue, generation, messageMap, diff --git a/internal/reconciler/logparser/status.go b/internal/reconciler/logparser/status.go index ae90cd99f..44aa28dfa 100644 --- a/internal/reconciler/logparser/status.go +++ b/internal/reconciler/logparser/status.go @@ -31,15 +31,12 @@ func (r *Reconciler) updateStatus(ctx context.Context, parserName string) error // If the "AgentHealthy" type doesn't exist in the conditions, // then we need to reset the conditions list to ensure that the "Pending" and "Running" conditions are appended to the end of the conditions list // Check step 3 in https://github.com/kyma-project/telemetry-manager/blob/main/docs/contributor/arch/004-consolidate-pipeline-statuses.md#decision - // At the same time, we need to store the "Pending" condition to preserve the pending reason - var preUpgradePendingCondition *metav1.Condition if meta.FindStatusCondition(parser.Status.Conditions, conditions.TypeAgentHealthy) == nil { - preUpgradePendingCondition = meta.FindStatusCondition(parser.Status.Conditions, conditions.TypePending) parser.Status.Conditions = []metav1.Condition{} } r.setAgentHealthyCondition(ctx, &parser) - r.setPendingAndRunningConditions(ctx, &parser, preUpgradePendingCondition) + r.setPendingAndRunningConditions(ctx, &parser) if err := r.Status().Update(ctx, &parser); err != nil { return fmt.Errorf("failed to update LogParser status: %w", err) @@ -65,7 +62,7 @@ func (r *Reconciler) setAgentHealthyCondition(ctx context.Context, parser *telem meta.SetStatusCondition(&parser.Status.Conditions, conditions.New(conditions.TypeAgentHealthy, reason, status, parser.Generation, conditions.LogsMessage)) } -func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, parser *telemetryv1alpha1.LogParser, preUpgradePendingCondition *metav1.Condition) { +func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, parser *telemetryv1alpha1.LogParser) { fluentBitReady, err := r.prober.IsReady(ctx, r.config.DaemonSet) if err != nil { logf.FromContext(ctx).V(1).Error(err, "Failed to probe fluent bit daemonset") @@ -83,9 +80,8 @@ func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, parser &parser.Status.Conditions, parser.Generation, conditions.ReasonFluentBitDSReady, + conditions.ReasonFluentBitDSNotReady, parser.Name, conditions.LogsMessage, - preUpgradePendingCondition, - conditions.ReasonFluentBitDSNotReady, ) } diff --git a/internal/reconciler/logpipeline/status.go b/internal/reconciler/logpipeline/status.go index d9d6b3bdb..4a062a8f0 100644 --- a/internal/reconciler/logpipeline/status.go +++ b/internal/reconciler/logpipeline/status.go @@ -38,16 +38,13 @@ func (r *Reconciler) updateStatus(ctx context.Context, pipelineName string) erro // If the "AgentHealthy" type doesn't exist in the conditions, // then we need to reset the conditions list to ensure that the "Pending" and "Running" conditions are appended to the end of the conditions list // Check step 3 in https://github.com/kyma-project/telemetry-manager/blob/main/docs/contributor/arch/004-consolidate-pipeline-statuses.md#decision - // At the same time, we need to store the "Pending" condition to preserve the pending reason - var preUpgradePendingCondition *metav1.Condition if meta.FindStatusCondition(pipeline.Status.Conditions, conditions.TypeAgentHealthy) == nil { - preUpgradePendingCondition = meta.FindStatusCondition(pipeline.Status.Conditions, conditions.TypePending) pipeline.Status.Conditions = []metav1.Condition{} } r.setAgentHealthyCondition(ctx, &pipeline) r.setFluentBitConfigGeneratedCondition(ctx, &pipeline) - r.setPendingAndRunningConditions(ctx, &pipeline, preUpgradePendingCondition) + r.setPendingAndRunningConditions(ctx, &pipeline) if err := r.Status().Update(ctx, &pipeline); err != nil { return fmt.Errorf("failed to update LogPipeline status: %w", err) @@ -102,7 +99,7 @@ func (r *Reconciler) setFluentBitConfigGeneratedCondition(ctx context.Context, p meta.SetStatusCondition(&pipeline.Status.Conditions, conditions.New(conditions.TypeConfigurationGenerated, reason, status, pipeline.Generation, conditions.LogsMessage)) } -func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipeline *telemetryv1alpha1.LogPipeline, preUpgradePendingCondition *metav1.Condition) { +func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipeline *telemetryv1alpha1.LogPipeline) { if pipeline.Spec.Output.IsLokiDefined() { conditions.SetPendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonUnsupportedLokiOutput, pipeline.Name, conditions.LogsMessage) @@ -131,9 +128,8 @@ func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipelin &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonFluentBitDSReady, + conditions.ReasonFluentBitDSNotReady, pipeline.Name, conditions.LogsMessage, - preUpgradePendingCondition, - conditions.ReasonFluentBitDSNotReady, ) } diff --git a/internal/reconciler/tracepipeline/status.go b/internal/reconciler/tracepipeline/status.go index 9e63854f4..6f656d551 100644 --- a/internal/reconciler/tracepipeline/status.go +++ b/internal/reconciler/tracepipeline/status.go @@ -34,16 +34,13 @@ func (r *Reconciler) updateStatus(ctx context.Context, pipelineName string, with // If the "GatewayHealthy" type doesn't exist in the conditions // then we need to reset the conditions list to ensure that the "Pending" and "Running" conditions are appended to the end of the conditions list // Check step 3 in https://github.com/kyma-project/telemetry-manager/blob/main/docs/contributor/arch/004-consolidate-pipeline-statuses.md#decision - // At the same time, we need to store the "Pending" condition to preserve the pending reason - var preUpgradePendingCondition *metav1.Condition if meta.FindStatusCondition(pipeline.Status.Conditions, conditions.TypeGatewayHealthy) == nil { - preUpgradePendingCondition = meta.FindStatusCondition(pipeline.Status.Conditions, conditions.TypePending) pipeline.Status.Conditions = []metav1.Condition{} } r.setGatewayHealthyCondition(ctx, &pipeline) r.setGatewayConfigGeneratedCondition(ctx, &pipeline, withinPipelineCountLimit) - r.setPendingAndRunningConditions(ctx, &pipeline, withinPipelineCountLimit, preUpgradePendingCondition) + r.setPendingAndRunningConditions(ctx, &pipeline, withinPipelineCountLimit) if err := r.Status().Update(ctx, &pipeline); err != nil { return fmt.Errorf("failed to update TracePipeline status: %w", err) @@ -86,7 +83,7 @@ func (r *Reconciler) setGatewayConfigGeneratedCondition(ctx context.Context, pip meta.SetStatusCondition(&pipeline.Status.Conditions, conditions.New(conditions.TypeConfigurationGenerated, reason, status, pipeline.Generation, conditions.TracesMessage)) } -func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipeline *telemetryv1alpha1.TracePipeline, withinPipelineCountLimit bool, preUpgradePendingCondition *metav1.Condition) { +func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipeline *telemetryv1alpha1.TracePipeline, withinPipelineCountLimit bool) { if !withinPipelineCountLimit { conditions.SetPendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonMaxPipelinesExceeded, pipeline.Name, conditions.TracesMessage) return @@ -114,9 +111,8 @@ func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipelin &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonTraceGatewayDeploymentReady, + conditions.ReasonTraceGatewayDeploymentNotReady, pipeline.Name, conditions.TracesMessage, - preUpgradePendingCondition, - conditions.ReasonTraceGatewayDeploymentNotReady, ) } From a93762eb3765843b49334b5ffcb3635ff36d92ce Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Thu, 14 Mar 2024 01:50:28 +0100 Subject: [PATCH 3/6] rename SetPendingCondition func to HandlePendingCondition --- internal/conditions/conditions.go | 2 +- internal/conditions/conditions_test.go | 4 ++-- internal/reconciler/logparser/status.go | 2 +- internal/reconciler/logpipeline/status.go | 6 +++--- internal/reconciler/tracepipeline/status.go | 6 +++--- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/conditions/conditions.go b/internal/conditions/conditions.go index 0578b1bb6..e70b91ecd 100644 --- a/internal/conditions/conditions.go +++ b/internal/conditions/conditions.go @@ -106,7 +106,7 @@ func MessageFor(reason string, messageMap map[string]string) string { return "" } -func SetPendingCondition(ctx context.Context, conditions *[]metav1.Condition, generation int64, reason, resourceName string, messageMap map[string]string) { +func HandlePendingCondition(ctx context.Context, conditions *[]metav1.Condition, generation int64, reason, resourceName string, messageMap map[string]string) { log := logf.FromContext(ctx) pending := New( diff --git a/internal/conditions/conditions_test.go b/internal/conditions/conditions_test.go index b89d530f6..6c5246bc7 100644 --- a/internal/conditions/conditions_test.go +++ b/internal/conditions/conditions_test.go @@ -38,7 +38,7 @@ func TestSetPendingCondition(t *testing.T) { generation := int64(1) reason := ReasonFluentBitDSNotReady - SetPendingCondition(context.Background(), &conditions, generation, reason, "pipeline", LogsMessage) + HandlePendingCondition(context.Background(), &conditions, generation, reason, "pipeline", LogsMessage) pendingCond := meta.FindStatusCondition(conditions, TypePending) require.Equal(t, TypePending, pendingCond.Type) @@ -70,7 +70,7 @@ func TestSetPendingCondition(t *testing.T) { generation := int64(1) reason := ReasonFluentBitDSNotReady - SetPendingCondition(context.Background(), &conditions, generation, reason, "pipeline", LogsMessage) + HandlePendingCondition(context.Background(), &conditions, generation, reason, "pipeline", LogsMessage) runningCond := meta.FindStatusCondition(conditions, TypeRunning) require.Nil(t, runningCond) diff --git a/internal/reconciler/logparser/status.go b/internal/reconciler/logparser/status.go index 44aa28dfa..e3f053df2 100644 --- a/internal/reconciler/logparser/status.go +++ b/internal/reconciler/logparser/status.go @@ -70,7 +70,7 @@ func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, parser } if !fluentBitReady { - conditions.SetPendingCondition(ctx, &parser.Status.Conditions, parser.Generation, conditions.ReasonFluentBitDSNotReady, parser.Name, conditions.LogsMessage) + conditions.HandlePendingCondition(ctx, &parser.Status.Conditions, parser.Generation, conditions.ReasonFluentBitDSNotReady, parser.Name, conditions.LogsMessage) return } diff --git a/internal/reconciler/logpipeline/status.go b/internal/reconciler/logpipeline/status.go index 4a062a8f0..f9e92ea83 100644 --- a/internal/reconciler/logpipeline/status.go +++ b/internal/reconciler/logpipeline/status.go @@ -102,13 +102,13 @@ func (r *Reconciler) setFluentBitConfigGeneratedCondition(ctx context.Context, p func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipeline *telemetryv1alpha1.LogPipeline) { if pipeline.Spec.Output.IsLokiDefined() { - conditions.SetPendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonUnsupportedLokiOutput, pipeline.Name, conditions.LogsMessage) + conditions.HandlePendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonUnsupportedLokiOutput, pipeline.Name, conditions.LogsMessage) return } referencesNonExistentSecret := secretref.ReferencesNonExistentSecret(ctx, r.Client, pipeline) if referencesNonExistentSecret { - conditions.SetPendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonReferencedSecretMissing, pipeline.Name, conditions.LogsMessage) + conditions.HandlePendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonReferencedSecretMissing, pipeline.Name, conditions.LogsMessage) return } @@ -119,7 +119,7 @@ func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipelin } if !fluentBitReady { - conditions.SetPendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonFluentBitDSNotReady, pipeline.Name, conditions.LogsMessage) + conditions.HandlePendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonFluentBitDSNotReady, pipeline.Name, conditions.LogsMessage) return } diff --git a/internal/reconciler/tracepipeline/status.go b/internal/reconciler/tracepipeline/status.go index 6f656d551..3545a00fb 100644 --- a/internal/reconciler/tracepipeline/status.go +++ b/internal/reconciler/tracepipeline/status.go @@ -85,13 +85,13 @@ func (r *Reconciler) setGatewayConfigGeneratedCondition(ctx context.Context, pip func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipeline *telemetryv1alpha1.TracePipeline, withinPipelineCountLimit bool) { if !withinPipelineCountLimit { - conditions.SetPendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonMaxPipelinesExceeded, pipeline.Name, conditions.TracesMessage) + conditions.HandlePendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonMaxPipelinesExceeded, pipeline.Name, conditions.TracesMessage) return } referencesNonExistentSecret := secretref.ReferencesNonExistentSecret(ctx, r.Client, pipeline) if referencesNonExistentSecret { - conditions.SetPendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonReferencedSecretMissing, pipeline.Name, conditions.TracesMessage) + conditions.HandlePendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonReferencedSecretMissing, pipeline.Name, conditions.TracesMessage) return } @@ -102,7 +102,7 @@ func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipelin } if !gatewayReady { - conditions.SetPendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonTraceGatewayDeploymentNotReady, pipeline.Name, conditions.TracesMessage) + conditions.HandlePendingCondition(ctx, &pipeline.Status.Conditions, pipeline.Generation, conditions.ReasonTraceGatewayDeploymentNotReady, pipeline.Name, conditions.TracesMessage) return } From 2e2acdf6092e05bb83445b2007864ec0d09f4003 Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Thu, 14 Mar 2024 01:51:31 +0100 Subject: [PATCH 4/6] rename SetRunningCondition func to HandleRunningCondition --- internal/conditions/conditions.go | 2 +- internal/conditions/conditions_test.go | 2 +- internal/reconciler/logparser/status.go | 2 +- internal/reconciler/logpipeline/status.go | 2 +- internal/reconciler/tracepipeline/status.go | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/conditions/conditions.go b/internal/conditions/conditions.go index e70b91ecd..7869cdc5a 100644 --- a/internal/conditions/conditions.go +++ b/internal/conditions/conditions.go @@ -127,7 +127,7 @@ func HandlePendingCondition(ctx context.Context, conditions *[]metav1.Condition, meta.SetStatusCondition(conditions, pending) } -func SetRunningCondition(ctx context.Context, conditions *[]metav1.Condition, generation int64, runningReason, pendingReason, resourceName string, messageMap map[string]string) { +func HandleRunningCondition(ctx context.Context, conditions *[]metav1.Condition, generation int64, runningReason, pendingReason, resourceName string, messageMap map[string]string) { log := logf.FromContext(ctx) // Set Pending condition to False diff --git a/internal/conditions/conditions_test.go b/internal/conditions/conditions_test.go index 6c5246bc7..775aec607 100644 --- a/internal/conditions/conditions_test.go +++ b/internal/conditions/conditions_test.go @@ -101,7 +101,7 @@ func TestSetRunningCondition(t *testing.T) { generation := int64(1) reason := ReasonFluentBitDSReady - SetRunningCondition(context.Background(), &conditions, generation, reason, "pipeline", LogsMessage) + HandleRunningCondition(context.Background(), &conditions, generation, reason, "pipeline", LogsMessage) pendingCond := meta.FindStatusCondition(conditions, TypePending) require.NotNil(t, pendingCond) diff --git a/internal/reconciler/logparser/status.go b/internal/reconciler/logparser/status.go index e3f053df2..6332b106b 100644 --- a/internal/reconciler/logparser/status.go +++ b/internal/reconciler/logparser/status.go @@ -75,7 +75,7 @@ func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, parser } - conditions.SetRunningCondition( + conditions.HandleRunningCondition( ctx, &parser.Status.Conditions, parser.Generation, diff --git a/internal/reconciler/logpipeline/status.go b/internal/reconciler/logpipeline/status.go index f9e92ea83..6a35728d6 100644 --- a/internal/reconciler/logpipeline/status.go +++ b/internal/reconciler/logpipeline/status.go @@ -123,7 +123,7 @@ func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipelin return } - conditions.SetRunningCondition( + conditions.HandleRunningCondition( ctx, &pipeline.Status.Conditions, pipeline.Generation, diff --git a/internal/reconciler/tracepipeline/status.go b/internal/reconciler/tracepipeline/status.go index 3545a00fb..6497462dc 100644 --- a/internal/reconciler/tracepipeline/status.go +++ b/internal/reconciler/tracepipeline/status.go @@ -106,7 +106,7 @@ func (r *Reconciler) setPendingAndRunningConditions(ctx context.Context, pipelin return } - conditions.SetRunningCondition( + conditions.HandleRunningCondition( ctx, &pipeline.Status.Conditions, pipeline.Generation, From 141247e6a8568266c479441daf7e97534ce46957 Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Thu, 14 Mar 2024 06:09:43 +0100 Subject: [PATCH 5/6] update unit tests --- internal/conditions/conditions_test.go | 80 ++++++++++++++----- internal/reconciler/logparser/status_test.go | 10 +++ .../reconciler/logpipeline/status_test.go | 10 +++ .../reconciler/tracepipeline/status_test.go | 10 +++ 4 files changed, 89 insertions(+), 21 deletions(-) diff --git a/internal/conditions/conditions_test.go b/internal/conditions/conditions_test.go index 775aec607..4736f689c 100644 --- a/internal/conditions/conditions_test.go +++ b/internal/conditions/conditions_test.go @@ -32,15 +32,31 @@ func TestMessageFor(t *testing.T) { }) } -func TestSetPendingCondition(t *testing.T) { - t.Run("should just add pending condition if the conditions list is empty", func(t *testing.T) { - var conditions []metav1.Condition +func TestHandlePendingCondition(t *testing.T) { + t.Run("should just set pending condition to true if running condition is not in the conditions list", func(t *testing.T) { + conditions := []metav1.Condition{ + { + Type: TypeAgentHealthy, + Status: metav1.ConditionFalse, + Reason: ReasonDaemonSetNotReady, + Message: MessageFor(ReasonDaemonSetNotReady, LogsMessage), + LastTransitionTime: metav1.Now(), + }, + { + Type: TypeConfigurationGenerated, + Status: metav1.ConditionTrue, + Reason: ReasonConfigurationGenerated, + Message: MessageFor(ReasonConfigurationGenerated, LogsMessage), + LastTransitionTime: metav1.Now(), + }, + } generation := int64(1) reason := ReasonFluentBitDSNotReady HandlePendingCondition(context.Background(), &conditions, generation, reason, "pipeline", LogsMessage) - pendingCond := meta.FindStatusCondition(conditions, TypePending) + conditionsSize := len(conditions) + pendingCond := conditions[conditionsSize-1] require.Equal(t, TypePending, pendingCond.Type) require.Equal(t, metav1.ConditionTrue, pendingCond.Status) require.Equal(t, reason, pendingCond.Reason) @@ -52,6 +68,20 @@ func TestSetPendingCondition(t *testing.T) { t.Run("should remove running condition and set pending condition to true", func(t *testing.T) { conditions := []metav1.Condition{ + { + Type: TypeAgentHealthy, + Status: metav1.ConditionFalse, + Reason: ReasonDaemonSetNotReady, + Message: MessageFor(ReasonDaemonSetNotReady, LogsMessage), + LastTransitionTime: metav1.Now(), + }, + { + Type: TypeConfigurationGenerated, + Status: metav1.ConditionTrue, + Reason: ReasonConfigurationGenerated, + Message: MessageFor(ReasonConfigurationGenerated, LogsMessage), + LastTransitionTime: metav1.Now(), + }, { Type: TypePending, Status: metav1.ConditionFalse, @@ -75,8 +105,8 @@ func TestSetPendingCondition(t *testing.T) { runningCond := meta.FindStatusCondition(conditions, TypeRunning) require.Nil(t, runningCond) - pendingCond := meta.FindStatusCondition(conditions, TypePending) - require.NotNil(t, pendingCond) + conditionsSize := len(conditions) + pendingCond := conditions[conditionsSize-1] require.Equal(t, TypePending, pendingCond.Type) require.Equal(t, metav1.ConditionTrue, pendingCond.Status) require.Equal(t, reason, pendingCond.Reason) @@ -87,38 +117,46 @@ func TestSetPendingCondition(t *testing.T) { }) } -func TestSetRunningCondition(t *testing.T) { - t.Run("should set pending condition to false and add running condition", func(t *testing.T) { +func TestHandleRunningCondition(t *testing.T) { + t.Run("should set pending condition to false and set running condition to true", func(t *testing.T) { conditions := []metav1.Condition{ { - Type: TypePending, + Type: TypeAgentHealthy, Status: metav1.ConditionTrue, - Reason: ReasonFluentBitDSNotReady, - Message: PendingTypeDeprecationMsg + MessageFor(ReasonFluentBitDSNotReady, LogsMessage), + Reason: ReasonDaemonSetReady, + Message: MessageFor(ReasonDaemonSetReady, LogsMessage), + LastTransitionTime: metav1.Now(), + }, + { + Type: TypeConfigurationGenerated, + Status: metav1.ConditionTrue, + Reason: ReasonConfigurationGenerated, + Message: MessageFor(ReasonConfigurationGenerated, LogsMessage), LastTransitionTime: metav1.Now(), }, } generation := int64(1) - reason := ReasonFluentBitDSReady + runningReason := ReasonFluentBitDSReady + pendingReason := ReasonFluentBitDSNotReady + + HandleRunningCondition(context.Background(), &conditions, generation, runningReason, pendingReason, "pipeline", LogsMessage) - HandleRunningCondition(context.Background(), &conditions, generation, reason, "pipeline", LogsMessage) + conditionsSize := len(conditions) - pendingCond := meta.FindStatusCondition(conditions, TypePending) - require.NotNil(t, pendingCond) + pendingCond := conditions[conditionsSize-2] require.Equal(t, TypePending, pendingCond.Type) require.Equal(t, metav1.ConditionFalse, pendingCond.Status) - require.Equal(t, ReasonFluentBitDSNotReady, pendingCond.Reason) - pendingCondMsg := PendingTypeDeprecationMsg + MessageFor(ReasonFluentBitDSNotReady, LogsMessage) + require.Equal(t, pendingReason, pendingCond.Reason) + pendingCondMsg := PendingTypeDeprecationMsg + MessageFor(pendingReason, LogsMessage) require.Equal(t, pendingCondMsg, pendingCond.Message) require.Equal(t, generation, pendingCond.ObservedGeneration) require.NotEmpty(t, pendingCond.LastTransitionTime) - runningCond := meta.FindStatusCondition(conditions, TypeRunning) - require.NotNil(t, runningCond) + runningCond := conditions[conditionsSize-1] require.Equal(t, TypeRunning, runningCond.Type) require.Equal(t, metav1.ConditionTrue, runningCond.Status) - require.Equal(t, reason, runningCond.Reason) - runningCondMsg := RunningTypeDeprecationMsg + MessageFor(reason, LogsMessage) + require.Equal(t, runningReason, runningCond.Reason) + runningCondMsg := RunningTypeDeprecationMsg + MessageFor(runningReason, LogsMessage) require.Equal(t, runningCondMsg, runningCond.Message) require.Equal(t, generation, runningCond.ObservedGeneration) require.NotEmpty(t, runningCond.LastTransitionTime) diff --git a/internal/reconciler/logparser/status_test.go b/internal/reconciler/logparser/status_test.go index b37102020..bd8d01500 100644 --- a/internal/reconciler/logparser/status_test.go +++ b/internal/reconciler/logparser/status_test.go @@ -121,6 +121,16 @@ func TestUpdateStatus(t *testing.T) { require.NotEmpty(t, agentHealthyCond.LastTransitionTime) conditionsSize := len(updatedParser.Status.Conditions) + + pendingCond := updatedParser.Status.Conditions[conditionsSize-2] + require.Equal(t, conditions.TypePending, pendingCond.Type) + require.Equal(t, metav1.ConditionFalse, pendingCond.Status) + require.Equal(t, conditions.ReasonFluentBitDSNotReady, pendingCond.Reason) + pendingCondMsg := conditions.PendingTypeDeprecationMsg + conditions.MessageFor(conditions.ReasonFluentBitDSNotReady, conditions.LogsMessage) + require.Equal(t, pendingCondMsg, pendingCond.Message) + require.Equal(t, updatedParser.Generation, pendingCond.ObservedGeneration) + require.NotEmpty(t, pendingCond.LastTransitionTime) + runningCond := updatedParser.Status.Conditions[conditionsSize-1] require.Equal(t, conditions.TypeRunning, runningCond.Type) require.Equal(t, metav1.ConditionTrue, runningCond.Status) diff --git a/internal/reconciler/logpipeline/status_test.go b/internal/reconciler/logpipeline/status_test.go index cfe4376dd..e83e34f12 100644 --- a/internal/reconciler/logpipeline/status_test.go +++ b/internal/reconciler/logpipeline/status_test.go @@ -121,6 +121,16 @@ func TestUpdateStatus(t *testing.T) { require.NotEmpty(t, agentHealthyCond.LastTransitionTime) conditionsSize := len(updatedPipeline.Status.Conditions) + + pendingCond := updatedPipeline.Status.Conditions[conditionsSize-2] + require.Equal(t, conditions.TypePending, pendingCond.Type) + require.Equal(t, metav1.ConditionFalse, pendingCond.Status) + require.Equal(t, conditions.ReasonFluentBitDSNotReady, pendingCond.Reason) + pendingCondMsg := conditions.PendingTypeDeprecationMsg + conditions.MessageFor(conditions.ReasonFluentBitDSNotReady, conditions.LogsMessage) + require.Equal(t, pendingCondMsg, pendingCond.Message) + require.Equal(t, updatedPipeline.Generation, pendingCond.ObservedGeneration) + require.NotEmpty(t, pendingCond.LastTransitionTime) + runningCond := updatedPipeline.Status.Conditions[conditionsSize-1] require.Equal(t, conditions.TypeRunning, runningCond.Type) require.Equal(t, metav1.ConditionTrue, runningCond.Status) diff --git a/internal/reconciler/tracepipeline/status_test.go b/internal/reconciler/tracepipeline/status_test.go index 47e5caee6..28a39ba34 100644 --- a/internal/reconciler/tracepipeline/status_test.go +++ b/internal/reconciler/tracepipeline/status_test.go @@ -120,6 +120,16 @@ func TestUpdateStatus(t *testing.T) { require.NotEmpty(t, gatewayHealthyCond.LastTransitionTime) conditionsSize := len(updatedPipeline.Status.Conditions) + + pendingCond := updatedPipeline.Status.Conditions[conditionsSize-2] + require.Equal(t, conditions.TypePending, pendingCond.Type) + require.Equal(t, metav1.ConditionFalse, pendingCond.Status) + require.Equal(t, conditions.ReasonTraceGatewayDeploymentNotReady, pendingCond.Reason) + pendingCondMsg := conditions.PendingTypeDeprecationMsg + conditions.MessageFor(conditions.ReasonTraceGatewayDeploymentNotReady, conditions.TracesMessage) + require.Equal(t, pendingCondMsg, pendingCond.Message) + require.Equal(t, updatedPipeline.Generation, pendingCond.ObservedGeneration) + require.NotEmpty(t, pendingCond.LastTransitionTime) + runningCond := updatedPipeline.Status.Conditions[conditionsSize-1] require.Equal(t, conditions.TypeRunning, runningCond.Type) require.Equal(t, metav1.ConditionTrue, runningCond.Status) From 5e917d1f57c6b6ee0afde733eb45b11ca00351b0 Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Thu, 14 Mar 2024 06:13:40 +0100 Subject: [PATCH 6/6] update e2e verifiers --- test/testkit/verifiers/logs.go | 3 ++- test/testkit/verifiers/traces.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/testkit/verifiers/logs.go b/test/testkit/verifiers/logs.go index 9b12bf841..b1a40e2ce 100644 --- a/test/testkit/verifiers/logs.go +++ b/test/testkit/verifiers/logs.go @@ -34,8 +34,9 @@ func LogPipelineShouldBeHealthy(ctx context.Context, k8sClient client.Client, pi var pipeline telemetryv1alpha1.LogPipeline key := types.NamespacedName{Name: pipelineName} g.Expect(k8sClient.Get(ctx, key, &pipeline)).To(Succeed()) - g.Expect(meta.IsStatusConditionTrue(pipeline.Status.Conditions, conditions.TypeRunning)).To(BeTrue()) g.Expect(meta.IsStatusConditionTrue(pipeline.Status.Conditions, conditions.TypeAgentHealthy)).To(BeTrue()) g.Expect(meta.IsStatusConditionTrue(pipeline.Status.Conditions, conditions.TypeConfigurationGenerated)).To(BeTrue()) + g.Expect(meta.IsStatusConditionTrue(pipeline.Status.Conditions, conditions.TypeRunning)).To(BeTrue()) + g.Expect(meta.IsStatusConditionFalse(pipeline.Status.Conditions, conditions.TypePending)).To(BeTrue()) }, periodic.EventuallyTimeout, periodic.DefaultInterval).Should(Succeed()) } diff --git a/test/testkit/verifiers/traces.go b/test/testkit/verifiers/traces.go index 31828ce34..cdda1d8d0 100644 --- a/test/testkit/verifiers/traces.go +++ b/test/testkit/verifiers/traces.go @@ -26,9 +26,10 @@ func TracePipelineShouldBeHealthy(ctx context.Context, k8sClient client.Client, var pipeline telemetryv1alpha1.TracePipeline key := types.NamespacedName{Name: pipelineName} g.Expect(k8sClient.Get(ctx, key, &pipeline)).To(Succeed()) - g.Expect(meta.IsStatusConditionTrue(pipeline.Status.Conditions, conditions.TypeRunning)).To(BeTrue()) g.Expect(meta.IsStatusConditionTrue(pipeline.Status.Conditions, conditions.TypeGatewayHealthy)).To(BeTrue()) g.Expect(meta.IsStatusConditionTrue(pipeline.Status.Conditions, conditions.TypeConfigurationGenerated)).To(BeTrue()) + g.Expect(meta.IsStatusConditionTrue(pipeline.Status.Conditions, conditions.TypeRunning)).To(BeTrue()) + g.Expect(meta.IsStatusConditionFalse(pipeline.Status.Conditions, conditions.TypePending)).To(BeTrue()) }, periodic.EventuallyTimeout, periodic.DefaultInterval).Should(Succeed()) }