Fix GKE Cluster Creation for User Provided Subnet

Signed-off-by: Shaad7 <abdullah.alshaad@appscode.com> Signed-off-by: AbdullahAlShaad <abdullah.alshaad@appscode.com>
kubernetes-sigs · Mar 5, 2024 · d6cf2e1 · d6cf2e1
1 parent be17146
commit d6cf2e1
Show file tree

Hide file tree

Showing 6 changed files with 152 additions and 6 deletions.
diff --git a/cloud/services/compute/subnets/reconcile.go b/cloud/services/compute/subnets/reconcile.go
@@ -43,7 +43,7 @@ func (s *Service) Delete(ctx context.Context) error {
 	logger := log.FromContext(ctx)
 	for _, subnetSpec := range s.scope.SubnetSpecs() {
 		logger.V(2).Info("Deleting a subnet", "name", subnetSpec.Name)
-		subnetKey := meta.RegionalKey(subnetSpec.Name, s.scope.Region())
+		subnetKey := meta.RegionalKey(subnetSpec.Name, s.getSubnetRegion(subnetSpec))
 		err := s.subnets.Delete(ctx, subnetKey)
 		if err != nil && !gcperrors.IsNotFound(err) {
 			logger.Error(err, "Error deleting subnet", "name", subnetSpec.Name)
@@ -60,7 +60,7 @@ func (s *Service) createOrGetSubnets(ctx context.Context) ([]*compute.Subnetwork
 	subnets := []*compute.Subnetwork{}
 	for _, subnetSpec := range s.scope.SubnetSpecs() {
 		logger.V(2).Info("Looking for subnet", "name", subnetSpec.Name)
-		subnetKey := meta.RegionalKey(subnetSpec.Name, s.scope.Region())
+		subnetKey := meta.RegionalKey(subnetSpec.Name, s.getSubnetRegion(subnetSpec))
 		subnet, err := s.subnets.Get(ctx, subnetKey)
 		if err != nil {
 			if !gcperrors.IsNotFound(err) {
@@ -86,3 +86,11 @@ func (s *Service) createOrGetSubnets(ctx context.Context) ([]*compute.Subnetwork
 
 	return subnets, nil
 }
+
+// getSubnetRegion returns subnet region if user provided it, otherwise returns default scope region.
+func (s *Service) getSubnetRegion(subnetSpec *compute.Subnetwork) string {
+	if subnetSpec.Region != "" {
+		return subnetSpec.Region
+	}
+	return s.scope.Region()
+}
diff --git a/cloud/services/container/clusters/reconcile.go b/cloud/services/container/clusters/reconcile.go
@@ -250,10 +250,10 @@ func (s *Service) createCluster(ctx context.Context, log *logr.Logger) error {
 	}
 
 	isRegional := shared.IsRegional(s.scope.Region())
-
 	cluster := &containerpb.Cluster{
-		Name:    s.scope.ClusterName(),
-		Network: *s.scope.GCPManagedCluster.Spec.Network.Name,
+		Name:       s.scope.ClusterName(),
+		Network:    *s.scope.GCPManagedCluster.Spec.Network.Name,
+		Subnetwork: s.getSubnetNameInClusterRegion(),
 		Autopilot: &containerpb.Autopilot{
 			Enabled: s.scope.GCPManagedControlPlane.Spec.EnableAutopilot,
 		},
@@ -295,6 +295,16 @@ func (s *Service) createCluster(ctx context.Context, log *logr.Logger) error {
 	return nil
 }
 
+// getSubnetNameInClusterRegion returns the subnet which is in the same region as cluster. If not found it returns empty string.
+func (s *Service) getSubnetNameInClusterRegion() string {
+	for _, subnet := range s.scope.GCPManagedCluster.Spec.Network.Subnets {
+		if subnet.Region == s.scope.Region() {
+			return subnet.Name
+		}
+	}
+	return ""
+}
+
 func (s *Service) updateCluster(ctx context.Context, updateClusterRequest *containerpb.UpdateClusterRequest, log *logr.Logger) error {
 	_, err := s.scope.ManagedControlPlaneClient().UpdateCluster(ctx, updateClusterRequest)
 	if err != nil {

diff --git a/exp/api/v1beta1/gcpmanagedcluster_webhook.go b/exp/api/v1beta1/gcpmanagedcluster_webhook.go
@@ -20,6 +20,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
+	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -53,7 +54,7 @@ var _ webhook.Validator = &GCPManagedCluster{}
 func (r *GCPManagedCluster) ValidateCreate() (admission.Warnings, error) {
 	gcpmanagedclusterlog.Info("validate create", "name", r.Name)
 
-	return nil, nil
+	return r.validate()
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
@@ -96,3 +97,36 @@ func (r *GCPManagedCluster) ValidateDelete() (admission.Warnings, error) {
 
 	return nil, nil
 }
+
+func (r *GCPManagedCluster) validate() (admission.Warnings, error) {
+	validators := []func() error{
+		r.validateCustomSubnet,
+	}
+
+	var errs []error
+	for _, validator := range validators {
+		if err := validator(); err != nil {
+			errs = append(errs, err)
+		}
+	}
+
+	return nil, kerrors.NewAggregate(errs)
+}
+
+func (r *GCPManagedCluster) validateCustomSubnet() error {
+	gcpmanagedclusterlog.Info("validate custom subnet", "name", r.Name)
+	if r.Spec.Network.AutoCreateSubnetworks == nil || *r.Spec.Network.AutoCreateSubnetworks {
+		return nil
+	}
+	var isSubnetExistInClusterRegion = false
+	for _, subnet := range r.Spec.Network.Subnets {
+		if subnet.Region == r.Spec.Region {
+			isSubnetExistInClusterRegion = true
+		}
+	}
+
+	if !isSubnetExistInClusterRegion {
+		return field.Required(field.NewPath("spec", "network", "subnet"), "at least one given subnets region should be same as spec.network.region when spec.network.autoCreateSubnetworks is false")
+	}
+	return nil
+}
diff --git a/test/e2e/config/gcp-ci.yaml b/test/e2e/config/gcp-ci.yaml
@@ -71,6 +71,7 @@ providers:
       - sourcePath: "${PWD}/test/e2e/data/infrastructure-gcp/cluster-template-ci-with-creds.yaml"
       - sourcePath: "${PWD}/test/e2e/data/infrastructure-gcp/cluster-template-ci-gke.yaml"
       - sourcePath: "${PWD}/test/e2e/data/infrastructure-gcp/cluster-template-ci-gke-autopilot.yaml"
+      - sourcePath: "${PWD}/test/e2e/data/infrastructure-gcp/cluster-template-ci-gke-custom-subnet.yaml"
 
 variables:
   KUBERNETES_VERSION: "${KUBERNETES_VERSION:-v1.28.3}"

diff --git a/test/e2e/data/infrastructure-gcp/cluster-template-ci-gke-custom-subnet.yaml b/test/e2e/data/infrastructure-gcp/cluster-template-ci-gke-custom-subnet.yaml
@@ -0,0 +1,64 @@
+---
+apiVersion: cluster.x-k8s.io/v1beta1
+kind: Cluster
+metadata:
+  name: "${CLUSTER_NAME}"
+spec:
+  clusterNetwork:
+    pods:
+      cidrBlocks: ["192.168.0.0/16"]
+  infrastructureRef:
+    apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+    kind: GCPManagedCluster
+    name: "${CLUSTER_NAME}"
+  controlPlaneRef:
+    kind: GCPManagedControlPlane
+    apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+    name: "${CLUSTER_NAME}-control-plane"
+---
+apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+kind: GCPManagedCluster
+metadata:
+  name: "${CLUSTER_NAME}"
+spec:
+  project: "${GCP_PROJECT}"
+  region: "${GCP_REGION}"
+  network:
+    name: "${GCP_NETWORK_NAME}"
+    autoCreateSubnetworks: false
+    subnets:
+      - name: "${GCP_SUBNET_NAME}"
+        cidrBlock: "${GCP_SUBNET_CIDR}"
+        region: "${GCP_REGION}"
+---
+apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+kind: GCPManagedControlPlane
+metadata:
+  name: "${CLUSTER_NAME}-control-plane"
+spec:
+  project: "${GCP_PROJECT}"
+  location: "${GCP_REGION}"
+---
+apiVersion: cluster.x-k8s.io/v1beta1
+kind: MachinePool
+metadata:
+  name: ${CLUSTER_NAME}-mp-0
+spec:
+  clusterName: ${CLUSTER_NAME}
+  replicas: ${WORKER_MACHINE_COUNT}
+  template:
+    spec:
+      bootstrap:
+        dataSecretName: ""
+      clusterName: ${CLUSTER_NAME}
+      infrastructureRef:
+        apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+        kind: GCPManagedMachinePool
+        name: ${CLUSTER_NAME}-mp-0
+---
+apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+kind: GCPManagedMachinePool
+metadata:
+  name: ${CLUSTER_NAME}-mp-0
+spec: {}
+
diff --git a/test/e2e/e2e_gke_test.go b/test/e2e/e2e_gke_test.go
@@ -159,4 +159,33 @@ var _ = Describe("GKE workload cluster creation", func() {
 			}, result)
 		})
 	})
+
+	Context("Creating a GKE cluster with custom subnet", func() {
+		It("Should create a cluster with 3 machine pool and custom subnet", func() {
+			By("Initializes with 3 machine pool")
+
+			ApplyManagedClusterTemplateAndWait(ctx, ApplyManagedClusterTemplateAndWaitInput{
+				ClusterProxy: bootstrapClusterProxy,
+				ConfigCluster: clusterctl.ConfigClusterInput{
+					LogFolder:                clusterctlLogFolder,
+					ClusterctlConfigPath:     clusterctlConfigPath,
+					KubeconfigPath:           bootstrapClusterProxy.GetKubeconfigPath(),
+					InfrastructureProvider:   clusterctl.DefaultInfrastructureProvider,
+					Flavor:                   "ci-gke-custom-subnet",
+					Namespace:                namespace.Name,
+					ClusterName:              clusterName,
+					KubernetesVersion:        e2eConfig.GetVariable(KubernetesVersion),
+					ControlPlaneMachineCount: ptr.To[int64](1),
+					WorkerMachineCount:       ptr.To[int64](3),
+					ClusterctlVariables: map[string]string{
+						"GCP_SUBNET_NAME": "capg-test-subnet",
+						"GCP_SUBNET_CIDR": "172.20.0.0/16",
+					},
+				},
+				WaitForClusterIntervals:      e2eConfig.GetIntervals(specName, "wait-cluster"),
+				WaitForControlPlaneIntervals: e2eConfig.GetIntervals(specName, "wait-control-plane"),
+				WaitForMachinePools:          e2eConfig.GetIntervals(specName, "wait-worker-machine-pools"),
+			}, result)
+		})
+	})
 })