Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix flaky controller tests #3560

Merged
merged 1 commit into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion controllers/opentelemetrycollector_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ func (r *OpenTelemetryCollectorReconciler) findOtelOwnedObjects(ctx context.Cont
default:
}
}

// at this point we don't know if the most recent ConfigMap will still be the most recent after reconciliation, or
// if a new one will be created. We keep one additional ConfigMap to account for this. The next reconciliation that
// doesn't spawn a new ConfigMap will delete the extra one we kept here.
Expand Down
11 changes: 8 additions & 3 deletions controllers/reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -978,7 +978,8 @@ func TestOpenTelemetryCollectorReconciler_VersionedConfigMaps(t *testing.T) {

assert.EventuallyWithT(t, func(collect *assert.CollectT) {
configMaps := &v1.ConfigMapList{}
listErr := k8sClient.List(clientCtx, configMaps, opts...)
// use the reconciler client here to ensure it sees the new ConfigMap, before running the next reconciliation
listErr := reconciler.Client.List(clientCtx, configMaps, opts...)
assert.NoError(collect, listErr)
assert.NotEmpty(collect, configMaps)
assert.Len(collect, configMaps.Items, 4)
Expand All @@ -992,8 +993,12 @@ func TestOpenTelemetryCollectorReconciler_VersionedConfigMaps(t *testing.T) {
listErr := k8sClient.List(clientCtx, configMaps, opts...)
assert.NoError(collect, listErr)
assert.NotEmpty(collect, configMaps)
assert.Len(collect, configMaps.Items, 3)
}, time.Second*5, time.Millisecond)
// actual deletion can happen with a delay in a K8s cluster, check the timestamp instead to speed things up
items := slices.DeleteFunc(configMaps.Items, func(item v1.ConfigMap) bool {
return item.DeletionTimestamp != nil
})
assert.Len(collect, items, 3)
}, time.Second*30, time.Second) // not sure why this can take so long to bubble up
}

func TestOpAMPBridgeReconciler_Reconcile(t *testing.T) {
Expand Down
4 changes: 4 additions & 0 deletions controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/yaml"
Expand Down Expand Up @@ -139,6 +140,9 @@ func TestMain(m *testing.M) {
ctx, cancel = context.WithCancel(context.TODO())
defer cancel()

// logging is useful for these tests
logf.SetLogger(zap.New())

// +kubebuilder:scaffold:scheme
utilruntime.Must(monitoringv1.AddToScheme(testScheme))
utilruntime.Must(networkingv1.AddToScheme(testScheme))
Expand Down
Loading