From 21f069b2c1136930f896d25db5f2dee9160b858c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Mon, 11 Sep 2023 11:27:30 +0200 Subject: [PATCH] rely on controller-runtime for kong crds --- internal/manager/run.go | 2 +- test/envtest/crds_envtest_test.go | 65 ++++---------------------- test/envtest/manager_envtest_test.go | 68 ++++++++++++++++++++++++++++ test/envtest/run.go | 18 +++++++- test/internal/helpers/tcp.go | 2 +- 5 files changed, 96 insertions(+), 59 deletions(-) create mode 100644 test/envtest/manager_envtest_test.go diff --git a/internal/manager/run.go b/internal/manager/run.go index c55ce1bd84..65e9a0bd17 100644 --- a/internal/manager/run.go +++ b/internal/manager/run.go @@ -130,7 +130,7 @@ func Run(ctx context.Context, c *Config, diagnostic util.ConfigDumpDiagnostic, d mgr, err := ctrl.NewManager(kubeconfig, controllerOpts) if err != nil { - return fmt.Errorf("unable to start controller manager: %w", err) + return fmt.Errorf("unable to create controller manager: %w", err) } if err := waitForKubernetesAPIReadiness(ctx, setupLog, mgr); err != nil { diff --git a/test/envtest/crds_envtest_test.go b/test/envtest/crds_envtest_test.go index be1a68d7da..b0c03cb4ac 100644 --- a/test/envtest/crds_envtest_test.go +++ b/test/envtest/crds_envtest_test.go @@ -4,8 +4,6 @@ package envtest import ( "context" - "fmt" - "net/url" "strings" "testing" "time" @@ -27,66 +25,17 @@ import ( kongv1beta1 "github.com/kong/kubernetes-ingress-controller/v2/pkg/apis/configuration/v1beta1" "github.com/kong/kubernetes-ingress-controller/v2/pkg/clientset" "github.com/kong/kubernetes-ingress-controller/v2/test/consts" - "github.com/kong/kubernetes-ingress-controller/v2/test/internal/helpers" ) -func TestManagerDoesntStartUntilKubernetesAPIReachable(t *testing.T) { - scheme := Scheme(t, WithKong) - envcfg := Setup(t, scheme) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - t.Log("Setting up a proxy for Kubernetes API server so that we can interrupt it") - u, err := url.Parse(envcfg.Host) - require.NoError(t, err) - apiServerProxy, err := helpers.NewTCPProxy(u.Host) - require.NoError(t, err) - go func() { - err := apiServerProxy.Run(ctx) - assert.NoError(t, err) - }() - apiServerProxy.StopHandlingConnections() - - t.Log("Replacing Kubernetes API server address with the proxy address") - envcfg.Host = fmt.Sprintf("https://%s", apiServerProxy.Address()) - - loggerHook := RunManager(ctx, t, envcfg) - hasLog := func(expectedLog string) bool { - return lo.ContainsBy(loggerHook.AllEntries(), func(entry *logrus.Entry) bool { - return strings.Contains(entry.Message, expectedLog) - }) - } - - t.Log("Ensuring manager is waiting for Kubernetes API to be ready") - const expectedKubernetesAPICheckErrorLog = "Retrying Kubernetes API readiness check after error" - require.Eventually(t, func() bool { return hasLog(expectedKubernetesAPICheckErrorLog) }, time.Minute, time.Millisecond) - - t.Log("Ensure manager hasn't been started yet and no config sync has happened") - const configurationSyncedToKongLog = "successfully synced configuration to Kong" - const startingManagerLog = "Starting manager" - require.False(t, hasLog(configurationSyncedToKongLog)) - require.False(t, hasLog(startingManagerLog)) - - t.Log("Starting accepting connections in Kubernetes API proxy so that manager can start") - apiServerProxy.StartHandlingConnections() - - t.Log("Ensuring manager has been started and config sync has happened") - require.Eventually(t, func() bool { - return hasLog(startingManagerLog) && - hasLog(configurationSyncedToKongLog) - }, time.Minute, time.Millisecond) -} - -// TestDynamicCRDController_StartsControllersWhenCRDsInstalled ensures that in case of missing CRDs installation in the +// TestGatewayAPIControllersMayBeDynamicallyStarted ensures that in case of missing CRDs installation in the // cluster, specific controllers are not started until the CRDs are installed. -func TestDynamicCRDController_StartsControllersWhenCRDsInstalled(t *testing.T) { +func TestGatewayAPIControllersMayBeDynamicallyStarted(t *testing.T) { scheme := Scheme(t, WithKong) envcfg := Setup(t, scheme, WithInstallGatewayCRDs(false)) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - loggerHook := RunManager(ctx, t, envcfg, WithGatewayFeatureEnabled) + loggerHook := RunManager(ctx, t, envcfg, WithGatewayFeatureEnabled, WithPublishService("ns")) controllers := []string{ "Gateway", @@ -131,7 +80,9 @@ func TestDynamicCRDController_StartsControllersWhenCRDsInstalled(t *testing.T) { requireLogForAllControllers(expectedLogOnCRDInstalled) } -func TestNoKongCRDsIsFatal(t *testing.T) { +// TestNoKongCRDsInstalledIsFatal ensures that in case of missing Kong CRDs installation, the manager Run() eventually +// returns an error due to cache synchronisation timeout. +func TestNoKongCRDsInstalledIsFatal(t *testing.T) { scheme := Scheme(t) envcfg := Setup(t, scheme, WithInstallKongCRDs(false)) @@ -143,8 +94,10 @@ func TestNoKongCRDsIsFatal(t *testing.T) { logger := logrusr.New(logrusLogger) ctrl.SetLogger(logger) + // Reducing the cache sync timeout to speed up the test. + cfg.CacheSyncTimeout = time.Millisecond * 500 err := manager.Run(ctx, &cfg, util.ConfigDumpDiagnostic{}, logrusLogger) - require.ErrorContains(t, err, "requirements not satisfied") + require.ErrorContains(t, err, "timed out waiting for cache to be synced") } func TestCRDValidations(t *testing.T) { diff --git a/test/envtest/manager_envtest_test.go b/test/envtest/manager_envtest_test.go new file mode 100644 index 0000000000..fc44c8824a --- /dev/null +++ b/test/envtest/manager_envtest_test.go @@ -0,0 +1,68 @@ +//go:build envtest + +package envtest + +import ( + "context" + "fmt" + "net/url" + "strings" + "testing" + "time" + + "github.com/kong/kubernetes-ingress-controller/v2/test/internal/helpers" + "github.com/samber/lo" + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestManagerDoesntStartUntilKubernetesAPIReachable ensures that the manager and its Runnables are not start until the +// Kubernetes API server is reachable. +func TestManagerDoesntStartUntilKubernetesAPIReachable(t *testing.T) { + scheme := Scheme(t, WithKong) + envcfg := Setup(t, scheme) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + t.Log("Setting up a proxy for Kubernetes API server so that we can interrupt it") + u, err := url.Parse(envcfg.Host) + require.NoError(t, err) + apiServerProxy, err := helpers.NewTCPProxy(u.Host) + require.NoError(t, err) + go func() { + err := apiServerProxy.Run(ctx) + assert.NoError(t, err) + }() + apiServerProxy.StopHandlingConnections() + + t.Log("Replacing Kubernetes API server address with the proxy address") + envcfg.Host = fmt.Sprintf("https://%s", apiServerProxy.Address()) + + loggerHook := RunManager(ctx, t, envcfg) + hasLog := func(expectedLog string) bool { + return lo.ContainsBy(loggerHook.AllEntries(), func(entry *logrus.Entry) bool { + return strings.Contains(entry.Message, expectedLog) + }) + } + + t.Log("Ensuring manager is waiting for Kubernetes API to be ready") + const expectedKubernetesAPICheckErrorLog = "Retrying Kubernetes API readiness check after error" + require.Eventually(t, func() bool { return hasLog(expectedKubernetesAPICheckErrorLog) }, time.Minute, time.Millisecond) + + t.Log("Ensure manager hasn't been started yet and no config sync has happened") + const configurationSyncedToKongLog = "successfully synced configuration to Kong" + const startingManagerLog = "Starting manager" + require.False(t, hasLog(configurationSyncedToKongLog)) + require.False(t, hasLog(startingManagerLog)) + + t.Log("Starting accepting connections in Kubernetes API proxy so that manager can start") + apiServerProxy.StartHandlingConnections() + + t.Log("Ensuring manager has been started and config sync has happened") + require.Eventually(t, func() bool { + return hasLog(startingManagerLog) && + hasLog(configurationSyncedToKongLog) + }, time.Minute, time.Millisecond) +} diff --git a/test/envtest/run.go b/test/envtest/run.go index 923e8e14e3..148fb7aed8 100644 --- a/test/envtest/run.go +++ b/test/envtest/run.go @@ -1,14 +1,17 @@ package envtest import ( + "bytes" "context" "fmt" + "sync" "testing" "github.com/bombsimon/logrusr/v4" "github.com/phayes/freeport" "github.com/samber/mo" "github.com/sirupsen/logrus/hooks/test" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" k8stypes "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/rest" @@ -95,13 +98,26 @@ func RunManager( } logrusLogger, loggerHook := test.NewNullLogger() + b := &bytes.Buffer{} + logrusLogger.Out = b logger := logrusr.New(logrusLogger) ctx = ctrl.LoggerInto(ctx, logger) + // This wait group makes it so that we wait for manager to exit. + // This way we get clean test logs not mixing between tests. + wg := sync.WaitGroup{} + wg.Add(1) go func() { + defer wg.Done() err := manager.Run(ctx, &cfg, util.ConfigDumpDiagnostic{}, logrusLogger) - require.NoError(t, err) + assert.NoError(t, err) }() + t.Cleanup(func() { + wg.Wait() + if t.Failed() { + t.Logf("manager logs:\n%s", b.String()) + } + }) return loggerHook } diff --git a/test/internal/helpers/tcp.go b/test/internal/helpers/tcp.go index d36fbed521..f593aecff6 100644 --- a/test/internal/helpers/tcp.go +++ b/test/internal/helpers/tcp.go @@ -27,7 +27,7 @@ type TCPProxy struct { func NewTCPProxy(destination string) (*TCPProxy, error) { listener, err := net.Listen("tcp", "localhost:0") if err != nil { - return nil, err + return nil, fmt.Errorf("failed to listen: %w", err) } return &TCPProxy{ destination: destination,