Skip to content

Commit

Permalink
remove unnecessary test, move some to sync_test UT
Browse files Browse the repository at this point in the history
  • Loading branch information
@jeffreylimnardy
jeffreylimnardy committed Jan 30, 2025
1 parent 167007c commit 3980385
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 159 deletions.
161 changes: 2 additions & 159 deletions internal/reconciler/logpipeline/fluentbit/reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,13 @@ import (
"context"
"errors"
"fmt"
"github.com/kyma-project/telemetry-manager/internal/reconciler/logpipeline/fluentbit/mocks"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
networkingv1 "k8s.io/api/networking/v1"
rbacv1 "k8s.io/api/rbac/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -28,6 +23,7 @@ import (
"github.com/kyma-project/telemetry-manager/internal/errortypes"
"github.com/kyma-project/telemetry-manager/internal/overrides"
commonStatusStubs "github.com/kyma-project/telemetry-manager/internal/reconciler/commonstatus/stubs"
"github.com/kyma-project/telemetry-manager/internal/reconciler/logpipeline/fluentbit/mocks"
logpipelinemocks "github.com/kyma-project/telemetry-manager/internal/reconciler/logpipeline/mocks"
"github.com/kyma-project/telemetry-manager/internal/reconciler/logpipeline/stubs"
"github.com/kyma-project/telemetry-manager/internal/resources/fluentbit"
Expand Down Expand Up @@ -156,11 +152,6 @@ func TestReconcile(t *testing.T) {
require.NoError(t, fakeClient.Get(context.Background(), types.NamespacedName{Name: pipeline.Name}, &pl1))
err := sut.Reconcile(context.Background(), &pl1)
require.NoError(t, err)

// check Fluent Bit sections configmap as an indicator of resources generation
cm := &corev1.ConfigMap{}
err = fakeClient.Get(context.Background(), testConfig.SectionsConfigMap, cm)
require.True(t, apierrors.IsNotFound(err), "sections configmap should not exist")
})

t.Run("log agent is not ready", func(t *testing.T) {
Expand Down Expand Up @@ -202,11 +193,6 @@ func TestReconcile(t *testing.T) {
conditions.ReasonAgentNotReady,
workloadstatus.ErrDaemonSetNotFound.Error(),
)

var cm corev1.ConfigMap
err = fakeClient.Get(context.Background(), testConfig.SectionsConfigMap, &cm)
require.NoError(t, err, "sections configmap must exist")
require.Contains(t, cm.Data[pipeline.Name+".conf"], pipeline.Name, "sections configmap must contain pipeline name")
})

t.Run("log agent is ready", func(t *testing.T) {
Expand Down Expand Up @@ -247,11 +233,6 @@ func TestReconcile(t *testing.T) {
conditions.ReasonAgentReady,
"Fluent Bit agent DaemonSet is ready",
)

var cm corev1.ConfigMap
err = fakeClient.Get(context.Background(), testConfig.SectionsConfigMap, &cm)
require.NoError(t, err, "sections configmap must exist")
require.Contains(t, cm.Data[pipeline.Name+".conf"], pipeline.Name, "sections configmap must contain pipeline name")
})

t.Run("log agent prober fails", func(t *testing.T) {
Expand Down Expand Up @@ -292,11 +273,6 @@ func TestReconcile(t *testing.T) {
conditions.ReasonAgentNotReady,
"Failed to get DaemonSet",
)

var cm corev1.ConfigMap
err = fakeClient.Get(context.Background(), testConfig.SectionsConfigMap, &cm)
require.NoError(t, err, "sections configmap must exist")
require.Contains(t, cm.Data[pipeline.Name+".conf"], pipeline.Name, "sections configmap must contain pipeline name")
})

