Merge pull request #3748 from sedefsavas/externalvpc-cc

k8s-ci-robot · web-flow · commit 3271b7a9dbfd · 2022-09-27T03:29:57.000-07:00
Make subnet spec id field required for SSA to work with CC
diff --git a/api/v1beta2/network_types.go b/api/v1beta2/network_types.go
@@ -242,7 +242,7 @@ func (v *VPCSpec) IsIPv6Enabled() bool {
 // SubnetSpec configures an AWS Subnet.
 type SubnetSpec struct {
 	// ID defines a unique identifier to reference this resource.
-	ID string `json:"id,omitempty"`
+	ID string `json:"id"`
 
 	// CidrBlock is the CIDR block to be used when the provider creates a managed VPC.
 	CidrBlock string `json:"cidrBlock,omitempty"`
@@ -284,6 +284,8 @@ func (s *SubnetSpec) String() string {
 }
 
 // Subnets is a slice of Subnet.
+// +listType=map
+// +listMapKey=id
 type Subnets []SubnetSpec
 
 // ToMap returns a map from id to subnet.
diff --git a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml
@@ -446,8 +446,13 @@ spec:
                           description: Tags is a collection of tags describing the
                             resource.
                           type: object
+                      required:
+                      - id
                       type: object
                     type: array
+                    x-kubernetes-list-map-keys:
+                    - id
+                    x-kubernetes-list-type: map
                   vpc:
                     description: VPC configuration.
                     properties:
@@ -1719,8 +1724,13 @@ spec:
                           description: Tags is a collection of tags describing the
                             resource.
                           type: object
+                      required:
+                      - id
                       type: object
                     type: array
+                    x-kubernetes-list-map-keys:
+                    - id
+                    x-kubernetes-list-type: map
                   vpc:
                     description: VPC configuration.
                     properties:
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml
@@ -1153,8 +1153,13 @@ spec:
                           description: Tags is a collection of tags describing the
                             resource.
                           type: object
+                      required:
+                      - id
                       type: object
                     type: array
+                    x-kubernetes-list-map-keys:
+                    - id
+                    x-kubernetes-list-type: map
                   vpc:
                     description: VPC configuration.
                     properties:
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml
@@ -757,8 +757,13 @@ spec:
                                   description: Tags is a collection of tags describing
                                     the resource.
                                   type: object
+                              required:
+                              - id
                               type: object
                             type: array
+                            x-kubernetes-list-map-keys:
+                            - id
+                            x-kubernetes-list-type: map
                           vpc:
                             description: VPC configuration.
                             properties:
diff --git a/test/e2e/data/e2e_conf.yaml b/test/e2e/data/e2e_conf.yaml
@@ -105,7 +105,6 @@ providers:
           - sourcePath: "./infrastructure-aws/generated/cluster-template-limit-az.yaml"
           - sourcePath: "./infrastructure-aws/generated/cluster-template-machine-pool.yaml"
           - sourcePath: "./infrastructure-aws/generated/cluster-template-md-remediation.yaml"
-          - sourcePath: "./infrastructure-aws/generated/cluster-template-multi-az.yaml"
           - sourcePath: "./infrastructure-aws/generated/cluster-template-nested-multitenancy.yaml"
           - sourcePath: "./infrastructure-aws/generated/cluster-template-remote-management-cluster.yaml"
           - sourcePath: "./infrastructure-aws/generated/cluster-template-simple-multitenancy.yaml"
@@ -122,6 +121,7 @@ providers:
           - sourcePath: "./infrastructure-aws/generated/cluster-template-nested-multitenancy-clusterclass.yaml"
           - sourcePath: "./infrastructure-aws/kustomize_sources/nested-multitenancy-clusterclass/clusterclass-multi-tenancy.yaml"
           - sourcePath: "./infrastructure-aws/generated/cluster-template-self-hosted-clusterclass.yaml"
+          - sourcePath: "./infrastructure-aws/generated/cluster-template-external-vpc-clusterclass.yaml"
           - sourcePath: "./shared/v1beta2_provider/metadata.yaml"
           - sourcePath: "./infrastructure-aws/generated/cluster-template-ignition.yaml"
         replacements:
diff --git a/test/e2e/data/infrastructure-aws/kustomize_sources/external-vpc-clusterclass/byo-infra-variables.yaml b/test/e2e/data/infrastructure-aws/kustomize_sources/external-vpc-clusterclass/byo-infra-variables.yaml
@@ -0,0 +1,28 @@
+- op: add
+  path: /spec/topology/variables/-
+  value:
+    name: byoInfra
+    value: "true"
+- op: add
+  path: /spec/topology/variables/-
+  value:
+    name: vpcID
+    value: "${BYO_VPC_ID}"
+- op: add
+  path: /spec/topology/variables/-
+  value:
+    name: publicSubnetID
+    value: "${BYO_PUBLIC_SUBNET_ID}"
+- op: add
+  path: /spec/topology/variables/-
+  value:
+    name: privateSubnetID
+    value: "${BYO_PRIVATE_SUBNET_ID}"
+- op: add
+  path: /spec/topology/variables/-
+  value:
+    name: fdForBYOSubnets
+    value: "us-west-2a"
+- op: replace
+  path: /spec/topology/workers/machineDeployments/0/failureDomain
+  value: "us-west-2a"
diff --git a/test/e2e/data/infrastructure-aws/kustomize_sources/external-vpc-clusterclass/kustomization.yaml b/test/e2e/data/infrastructure-aws/kustomize_sources/external-vpc-clusterclass/kustomization.yaml
@@ -0,0 +1,16 @@
+bases:
+  - ../topology/cluster-template.yaml
+
+patches:
+  - path: ./byo-infra-variables.yaml
+    target:
+      group: cluster.x-k8s.io
+      version: v1beta1
+      kind: Cluster
+  - path: ./limited-az-variable.yaml
+    target:
+      group: cluster.x-k8s.io
+      version: v1beta1
+      kind: Cluster
+
+
diff --git a/test/e2e/data/infrastructure-aws/kustomize_sources/external-vpc-clusterclass/limited-az-variable.yaml b/test/e2e/data/infrastructure-aws/kustomize_sources/external-vpc-clusterclass/limited-az-variable.yaml
@@ -0,0 +1,5 @@
+- op: add
+  path: /spec/topology/variables/-
+  value:
+    name: vpcAZUsageLimit
+    value: "1"
diff --git a/test/e2e/data/infrastructure-aws/kustomize_sources/multi-az/kustomization.yaml b/test/e2e/data/infrastructure-aws/kustomize_sources/multi-az/kustomization.yaml
diff --git a/test/e2e/data/infrastructure-aws/kustomize_sources/multi-az/patches/multi-az.yaml b/test/e2e/data/infrastructure-aws/kustomize_sources/multi-az/patches/multi-az.yaml
diff --git a/test/e2e/data/infrastructure-aws/kustomize_sources/topology/clusterclass-ci-default.yaml b/test/e2e/data/infrastructure-aws/kustomize_sources/topology/clusterclass-ci-default.yaml
@@ -76,6 +76,36 @@ spec:
         type: string
         default: ""
         example: "1"
+  - name: vpcID
+    required: false
+    schema:
+      openAPIV3Schema:
+        type: string
+        default: ""
+  - name: publicSubnetID
+    required: false
+    schema:
+      openAPIV3Schema:
+        type: string
+        default: ""
+  - name: privateSubnetID
+    required: false
+    schema:
+      openAPIV3Schema:
+        type: string
+        default: ""
+  - name: fdForBYOSubnets
+    required: false
+    schema:
+      openAPIV3Schema:
+        type: string
+        default: ""
+  - name: byoInfra
+    required: false
+    schema:
+      openAPIV3Schema:
+        type: string
+        default: "false"
   - name: selfHosted
     required: false
     schema:
@@ -214,6 +244,53 @@ spec:
             path: "/spec/template/spec/network/vpc/availabilityZoneUsageLimit"
             valueFrom:
               template: "{{ .vpcAZUsageLimit }}"
+  - name: byoInfra
+    enabledIf: '{{ eq .byoInfra "true" }}'
+    definitions:
+      - selector:
+          apiVersion: infrastructure.cluster.x-k8s.io/v1beta2
+          kind: AWSClusterTemplate
+          matchResources:
+            infrastructureCluster: true
+        jsonPatches:
+          - op: add
+            path: "/spec/template/spec/network/vpc/id"
+            valueFrom:
+              variable: vpcID
+          - op: add
+            path: /spec/template/spec/network/subnets
+            valueFrom:
+              template: |
+                - id: "{{ .publicSubnetID }}"
+                - id: "{{ .privateSubnetID }}"
+  - name: awsMachineTemplateControlPlaneForBYO
+    enabledIf: '{{ eq .byoInfra "true" }}'
+    definitions:
+      - selector:
+          apiVersion: infrastructure.cluster.x-k8s.io/v1beta2
+          kind: AWSMachineTemplate
+          matchResources:
+            controlPlane: true
+        jsonPatches:
+          - op: add
+            path: "/spec/template/spec/failureDomain"
+            valueFrom:
+              variable: fdForBYOSubnets
+  - name: awsMachineTemplateWorkerForBYO
+    enabledIf: '{{ eq .byoInfra "true" }}'
+    definitions:
+      - selector:
+          apiVersion: infrastructure.cluster.x-k8s.io/v1beta2
+          kind: AWSMachineTemplate
+          matchResources:
+            machineDeploymentClass:
+              names:
+                - default-worker
+        jsonPatches:
+          - op: add
+            path: "/spec/template/spec/failureDomain"
+            valueFrom:
+              variable: fdForBYOSubnets
 ---
 apiVersion: infrastructure.cluster.x-k8s.io/v1beta2
 kind: AWSClusterTemplate
diff --git a/test/e2e/shared/defaults.go b/test/e2e/shared/defaults.go
@@ -49,7 +49,6 @@ const (
 	AwsNodeMachineType                   = "AWS_NODE_MACHINE_TYPE"
 	AwsAvailabilityZone1                 = "AWS_AVAILABILITY_ZONE_1"
 	AwsAvailabilityZone2                 = "AWS_AVAILABILITY_ZONE_2"
-	MultiAzFlavor                        = "multi-az"
 	LimitAzFlavor                        = "limit-az"
 	SpotInstancesFlavor                  = "spot-instances"
 	SSMFlavor                            = "ssm"
diff --git a/test/e2e/suites/unmanaged/helpers_test.go b/test/e2e/suites/unmanaged/helpers_test.go
@@ -388,35 +388,6 @@ func getEvents(namespace string) *corev1.EventList {
 	return eventsList
 }
 
-func getSubnetID(filterKey, filterValue, clusterName string) *string {
-	var subnetOutput *ec2.DescribeSubnetsOutput
-	var err error
-
-	ec2Client := ec2.New(e2eCtx.AWSSession)
-	subnetInput := &ec2.DescribeSubnetsInput{
-		Filters: []*ec2.Filter{
-			{
-				Name: aws.String(filterKey),
-				Values: []*string{
-					aws.String(filterValue),
-				},
-			},
-			{
-				Name:   aws.String("tag-key"),
-				Values: aws.StringSlice([]string{"sigs.k8s.io/cluster-api-provider-aws/cluster/" + clusterName}),
-			},
-		},
-	}
-
-	Eventually(func() int {
-		subnetOutput, err = ec2Client.DescribeSubnets(subnetInput)
-		Expect(err).NotTo(HaveOccurred())
-		return len(subnetOutput.Subnets)
-	}, e2eCtx.E2EConfig.GetIntervals("", "wait-infra-subnets")...).Should(Equal(1))
-
-	return subnetOutput.Subnets[0].SubnetId
-}
-
 func getVolumeIds(info statefulSetInfo, k8sclient crclient.Client) []*string {
 	ginkgo.By("Retrieving IDs of dynamically provisioned volumes.")
 	statefulset := &appsv1.StatefulSet{}
diff --git a/test/e2e/suites/unmanaged/unmanaged_functional_clusterclass_test.go b/test/e2e/suites/unmanaged/unmanaged_functional_clusterclass_test.go
@@ -125,4 +125,79 @@ var _ = ginkgo.Context("[unmanaged] [functional] [ClusterClass]", func() {
 			Expect(len(controlPlaneMachines)).To(Equal(1))
 		})
 	})
+
+	// This test creates a workload cluster using an externally managed VPC and subnets. CAPA is still handling security group
+	// creation for the cluster. All applicable resources are restricted to us-west-2a for simplicity.
+	ginkgo.Describe("Workload cluster with external infrastructure [ClusterClass]", func() {
+		var namespace *corev1.Namespace
+		var requiredResources *shared.TestResource
+		specName := "functional-test-extinfra-clusterclass"
+		mgmtClusterName := fmt.Sprintf("%s-%s", specName, util.RandomString(6))
+		mgmtClusterInfra := new(shared.AWSInfrastructure)
+
+		// Some infrastructure creation was moved to a setup node to better organize the test.
+		ginkgo.JustBeforeEach(func() {
+			requiredResources = &shared.TestResource{EC2Normal: 2 * e2eCtx.Settings.InstanceVCPU, IGW: 2, NGW: 2, VPC: 2, ClassicLB: 2, EIP: 5}
+			requiredResources.WriteRequestedResources(e2eCtx, specName)
+			Expect(shared.AcquireResources(requiredResources, config.GinkgoConfig.ParallelNode, flock.New(shared.ResourceQuotaFilePath))).To(Succeed())
+			namespace = shared.SetupSpecNamespace(ctx, specName, e2eCtx)
+			ginkgo.By("Creating the management cluster infrastructure")
+			mgmtClusterInfra.New(shared.AWSInfrastructureSpec{
+				ClusterName:       mgmtClusterName,
+				VpcCidr:           "10.0.0.0/23",
+				PublicSubnetCidr:  "10.0.0.0/24",
+				PrivateSubnetCidr: "10.0.1.0/24",
+				AvailabilityZone:  "us-west-2a",
+			}, e2eCtx)
+			mgmtClusterInfra.CreateInfrastructure()
+		})
+
+		// Infrastructure cleanup is done in setup node so it is not bypassed if there is a test failure in the subject node.
+		ginkgo.JustAfterEach(func() {
+			shared.ReleaseResources(requiredResources, config.GinkgoConfig.ParallelNode, flock.New(shared.ResourceQuotaFilePath))
+			shared.DumpSpecResourcesAndCleanup(ctx, "", namespace, e2eCtx)
+			if !e2eCtx.Settings.SkipCleanup {
+				ginkgo.By("Deleting the management cluster infrastructure")
+				mgmtClusterInfra.DeleteInfrastructure()
+			}
+		})
+
+		ginkgo.It("should create workload cluster in external VPC", func() {
+			ginkgo.By("Validating management infrastructure")
+			Expect(mgmtClusterInfra.VPC).NotTo(BeNil())
+			Expect(*mgmtClusterInfra.State.VpcState).To(Equal("available"))
+			Expect(len(mgmtClusterInfra.Subnets)).To(Equal(2))
+			Expect(mgmtClusterInfra.InternetGateway).NotTo(BeNil())
+			Expect(mgmtClusterInfra.ElasticIP).NotTo(BeNil())
+			Expect(mgmtClusterInfra.NatGateway).NotTo(BeNil())
+			Expect(len(mgmtClusterInfra.RouteTables)).To(Equal(2))
+
+			shared.SetEnvVar("BYO_VPC_ID", *mgmtClusterInfra.VPC.VpcId, false)
+			shared.SetEnvVar("BYO_PUBLIC_SUBNET_ID", *mgmtClusterInfra.State.PublicSubnetID, false)
+			shared.SetEnvVar("BYO_PRIVATE_SUBNET_ID", *mgmtClusterInfra.State.PrivateSubnetID, false)
+
+			ginkgo.By("Creating a management cluster in a peered VPC")
+			mgmtConfigCluster := defaultConfigCluster(mgmtClusterName, namespace.Name)
+			mgmtConfigCluster.WorkerMachineCount = pointer.Int64Ptr(1)
+			mgmtConfigCluster.Flavor = "external-vpc-clusterclass"
+			mgmtCluster, mgmtMD, _ := createCluster(ctx, mgmtConfigCluster, result)
+
+			mgmtWM := framework.GetMachinesByMachineDeployments(ctx, framework.GetMachinesByMachineDeploymentsInput{
+				Lister:            e2eCtx.Environment.BootstrapClusterProxy.GetClient(),
+				ClusterName:       mgmtClusterName,
+				Namespace:         namespace.Name,
+				MachineDeployment: *mgmtMD[0],
+			})
+			mgmtCPM := framework.GetControlPlaneMachinesByCluster(ctx, framework.GetControlPlaneMachinesByClusterInput{
+				Lister:      e2eCtx.Environment.BootstrapClusterProxy.GetClient(),
+				ClusterName: mgmtClusterName,
+				Namespace:   namespace.Name,
+			})
+			Expect(len(mgmtWM)).To(Equal(1))
+			Expect(len(mgmtCPM)).To(Equal(1))
+			ginkgo.By("Deleting the management cluster")
+			deleteCluster(ctx, mgmtCluster)
+		})
+	})
+
 })
diff --git a/test/e2e/suites/unmanaged/unmanaged_functional_test.go b/test/e2e/suites/unmanaged/unmanaged_functional_test.go
