Skip to content

Commit

Permalink
Fix GKE Cluster Creation for User Provided Subnet
Browse files Browse the repository at this point in the history
Signed-off-by: Shaad7 <[email protected]>
  • Loading branch information
AbdullahAlShaad committed Feb 1, 2024
1 parent 245e21e commit 073e360
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 6 deletions.
12 changes: 10 additions & 2 deletions cloud/services/compute/subnets/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) {
Expand All @@ -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()
}
16 changes: 13 additions & 3 deletions cloud/services/container/clusters/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,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,
},
Expand Down Expand Up @@ -294,6 +294,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 {
Expand Down
36 changes: 35 additions & 1 deletion exp/api/v1beta1/gcpmanagedcluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
1 change: 1 addition & 0 deletions test/e2e/config/gcp-ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.27.3}"
Expand Down
Original file line number Diff line number Diff line change
@@ -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: {}

29 changes: 29 additions & 0 deletions test/e2e/e2e_gke_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: pointer.Int64Ptr(1),
WorkerMachineCount: pointer.Int64Ptr(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)
})
})
})

0 comments on commit 073e360

Please sign in to comment.