t.Run("referenced secret missing", func(t *testing.T) {
Expand Down Expand Up @@ -347,40 +323,6 @@ func TestReconcile(t *testing.T) {
conditions.ReasonSelfMonConfigNotGenerated,
"No logs delivered to backend because LogPipeline specification is not applied to the configuration of Fluent Bit agent. Check the 'ConfigurationGenerated' condition for more details",
)

name := types.NamespacedName{Name: testConfig.DaemonSet.Name, Namespace: testConfig.DaemonSet.Namespace}

var cm corev1.ConfigMap
err = fakeClient.Get(context.Background(), testConfig.SectionsConfigMap, &cm)
require.Error(t, err, "sections configmap should not exist")

var cmLua corev1.ConfigMap
err = fakeClient.Get(context.Background(), testConfig.LuaConfigMap, &cmLua)
require.Error(t, err, "lua configmap should not exist")

var cmParser corev1.ConfigMap
err = fakeClient.Get(context.Background(), testConfig.ParsersConfigMap, &cmParser)
require.Error(t, err, "parser configmap should not exist")

var serviceAccount corev1.ServiceAccount
err = fakeClient.Get(context.Background(), name, &serviceAccount)
require.Error(t, err, "service account should not exist")

var clusterRole rbacv1.ClusterRole
err = fakeClient.Get(context.Background(), name, &clusterRole)
require.Error(t, err, "clusterrole should not exist")

var clusterRoleBinding rbacv1.ClusterRoleBinding
err = fakeClient.Get(context.Background(), name, &clusterRoleBinding)
require.Error(t, err, "clusterrolebinding should not exist")

var daemonSet appsv1.DaemonSet
err = fakeClient.Get(context.Background(), name, &daemonSet)
require.Error(t, err, "daemonset should not exist")

var networkPolicy networkingv1.NetworkPolicy
err = fakeClient.Get(context.Background(), name, &networkPolicy)
require.Error(t, err, "network policy should not exist")
})

t.Run("referenced secret exists", func(t *testing.T) {
Expand Down Expand Up @@ -433,11 +375,6 @@ func TestReconcile(t *testing.T) {
conditions.ReasonAgentConfigured,
"LogPipeline specification is successfully applied to the configuration of Fluent Bit agent",
)

var cm corev1.ConfigMap
err = fakeClient.Get(context.Background(), testConfig.SectionsConfigMap, &cm)
require.NoError(t, err, "sections configmap must exist")
require.Contains(t, cm.Data[pipeline.Name+".conf"], pipeline.Name, "sections configmap must contain pipeline name")
})

t.Run("flow healthy", func(t *testing.T) {
Expand Down Expand Up @@ -575,11 +512,6 @@ func TestReconcile(t *testing.T) {
tt.expectedReason,
tt.expectedMessage,
)

var cm corev1.ConfigMap
err = fakeClient.Get(context.Background(), testConfig.SectionsConfigMap, &cm)
require.NoError(t, err, "sections configmap must exist")
require.Contains(t, cm.Data[pipeline.Name+".conf"], pipeline.Name, "sections configmap must contain pipeline name")
})
}
})
Expand Down Expand Up @@ -711,16 +643,6 @@ func TestReconcile(t *testing.T) {
"No logs delivered to backend because LogPipeline specification is not applied to the configuration of Fluent Bit agent. Check the 'ConfigurationGenerated' condition for more details",
)
}

var cm corev1.ConfigMap

err = fakeClient.Get(context.Background(), testConfig.SectionsConfigMap, &cm)
if !tt.expectAgentConfigured {
require.Error(t, err, "sections configmap should not exist")
} else {
require.NoError(t, err, "sections configmap must exist")
require.Contains(t, cm.Data[pipeline.Name+".conf"], pipeline.Name, "sections configmap must contain pipeline name")
}
})
}
})
Expand Down Expand Up @@ -799,11 +721,6 @@ func TestReconcile(t *testing.T) {
require.Equal(t, tt.expectedReason, cond.Reason)

require.Equal(t, tt.expectedMessage, cond.Message)

var cm corev1.ConfigMap
err = fakeClient.Get(context.Background(), testConfig.SectionsConfigMap, &cm)
require.NoError(t, err, "sections configmap must exist")
require.Contains(t, cm.Data[pipeline.Name+".conf"], pipeline.Name, "sections configmap must contain pipeline name")
})
}
})
Expand Down Expand Up @@ -856,80 +773,6 @@ func TestReconcile(t *testing.T) {
conditions.ReasonSelfMonConfigNotGenerated,
"No logs delivered to backend because LogPipeline specification is not applied to the configuration of Fluent Bit agent. Check the 'ConfigurationGenerated' condition for more details",
)

