gce: Always create an internal load balancer

When we create a external load balancer on GCE, we now also create an internal load balancer. The internal load balancer is used for node/pod -> control-plane traffic, the external load balancer is used for other traffic (e.g. "user" traffic to kube-apiserver). This means that we can apply more granular firewall rules, and generally avoid complex logic around discovery of the internal control plane addresses for GCE.
justinsb · Jan 12, 2024 · 2a0131b · 2a0131b
1 parent b605ebd
commit 2a0131b
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 13 deletions.
diff --git a/pkg/model/gcemodel/api_loadbalancer.go b/pkg/model/gcemodel/api_loadbalancer.go
@@ -92,6 +92,7 @@ func (b *APILoadBalancerBuilder) createPublicLB(c *fi.CloudupModelBuilderContext
 	if b.Cluster.UsesNoneDNS() {
 		ipAddress.WellKnownServices = append(ipAddress.WellKnownServices, wellknownservices.KopsController)
 
+		// TODO: Can we replace this?
 		c.AddTask(&gcetasks.ForwardingRule{
 			Name:                s(b.NameForForwardingRule("kops-controller")),
 			Lifecycle:           b.Lifecycle,
@@ -107,7 +108,22 @@ func (b *APILoadBalancerBuilder) createPublicLB(c *fi.CloudupModelBuilderContext
 		})
 	}
 
-	return b.addFirewallRules(c)
+	// // TODO: Can we replace this?
+	// c.AddTask(&gcetasks.ForwardingRule{
+	// 	Name:                s(b.NameForForwardingRule("api-internal")),
+	// 	Lifecycle:           b.Lifecycle,
+	// 	PortRange:           s(strconv.Itoa(wellknownports.KopsControllerPort) + "-" + strconv.Itoa(wellknownports.KopsControllerPort)),
+	// 	TargetPool:          targetPool,
+	// 	IPAddress:           ipAddress,
+	// 	IPProtocol:          "TCP",
+	// 	LoadBalancingScheme: s("INTERNAL"),
+	// 	Labels: map[string]string{
+	// 		clusterLabel.Key: clusterLabel.Value,
+	// 		"name":           "kops-controller",
+	// 	},
+	// })
+
+	return nil
 }
 
 func (b *APILoadBalancerBuilder) addFirewallRules(c *fi.CloudupModelBuilderContext) error {
@@ -252,7 +268,7 @@ func (b *APILoadBalancerBuilder) createInternalLB(c *fi.CloudupModelBuilderConte
 			})
 		}
 	}
-	return b.addFirewallRules(c)
+	return nil
 }
 
 func (b *APILoadBalancerBuilder) Build(c *fi.CloudupModelBuilderContext) error {
@@ -268,22 +284,33 @@ func (b *APILoadBalancerBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 
 	switch lbSpec.Type {
 	case kops.LoadBalancerTypePublic:
-		return b.createPublicLB(c)
+		if err := b.createPublicLB(c); err != nil {
+			return err
+		}
+		if err := b.createInternalLB(c); err != nil {
+			return err
+		}
+
+		return b.addFirewallRules(c)
 
 	case kops.LoadBalancerTypeInternal:
-		return b.createInternalLB(c)
+		if err := b.createInternalLB(c); err != nil {
+			return err
+		}
+
+		return b.addFirewallRules(c)
 
 	default:
 		return fmt.Errorf("unhandled LoadBalancer type %q", lbSpec.Type)
 	}
 }
 
-// subnetNotSpecified returns true if the given LB subnet is not listed in the list of cluster subnets.
-func subnetNotSpecified(sn kops.LoadBalancerSubnetSpec, subnets []kops.ClusterSubnetSpec) bool {
-	for _, csn := range subnets {
-		if csn.Name == sn.Name || csn.ID == sn.Name {
-			return false
-		}
-	}
-	return true
-}
+// // subnetNotSpecified returns true if the given LB subnet is not listed in the list of cluster subnets.
+// func subnetNotSpecified(sn kops.LoadBalancerSubnetSpec, subnets []kops.ClusterSubnetSpec) bool {
+// 	for _, csn := range subnets {
+// 		if csn.Name == sn.Name || csn.ID == sn.Name {
+// 			return false
+// 		}
+// 	}
+// 	return true
+// }
diff --git a/tests/integration/update_cluster/minimal_gce_plb/kubernetes.tf b/tests/integration/update_cluster/minimal_gce_plb/kubernetes.tf
@@ -182,6 +182,23 @@ resource "google_compute_address" "api-minimal-gce-plb-example-com" {
   name = "api-minimal-gce-plb-example-com"
 }
 
+resource "google_compute_address" "api-us-test1-minimal-gce-plb-example-com" {
+  address_type = "INTERNAL"
+  name         = "api-us-test1-minimal-gce-plb-example-com"
+  purpose      = "SHARED_LOADBALANCER_VIP"
+  subnetwork   = google_compute_subnetwork.us-test1-minimal-gce-plb-example-com.name
+}
+
+resource "google_compute_backend_service" "api-minimal-gce-plb-example-com" {
+  backend {
+    group = google_compute_instance_group_manager.a-master-us-test1-a-minimal-gce-plb-example-com.instance_group
+  }
+  health_checks         = [google_compute_health_check.api-minimal-gce-plb-example-com.id]
+  load_balancing_scheme = "INTERNAL_SELF_MANAGED"
+  name                  = "api-minimal-gce-plb-example-com"
+  protocol              = "TCP"
+}
+
 resource "google_compute_disk" "a-etcd-events-minimal-gce-plb-example-com" {
   labels = {
     "k8s-io-cluster-name" = "minimal-gce-plb-example-com"
@@ -432,6 +449,28 @@ resource "google_compute_forwarding_rule" "api-minimal-gce-plb-example-com" {
   target                = google_compute_target_pool.api-minimal-gce-plb-example-com.self_link
 }
 
+resource "google_compute_forwarding_rule" "api-us-test1-minimal-gce-plb-example-com" {
+  backend_service = google_compute_backend_service.api-minimal-gce-plb-example-com.id
+  ip_address      = google_compute_address.api-us-test1-minimal-gce-plb-example-com.address
+  ip_protocol     = "TCP"
+  labels = {
+    "k8s-io-cluster-name" = "minimal-gce-plb-example-com"
+    "name"                = "api-us-test1"
+  }
+  load_balancing_scheme = "INTERNAL"
+  name                  = "api-us-test1-minimal-gce-plb-example-com"
+  network               = google_compute_network.minimal-gce-plb-example-com.name
+  ports                 = ["443"]
+  subnetwork            = google_compute_subnetwork.us-test1-minimal-gce-plb-example-com.name
+}
+
+resource "google_compute_health_check" "api-minimal-gce-plb-example-com" {
+  name = "api-minimal-gce-plb-example-com"
+  tcp_health_check {
+    port = 443
+  }
+}
+
 resource "google_compute_http_health_check" "api-minimal-gce-plb-example-com" {
   name         = "api-minimal-gce-plb-example-com"
   port         = 3990