Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Cluster Creation in User Provided Subnet #961

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 10 additions & 2 deletions cloud/services/compute/subnets/reconcile.go
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
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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 {
Expand Down
36 changes: 35 additions & 1 deletion exp/api/v1beta1/gcpmanagedcluster_webhook.go
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
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.28.3}"
Expand Down
@@ -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
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: 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)
})
})
})