var cm corev1.ConfigMap
err = fakeClient.Get(context.Background(), testConfig.SectionsConfigMap, &cm)
require.Error(t, err, "sections configmap should not exist")
})

t.Run("create 2 pipelines and delete 1 should update sections configmap properly", func(t *testing.T) {
pipeline1 := testutils.NewLogPipelineBuilder().
WithName("pipeline1").
WithFinalizer("FLUENT_BIT_SECTIONS_CONFIG_MAP").
WithHTTPOutput(testutils.HTTPHost("host")).
Build()
pipeline2 := testutils.NewLogPipelineBuilder().
WithName("pipeline2").
WithFinalizer("FLUENT_BIT_SECTIONS_CONFIG_MAP").
WithHTTPOutput(testutils.HTTPHost("host")).
Build()
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&pipeline1, &pipeline2).WithStatusSubresource(&pipeline1, &pipeline2).Build()

agentApplierDeleterMock := &mocks.AgentApplierDeleter{}
agentApplierDeleterMock.On("ApplyResources", mock.Anything, mock.Anything, mock.Anything).Return(nil)
agentApplierDeleterMock.On("DeleteResources", mock.Anything, mock.Anything, mock.Anything).Return(nil)

proberStub := commonStatusStubs.NewDaemonSetProber(nil)

flowHealthProberStub := &logpipelinemocks.FlowHealthProber{}
flowHealthProberStub.On("Probe", mock.Anything, pipeline1.Name).Return(prober.LogPipelineProbeResult{}, nil)
flowHealthProberStub.On("Probe", mock.Anything, pipeline2.Name).Return(prober.LogPipelineProbeResult{}, nil)

pipelineValidatorWithStubs := &Validator{
EndpointValidator: stubs.NewEndpointValidator(nil),
TLSCertValidator: stubs.NewTLSCertValidator(nil),
SecretRefValidator: stubs.NewSecretRefValidator(nil),
}

errToMsgStub := &logpipelinemocks.ErrorToMessageConverter{}

sut := New(fakeClient, testConfig, agentApplierDeleterMock, proberStub, flowHealthProberStub, istioStatusCheckerStub, pipelineValidatorWithStubs, errToMsgStub)

var pl1 telemetryv1alpha1.LogPipeline

require.NoError(t, fakeClient.Get(context.Background(), types.NamespacedName{Name: pipeline1.Name}, &pl1))
err := sut.Reconcile(context.Background(), &pl1)
require.NoError(t, err)

var pl2 telemetryv1alpha1.LogPipeline

require.NoError(t, fakeClient.Get(context.Background(), types.NamespacedName{Name: pipeline2.Name}, &pl2))
err = sut.Reconcile(context.Background(), &pl2)
require.NoError(t, err)

cm := &corev1.ConfigMap{}
err = fakeClient.Get(context.Background(), testConfig.SectionsConfigMap, cm)
require.NoError(t, err, "sections configmap must exist")
require.Contains(t, cm.Data[pipeline1.Name+".conf"], pipeline1.Name, "sections configmap must contain pipeline1 name")
require.Contains(t, cm.Data[pipeline2.Name+".conf"], pipeline2.Name, "sections configmap must contain pipeline2 name")

pipeline1Deleted := testutils.NewLogPipelineBuilder().
WithName("pipeline1").
WithFinalizer("FLUENT_BIT_SECTIONS_CONFIG_MAP").
WithHTTPOutput(testutils.HTTPHost("host")).
WithDeletionTimeStamp(metav1.Now()).
Build()

fakeClient.Delete(context.Background(), &pipeline1)
require.NoError(t, fakeClient.Get(context.Background(), types.NamespacedName{Name: pipeline1.Name}, &pl1))
err = sut.Reconcile(context.Background(), &pl1)
require.NoError(t, err)

pipeline1 = pipeline1Deleted
err = fakeClient.Get(context.Background(), testConfig.SectionsConfigMap, cm)
require.NoError(t, err, "sections configmap must exist")
require.NotContains(t, cm.Data[pipeline1.Name+".conf"], pipeline1.Name, "sections configmap must not contain pipeline1")
require.Contains(t, cm.Data[pipeline2.Name+".conf"], pipeline2.Name, "sections configmap must contain pipeline2 name")
})
}

