Merge pull request #8026 from killianmuldoon/pr-exvars-add-test-extension-hook

 🌱 Add DiscoverVariables hook implementation for test extension

Author: k8s-ci-robot

test/e2e/data/infrastructure-docker/main/cluster-template-upgrades-runtimesdk/cluster-runtimesdk.yaml

@@ -29,3 +29,9 @@ spec:
     variables:
       - name: kubeadmControlPlaneMaxSurge
         value: "1"
+      # imageRepository has definitions for both an inline patch and the test-patch. This value is required in the
+      # test-patch and requires definitionFrom to reflect that.
+      - name: imageRepository
+        definitionFrom: test-patch
+        value: "kindest"
+

test/e2e/data/infrastructure-docker/main/clusterclass-quick-start-runtimesdk.yaml

@@ -33,12 +33,8 @@ spec:
             kind: DockerMachineTemplate
             name: quick-start-default-worker-machinetemplate
   variables:
-  - name: lbImageRepository
-    required: false
-    schema:
-      openAPIV3Schema:
-        type: string
-        default: kindest
+  # This variable is not used in any patch, but is here to ensure the variable discovered from the runtime hook is correctly
+  # used.
   - name: imageRepository
     required: true
     schema:
@@ -47,19 +43,12 @@ spec:
         default: "registry.k8s.io"
         example: "registry.k8s.io"
         description: "imageRepository sets the container registry to pull images from. If empty, `registry.k8s.io` will be used by default."
-  - name: kubeadmControlPlaneMaxSurge
-    required: false
-    schema:
-      openAPIV3Schema:
-        type: string
-        default: ""
-        example: "0"
-        description: "kubeadmControlPlaneMaxSurge is the maximum number of control planes that can be scheduled above or under the desired number of control plane machines."
   patches:
-  - name: lbImageRepository
+  - name: test-patch
     external:
       generateExtension: generate-patches.k8s-upgrade-with-runtimesdk
       validateExtension: validate-topology.k8s-upgrade-with-runtimesdk
+      discoverVariablesExtension: discover-variables.k8s-upgrade-with-runtimesdk
 ---
 apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
 kind: DockerClusterTemplate

test/extension/handlers/topologymutation/handler.go

