hooks: split feature detection from setup and validation

Currently, the SetupAndValidate extension hook is executed right after
feature detection, but before the setup and validation of the different
connectivity test pods and artifacts. While this has the advantage of
nicely integrating with the logging of the detected features, it does
not allow the hook function to rely on the existence of the pods created
immediately afterwards. To enable both functionalities, let's split the
hook into two -- one first function to detect extra features, and a second
to perform the actual setup and validation of extra pods.

Signed-off-by: Marco Iorio <[email protected]>

cli/cmd.go

@@ -45,4 +45,5 @@ func (*NopHooks) AddSysdumpFlags(*pflag.FlagSet)
 func (*NopHooks) AddSysdumpTasks(*sysdump.Collector) error                        { return nil }
 func (*NopHooks) AddConnectivityTestFlags(*pflag.FlagSet)                         {}
 func (*NopHooks) AddConnectivityTests(*check.ConnectivityTest) error              { return nil }
+func (*NopHooks) DetectFeatures(context.Context, *check.ConnectivityTest) error   { return nil }
 func (*NopHooks) SetupAndValidate(context.Context, *check.ConnectivityTest) error { return nil }

connectivity/check/context.go

@@ -295,10 +295,15 @@ func (ct *ConnectivityTest) MustGetTest(name string) *Test {
 	return test
 }
 
+type setupHooks interface {
+	DetectFeatures(ctx context.Context, ct *ConnectivityTest) error
+	SetupAndValidate(ctx context.Context, ct *ConnectivityTest) error
+}
+
 // SetupAndValidate sets up and validates the connectivity test infrastructure
 // such as the client pods and validates the deployment of them along with
 // Cilium. This must be run before Run() is called.
-func (ct *ConnectivityTest) SetupAndValidate(ctx context.Context, setupAndValidateExtras func(ctx context.Context, ct *ConnectivityTest) error) error {
+func (ct *ConnectivityTest) SetupAndValidate(ctx context.Context, extra setupHooks) error {
 	if err := ctx.Err(); err != nil {
 		return err
 	}
@@ -322,8 +327,7 @@ func (ct *ConnectivityTest) SetupAndValidate(ctx context.Context, setupAndValida
 	if err := ct.detectFeatures(ctx); err != nil {
 		return err
 	}
-	// Setup and validate all the extras coming from extended functionalities.
-	if err := setupAndValidateExtras(ctx, ct); err != nil {
+	if err := extra.DetectFeatures(ctx, ct); err != nil {
 		return err
 	}
 
@@ -373,7 +377,9 @@ func (ct *ConnectivityTest) SetupAndValidate(ctx context.Context, setupAndValida
 			return fmt.Errorf("unable to detect K8s CIDR: %w", err)
 		}
 	}
-	return nil
+
+	// Setup and validate all the extras coming from extended functionalities.
+	return extra.SetupAndValidate(ctx, ct)
 }
 
 // Run kicks off execution of all Tests registered to the ConnectivityTest.

connectivity/suite.go

@@ -195,9 +195,14 @@ var (
 	client2Label = map[string]string{"name": "client2"}
 )
 
-func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*check.ConnectivityTest) error,
-	addExtraSetup func(context.Context, *check.ConnectivityTest) error) error {
-	if err := ct.SetupAndValidate(ctx, addExtraSetup); err != nil {
+type hooks interface {
+	DetectFeatures(ctx context.Context, ct *check.ConnectivityTest) error
+	SetupAndValidate(ctx context.Context, ct *check.ConnectivityTest) error
+	AddConnectivityTests(ct *check.ConnectivityTest) error
+}
+
+func Run(ctx context.Context, ct *check.ConnectivityTest, extra hooks) error {
+	if err := ct.SetupAndValidate(ctx, extra); err != nil {
 		return err
 	}
 
@@ -1222,7 +1227,7 @@ func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*ch
 	// Tests with DNS redirects to the proxy (e.g., client-egress-l7, dns-only,
 	// and to-fqdns) should always be executed last. See #367 for details.
 
-	if err := addExtraTests(ct); err != nil {
+	if err := extra.AddConnectivityTests(ct); err != nil {
 		return err
 	}
 

internal/cli/cmd/connectivity.go

@@ -95,7 +95,7 @@ func RunE(hooks Hooks) func(cmd *cobra.Command, args []string) error {
 		// and end the goroutine without returning.
 		go func() {
 			defer func() { done <- struct{}{} }()
-			err = connectivity.Run(ctx, cc, hooks.AddConnectivityTests, hooks.SetupAndValidate)
+			err = connectivity.Run(ctx, cc, hooks)
 
 			// If Fatal() was called in the test suite, the statement below won't fire.
 			finished = true

internal/cli/cmd/hooks.go

@@ -20,6 +20,7 @@ type Hooks interface {
 
 // ConnectivityTestHooks to extend cilium-cli with additional connectivity tests and related flags.
 type ConnectivityTestHooks interface {
+	DetectFeatures(ctx context.Context, ct *check.ConnectivityTest) error
 	SetupAndValidate(ctx context.Context, ct *check.ConnectivityTest) error
 	AddConnectivityTestFlags(flags *pflag.FlagSet)
 	AddConnectivityTests(ct *check.ConnectivityTest) error