Expand All @@ -943,7 +786,7 @@ func requireHasStatusCondition(t *testing.T, pipeline telemetryv1alpha1.LogPipel
require.NotEmpty(t, cond.LastTransitionTime)
}

//func TestCalculateChecksum(t *testing.T) {
// func TestCalculateChecksum(t *testing.T) {
// config := Config{
// DaemonSet: types.NamespacedName{
// Namespace: "default",
Expand Down
71 changes: 71 additions & 0 deletions internal/resources/fluentbit/sync_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,76 @@ alias foo`,

require.Error(t, err)
})

t.Run("should handle multiple pipelines and deletion properly", func(t *testing.T) {
sut := Syncer{
Client: fakeClient,
Config: Config{
SectionsConfigMap: sectionsCmName,
},
}

// Create pipeline1 with HTTP output
pipeline1 := &telemetryv1alpha1.LogPipeline{
ObjectMeta: metav1.ObjectMeta{
Name: "pipeline1",
},
Spec: telemetryv1alpha1.LogPipelineSpec{
Output: telemetryv1alpha1.LogPipelineOutput{
HTTP: &telemetryv1alpha1.LogPipelineHTTPOutput{
Host: telemetryv1alpha1.ValueType{Value: "host"},
},
},
},
}

// Create pipeline2 with HTTP output
pipeline2 := &telemetryv1alpha1.LogPipeline{
ObjectMeta: metav1.ObjectMeta{
Name: "pipeline2",
},
Spec: telemetryv1alpha1.LogPipelineSpec{
Output: telemetryv1alpha1.LogPipelineOutput{
HTTP: &telemetryv1alpha1.LogPipelineHTTPOutput{
Host: telemetryv1alpha1.ValueType{Value: "host"},
},
},
},
}

// Test adding both pipelines
deployablePipelines := []telemetryv1alpha1.LogPipeline{*pipeline1, *pipeline2}

err := sut.syncSectionsConfigMap(context.Background(), pipeline1, deployablePipelines)
require.NoError(t, err)
err = sut.syncSectionsConfigMap(context.Background(), pipeline2, deployablePipelines)
require.NoError(t, err)

// Verify both pipelines are in configmap
var cm corev1.ConfigMap
err = fakeClient.Get(context.Background(), sectionsCmName, &cm)
require.NoError(t, err)
require.Contains(t, cm.Data, "pipeline1.conf")
require.Contains(t, cm.Data, "pipeline2.conf")
require.Contains(t, cm.Data["pipeline1.conf"], "pipeline1")
require.Contains(t, cm.Data["pipeline2.conf"], "pipeline2")

// Mark pipeline1 for deletion
now := metav1.Now()
pipeline1.DeletionTimestamp = &now

// Update configmap with pipeline1 deleted
deployablePipelineAfterDeletion := []telemetryv1alpha1.LogPipeline{*pipeline2}
err = sut.syncSectionsConfigMap(context.Background(), pipeline1, deployablePipelineAfterDeletion)
require.NoError(t, err)

// Verify pipeline1 is removed but pipeline2 remains
err = fakeClient.Get(context.Background(), sectionsCmName, &cm)
require.NoError(t, err)
require.NotContains(t, cm.Data, "pipeline1.conf")
require.Contains(t, cm.Data, "pipeline2.conf")
require.Contains(t, cm.Data["pipeline2.conf"], "pipeline2")
})
}

func TestSyncFilesConfigMap(t *testing.T) {
Expand Down Expand Up @@ -264,6 +334,7 @@ alias foo`,

require.Error(t, err)
})

}

Check failure on line 338 in internal/resources/fluentbit/sync_test.go

View workflow job for this annotation

GitHub Actions / lint 

unnecessary trailing newline (whitespace)

func TestSyncEnvSecret(t *testing.T) {
Expand Down

0 comments on commit 3980385

Please sign in to comment.