@@ -34,6 +34,7 @@ import (
 	intstrutil "k8s.io/apimachinery/pkg/util/intstr"
 	ctrl "sigs.k8s.io/controller-runtime"
 
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
 	controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
 	runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
@@ -119,15 +120,15 @@ func (h *ExtensionHandlers) GeneratePatches(ctx context.Context, req *runtimehoo
 }
 
 // patchDockerClusterTemplate patches the DockerClusterTemplate.
-// It sets the LoadBalancer.ImageRepository if the lbImageRepository variable is provided.
+// It sets the LoadBalancer.ImageRepository if the imageRepository variable is provided.
 // NOTE: this patch is not required for any special reason, it is used for testing the patch machinery itself.
 func patchDockerClusterTemplate(_ context.Context, dockerClusterTemplate *infrav1.DockerClusterTemplate, templateVariables map[string]apiextensionsv1.JSON) error {
-	lbImageRepo, found, err := topologymutation.GetStringVariable(templateVariables, "lbImageRepository")
+	imageRepo, found, err := topologymutation.GetStringVariable(templateVariables, "imageRepository")
 	if err != nil {
 		return errors.Wrap(err, "could not set DockerClusterTemplate loadBalancer imageRepository")
 	}
 	if found {
-		dockerClusterTemplate.Spec.Template.Spec.LoadBalancer.ImageRepository = lbImageRepo
+		dockerClusterTemplate.Spec.Template.Spec.LoadBalancer.ImageRepository = imageRepo
 	}
 	return nil
 }
@@ -313,3 +314,36 @@ func (h *ExtensionHandlers) ValidateTopology(ctx context.Context, _ *runtimehook
 
 	resp.Status = runtimehooksv1.ResponseStatusSuccess
 }
+
+// DiscoverVariables implements the HandlerFunc for the DiscoverVariables hook.
+func (h *ExtensionHandlers) DiscoverVariables(ctx context.Context, _ *runtimehooksv1.DiscoverVariablesRequest, resp *runtimehooksv1.DiscoverVariablesResponse) {
+	log := ctrl.LoggerFrom(ctx)
+	log.Info("DiscoverVariables called")
+
+	resp.Status = runtimehooksv1.ResponseStatusSuccess
+	resp.Variables = []clusterv1.ClusterClassVariable{
+		{
+			Name:     "kubeadmControlPlaneMaxSurge",
+			Required: false,
+			Schema: clusterv1.VariableSchema{
+				OpenAPIV3Schema: clusterv1.JSONSchemaProps{
+					Type:        "string",
+					Default:     &apiextensionsv1.JSON{Raw: []byte(`""`)},
+					Example:     &apiextensionsv1.JSON{Raw: []byte(`""`)},
+					Description: "kubeadmControlPlaneMaxSurge is the maximum number of control planes that can be scheduled above or under the desired number of control plane machines.",
+				},
+			},
+		},
+		// This variable must be set in the Cluster as it has no default value and is required.
+		{
+			Name:     "imageRepository",
+			Required: true,
+			Schema: clusterv1.VariableSchema{
+				OpenAPIV3Schema: clusterv1.JSONSchemaProps{
+					Type:    "string",
+					Example: &apiextensionsv1.JSON{Raw: []byte(`"kindest"`)},
+				},
+			},
+		},
+	}
+}

test/extension/handlers/topologymutation/handler_test.go

@@ -58,16 +58,16 @@ func Test_patchDockerClusterTemplate(t *testing.T) {
 		expectedErr      bool
 	}{
 		{
-			name:             "no op if lbImageRepository is not set",
+			name:             "no op if imageRepository is not set",
 			template:         &infrav1.DockerClusterTemplate{},
 			variables:        nil,
 			expectedTemplate: &infrav1.DockerClusterTemplate{},
 		},
 		{
-			name:     "set LoadBalancer.ImageRepository if lbImageRepository is set",
+			name:     "set LoadBalancer.ImageRepository if imageRepository is set",
 			template: &infrav1.DockerClusterTemplate{},
 			variables: map[string]apiextensionsv1.JSON{
-				"lbImageRepository": {Raw: toJSON("testImage")},
+				"imageRepository": {Raw: toJSON("testImage")},
 			},
 			expectedTemplate: &infrav1.DockerClusterTemplate{
 				Spec: infrav1.DockerClusterTemplateSpec{
@@ -353,8 +353,8 @@ func TestHandler_GeneratePatches(t *testing.T) {
 		}),
 		newVariable("kubeadmControlPlaneMaxSurge", "3"),
 	}
-	lbImageRepositoryVar := []runtimehooksv1.Variable{
-		newVariable("lbImageRepository", "docker.io"),
+	imageRepositoryVar := []runtimehooksv1.Variable{
+		newVariable("imageRepository", "docker.io"),
 	}
 	machineDeploymentVars123 := []runtimehooksv1.Variable{
 		newVariable(variables.BuiltinsName, variables.Builtins{
@@ -399,7 +399,7 @@ func TestHandler_GeneratePatches(t *testing.T) {
 				requestItem("1", kubeadmControlPlaneTemplate, controlPlaneVarsV123WithMaxSurge),
 				requestItem("2", dockerMachineTemplate, controlPlaneVarsV123WithMaxSurge),
 				requestItem("3", dockerMachineTemplate, machineDeploymentVars123),
-				requestItem("4", dockerClusterTemplate, lbImageRepositoryVar),
+				requestItem("4", dockerClusterTemplate, imageRepositoryVar),
 				requestItem("5", kubeadmConfigTemplate, machineDeploymentVars123),
 			},
 			expectedResponse: &runtimehooksv1.GeneratePatchesResponse{

test/extension/main.go

@@ -178,6 +178,15 @@ func main() {
 		os.Exit(1)
 	}
 
+	if err := webhookServer.AddExtensionHandler(server.ExtensionHandler{
+		Hook:        runtimehooksv1.DiscoverVariables,
+		Name:        "discover-variables",
+		HandlerFunc: topologyMutationExtensionHandlers.DiscoverVariables,
+	}); err != nil {
+		setupLog.Error(err, "error adding handler")
+		os.Exit(1)
+	}
+
 	// Lifecycle Hooks
 
 	// Gets a client to access the Kubernetes cluster where this RuntimeExtension will be deployed to;