diff --git a/pkg/model/gcemodel/api_loadbalancer.go b/pkg/model/gcemodel/api_loadbalancer.go index 65c10a8fbb..90f75a9f3d 100644 --- a/pkg/model/gcemodel/api_loadbalancer.go +++ b/pkg/model/gcemodel/api_loadbalancer.go @@ -25,6 +25,7 @@ import ( "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/gce" "k8s.io/kops/upup/pkg/fi/cloudup/gcetasks" + "k8s.io/utils/strings/slices" ) // APILoadBalancerBuilder builds a LoadBalancer for accessing the API @@ -132,7 +133,6 @@ func createPublicLB(b *APILoadBalancerBuilder, c *fi.CloudupModelBuilderContext) // GCP this entails creating a health check, backend service, and one forwarding rule // per specified subnet pointing to that backend service. func createInternalLB(b *APILoadBalancerBuilder, c *fi.CloudupModelBuilderContext) error { - lbSpec := b.Cluster.Spec.API.LoadBalancer hc := &gcetasks.HealthCheck{ Name: s(b.NameForHealthCheck("api")), Port: wellknownports.KubeAPIServer, @@ -162,30 +162,40 @@ func createInternalLB(b *APILoadBalancerBuilder, c *fi.CloudupModelBuilderContex InstanceGroupManagers: igms, } c.AddTask(bs) - for _, sn := range lbSpec.Subnets { - network, err := b.LinkToNetwork() - if err != nil { - return err + + network, err := b.LinkToNetwork() + if err != nil { + return err + } + + for _, sn := range b.Cluster.Spec.Networking.Subnets { + var subnet *gcetasks.Subnet + for _, ig := range b.InstanceGroups { + if ig.HasAPIServer() && slices.Contains(ig.Spec.Subnets, sn.Name) { + subnet = b.LinkToSubnet(&sn) + break + } } - t := true - subnet := &gcetasks.Subnet{ - Name: s(sn.Name), - Network: network, - Shared: &t, - // Override lifecycle because these subnets are specified - // to already exist. - Lifecycle: fi.LifecycleExistsAndWarnIfChanges, + if subnet == nil { + continue } - // TODO: automatically associate forwarding rule to subnets if no subnets are specified here. - if subnetNotSpecified(sn, b.Cluster.Spec.Networking.Subnets) { - c.AddTask(subnet) + + ipAddress := &gcetasks.Address{ + Name: s(b.NameForIPAddress("api-" + sn.Name)), + IPAddressType: s("INTERNAL"), + Purpose: s("SHARED_LOADBALANCER_VIP"), + Subnetwork: subnet, + ForAPIServer: true, + Lifecycle: b.Lifecycle, } + c.AddTask(ipAddress) + c.AddTask(&gcetasks.ForwardingRule{ - Name: s(b.NameForForwardingRule(sn.Name)), + Name: s(b.NameForForwardingRule("api-" + sn.Name)), Lifecycle: b.Lifecycle, BackendService: bs, Ports: []string{strconv.Itoa(wellknownports.KubeAPIServer)}, - RuleIPAddress: sn.PrivateIPv4Address, + IPAddress: ipAddress, IPProtocol: "TCP", LoadBalancingScheme: s("INTERNAL"), Network: network, @@ -197,7 +207,7 @@ func createInternalLB(b *APILoadBalancerBuilder, c *fi.CloudupModelBuilderContex Lifecycle: b.Lifecycle, BackendService: bs, Ports: []string{strconv.Itoa(wellknownports.KopsControllerPort)}, - RuleIPAddress: sn.PrivateIPv4Address, + IPAddress: ipAddress, IPProtocol: "TCP", LoadBalancingScheme: s("INTERNAL"), Network: network, diff --git a/pkg/model/gcemodel/firewall.go b/pkg/model/gcemodel/firewall.go index 70b1789720..3843eb743c 100644 --- a/pkg/model/gcemodel/firewall.go +++ b/pkg/model/gcemodel/firewall.go @@ -46,6 +46,28 @@ func (b *FirewallModelBuilder) Build(c *fi.CloudupModelBuilderContext) error { allProtocols = append(allProtocols, "ipip") } + // Allow all TCP traffic from load balancer health checks + if b.Cluster.Spec.API.LoadBalancer != nil { + network, err := b.LinkToNetwork() + if err != nil { + return err + } + c.AddTask(&gcetasks.FirewallRule{ + Name: s(b.NameForFirewallRule("lb-health-checks")), + Lifecycle: b.Lifecycle, + Network: network, + Family: gcetasks.AddressFamilyIPv4, + SourceRanges: []string{ + // IP ranges for load balancer health checks + // https://cloud.google.com/load-balancing/docs/health-checks + "35.191.0.0/16", + "130.211.0.0/22", + }, + TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleControlPlane)}, + Allowed: []string{"tcp"}, + }) + } + // Allow all traffic from nodes -> nodes { network, err := b.LinkToNetwork() diff --git a/tests/integration/update_cluster/minimal_gce_dns-none/kubernetes.tf b/tests/integration/update_cluster/minimal_gce_dns-none/kubernetes.tf index 9e25652912..62fd945108 100644 --- a/tests/integration/update_cluster/minimal_gce_dns-none/kubernetes.tf +++ b/tests/integration/update_cluster/minimal_gce_dns-none/kubernetes.tf @@ -170,6 +170,13 @@ resource "aws_s3_object" "nodeupconfig-nodes" { server_side_encryption = "AES256" } +resource "google_compute_address" "api-us-test1-minimal-gce-example-com" { + address_type = "INTERNAL" + name = "api-us-test1-minimal-gce-example-com" + purpose = "SHARED_LOADBALANCER_VIP" + subnetwork = google_compute_subnetwork.us-test1-minimal-gce-example-com.name +} + resource "google_compute_backend_service" "api-minimal-gce-example-com" { backend { group = google_compute_instance_group_manager.a-master-us-test1-a-minimal-gce-example-com.instance_group @@ -204,6 +211,17 @@ resource "google_compute_disk" "a-etcd-main-minimal-gce-example-com" { zone = "us-test1-a" } +resource "google_compute_firewall" "lb-health-checks-minimal-gce-example-com" { + allow { + protocol = "tcp" + } + disabled = false + name = "lb-health-checks-minimal-gce-example-com" + network = google_compute_network.minimal-gce-example-com.name + source_ranges = ["35.191.0.0/16", "130.211.0.0/22"] + target_tags = ["minimal-gce-example-com-k8s-io-role-control-plane"] +} + resource "google_compute_firewall" "master-to-master-minimal-gce-example-com" { allow { protocol = "tcp" @@ -382,24 +400,26 @@ resource "google_compute_firewall" "ssh-external-to-node-minimal-gce-example-com target_tags = ["minimal-gce-example-com-k8s-io-role-node"] } -resource "google_compute_forwarding_rule" "kops-controller-us-test-1-minimal-gce-example-com" { +resource "google_compute_forwarding_rule" "api-us-test1-minimal-gce-example-com" { backend_service = google_compute_backend_service.api-minimal-gce-example-com.id + ip_address = google_compute_address.api-us-test1-minimal-gce-example-com.address ip_protocol = "TCP" load_balancing_scheme = "INTERNAL" - name = "kops-controller-us-test-1-minimal-gce-example-com" - network = google_compute_network.minimal-gce-example-com.name - ports = ["3988"] - subnetwork = "us-test-1" -} - -resource "google_compute_forwarding_rule" "us-test-1-minimal-gce-example-com" { - backend_service = google_compute_backend_service.api-minimal-gce-example-com.id - ip_protocol = "TCP" - load_balancing_scheme = "INTERNAL" - name = "us-test-1-minimal-gce-example-com" + name = "api-us-test1-minimal-gce-example-com" network = google_compute_network.minimal-gce-example-com.name ports = ["443"] - subnetwork = "us-test-1" + subnetwork = google_compute_subnetwork.us-test1-minimal-gce-example-com.name +} + +resource "google_compute_forwarding_rule" "kops-controller-us-test1-minimal-gce-example-com" { + backend_service = google_compute_backend_service.api-minimal-gce-example-com.id + ip_address = google_compute_address.api-us-test1-minimal-gce-example-com.address + ip_protocol = "TCP" + load_balancing_scheme = "INTERNAL" + name = "kops-controller-us-test1-minimal-gce-example-com" + network = google_compute_network.minimal-gce-example-com.name + ports = ["3988"] + subnetwork = google_compute_subnetwork.us-test1-minimal-gce-example-com.name } resource "google_compute_health_check" "api-minimal-gce-example-com" { diff --git a/tests/integration/update_cluster/minimal_gce_ilb/kubernetes.tf b/tests/integration/update_cluster/minimal_gce_ilb/kubernetes.tf index 7a6efb9874..aeb6d4eaae 100644 --- a/tests/integration/update_cluster/minimal_gce_ilb/kubernetes.tf +++ b/tests/integration/update_cluster/minimal_gce_ilb/kubernetes.tf @@ -178,6 +178,13 @@ resource "aws_s3_object" "nodeupconfig-nodes" { server_side_encryption = "AES256" } +resource "google_compute_address" "api-us-test1-minimal-gce-ilb-example-com" { + address_type = "INTERNAL" + name = "api-us-test1-minimal-gce-ilb-example-com" + purpose = "SHARED_LOADBALANCER_VIP" + subnetwork = google_compute_subnetwork.us-test1-minimal-gce-ilb-example-com.name +} + resource "google_compute_backend_service" "api-minimal-gce-ilb-example-com" { backend { group = google_compute_instance_group_manager.a-master-us-test1-a-minimal-gce-ilb-example-com.instance_group @@ -212,6 +219,17 @@ resource "google_compute_disk" "a-etcd-main-minimal-gce-ilb-example-com" { zone = "us-test1-a" } +resource "google_compute_firewall" "lb-health-checks-minimal-gce-ilb-example-com" { + allow { + protocol = "tcp" + } + disabled = false + name = "lb-health-checks-minimal-gce-ilb-example-com" + network = google_compute_network.minimal-gce-ilb-example-com.name + source_ranges = ["35.191.0.0/16", "130.211.0.0/22"] + target_tags = ["minimal-gce-ilb-example-com-k8s-io-role-control-plane"] +} + resource "google_compute_firewall" "master-to-master-minimal-gce-ilb-example-com" { allow { protocol = "tcp" @@ -390,14 +408,15 @@ resource "google_compute_firewall" "ssh-external-to-node-minimal-gce-ilb-example target_tags = ["minimal-gce-ilb-example-com-k8s-io-role-node"] } -resource "google_compute_forwarding_rule" "us-test-1-minimal-gce-ilb-example-com" { +resource "google_compute_forwarding_rule" "api-us-test1-minimal-gce-ilb-example-com" { backend_service = google_compute_backend_service.api-minimal-gce-ilb-example-com.id + ip_address = google_compute_address.api-us-test1-minimal-gce-ilb-example-com.address ip_protocol = "TCP" load_balancing_scheme = "INTERNAL" - name = "us-test-1-minimal-gce-ilb-example-com" + name = "api-us-test1-minimal-gce-ilb-example-com" network = google_compute_network.minimal-gce-ilb-example-com.name ports = ["443"] - subnetwork = "us-test-1" + subnetwork = google_compute_subnetwork.us-test1-minimal-gce-ilb-example-com.name } resource "google_compute_health_check" "api-minimal-gce-ilb-example-com" { diff --git a/tests/integration/update_cluster/minimal_gce_ilb_longclustername/kubernetes.tf b/tests/integration/update_cluster/minimal_gce_ilb_longclustername/kubernetes.tf index 0105775dff..8f29e87b1d 100644 --- a/tests/integration/update_cluster/minimal_gce_ilb_longclustername/kubernetes.tf +++ b/tests/integration/update_cluster/minimal_gce_ilb_longclustername/kubernetes.tf @@ -178,6 +178,13 @@ resource "aws_s3_object" "nodeupconfig-nodes" { server_side_encryption = "AES256" } +resource "google_compute_address" "api-us-test1-minimal-gce-with-a-very-very-very-very-very-96dqvi" { + address_type = "INTERNAL" + name = "api-us-test1-minimal-gce-with-a-very-very-very-very-very-96dqvi" + purpose = "SHARED_LOADBALANCER_VIP" + subnetwork = google_compute_subnetwork.us-test1-minimal-gce-with-a-very-very-very-very-very-lon-96dqvi.name +} + resource "google_compute_backend_service" "api-minimal-gce-with-a-very-very-very-very-very-long-name-example-com" { backend { group = google_compute_instance_group_manager.a-master-us-test1-a-minimal-gce-with-a-very-very-very-ve-j0fh8f.instance_group @@ -212,6 +219,17 @@ resource "google_compute_disk" "a-etcd-main-minimal-gce-with-a-very-very-very-ve zone = "us-test1-a" } +resource "google_compute_firewall" "lb-health-checks-minimal-gce-with-a-very-very-very-very--96dqvi" { + allow { + protocol = "tcp" + } + disabled = false + name = "lb-health-checks-minimal-gce-with-a-very-very-very-very--96dqvi" + network = google_compute_network.minimal-gce-with-a-very-very-very-very-very-long-name-ex-96dqvi.name + source_ranges = ["35.191.0.0/16", "130.211.0.0/22"] + target_tags = ["minimal-gce-with-a-very-very-v-96dqvi-k8s-io-role-control-plane"] +} + resource "google_compute_firewall" "master-to-master-minimal-gce-with-a-very-very-very-very--96dqvi" { allow { protocol = "tcp" @@ -390,14 +408,15 @@ resource "google_compute_firewall" "ssh-external-to-node-minimal-gce-with-a-very target_tags = ["minimal-gce-with-a-very-very-very-very--96dqvi-k8s-io-role-node"] } -resource "google_compute_forwarding_rule" "us-test-1-minimal-gce-with-a-very-very-very-very-very-lo-96dqvi" { +resource "google_compute_forwarding_rule" "api-us-test1-minimal-gce-with-a-very-very-very-very-very-96dqvi" { backend_service = google_compute_backend_service.api-minimal-gce-with-a-very-very-very-very-very-long-name-example-com.id + ip_address = google_compute_address.api-us-test1-minimal-gce-with-a-very-very-very-very-very-96dqvi.address ip_protocol = "TCP" load_balancing_scheme = "INTERNAL" - name = "us-test-1-minimal-gce-with-a-very-very-very-very-very-lo-96dqvi" + name = "api-us-test1-minimal-gce-with-a-very-very-very-very-very-96dqvi" network = google_compute_network.minimal-gce-with-a-very-very-very-very-very-long-name-ex-96dqvi.name ports = ["443"] - subnetwork = "us-test-1" + subnetwork = google_compute_subnetwork.us-test1-minimal-gce-with-a-very-very-very-very-very-lon-96dqvi.name } resource "google_compute_health_check" "api-minimal-gce-with-a-very-very-very-very-very-long-name-example-com" { diff --git a/tests/integration/update_cluster/minimal_gce_plb/kubernetes.tf b/tests/integration/update_cluster/minimal_gce_plb/kubernetes.tf index c2678c41e3..f4cbced6b0 100644 --- a/tests/integration/update_cluster/minimal_gce_plb/kubernetes.tf +++ b/tests/integration/update_cluster/minimal_gce_plb/kubernetes.tf @@ -230,6 +230,17 @@ resource "google_compute_firewall" "https-api-minimal-gce-plb-example-com" { target_tags = ["minimal-gce-plb-example-com-k8s-io-role-control-plane"] } +resource "google_compute_firewall" "lb-health-checks-minimal-gce-plb-example-com" { + allow { + protocol = "tcp" + } + disabled = false + name = "lb-health-checks-minimal-gce-plb-example-com" + network = google_compute_network.minimal-gce-plb-example-com.name + source_ranges = ["35.191.0.0/16", "130.211.0.0/22"] + target_tags = ["minimal-gce-plb-example-com-k8s-io-role-control-plane"] +} + resource "google_compute_firewall" "master-to-master-minimal-gce-plb-example-com" { allow { protocol = "tcp" diff --git a/upup/pkg/fi/cloudup/gce/gce_cloud.go b/upup/pkg/fi/cloudup/gce/gce_cloud.go index 4481ac6be5..445fd62a80 100644 --- a/upup/pkg/fi/cloudup/gce/gce_cloud.go +++ b/upup/pkg/fi/cloudup/gce/gce_cloud.go @@ -309,10 +309,7 @@ func (c *gceCloudImplementation) WaitForOp(op *compute.Operation) error { func (c *gceCloudImplementation) GetApiIngressStatus(cluster *kops.Cluster) ([]fi.ApiIngressStatus, error) { var ingresses []fi.ApiIngressStatus - // Note that this must match GCEModelContext::NameForForwardingRule - name := ClusterSuffixedName("api", cluster.ObjectMeta.Name, 63) - - klog.V(2).Infof("Querying GCE to find ForwardingRules for API (%q)", name) + klog.V(2).Infof("Querying GCE to find ForwardingRules for API") // These are the ingress rules, so we search for them in the network project. _, project, err := ParseNameAndProjectFromNetworkID(cluster.Spec.Networking.NetworkID) if err != nil { @@ -321,18 +318,21 @@ func (c *gceCloudImplementation) GetApiIngressStatus(cluster *kops.Cluster) ([]f project = c.Project() } - forwardingRule, err := c.compute.ForwardingRules().Get(project, c.region, name) + forwardingRules, err := c.compute.ForwardingRules().List(context.Background(), project, c.region) if err != nil { if !IsNotFound(err) { - forwardingRule = nil + forwardingRules = nil } else { - return nil, fmt.Errorf("error getting ForwardingRule %q: %v", name, err) + return nil, fmt.Errorf("error listing ForwardingRules: %v", err) } } - if forwardingRule != nil { + for _, forwardingRule := range forwardingRules { + if !strings.HasPrefix(forwardingRule.Name, "api-") { + continue + } if forwardingRule.IPAddress == "" { - return nil, fmt.Errorf("found forward rule %q, but it did not have an IPAddress", name) + return nil, fmt.Errorf("found forward rule %q, but it did not have an IPAddress", forwardingRule.Name) } ingresses = append(ingresses, fi.ApiIngressStatus{ diff --git a/upup/pkg/fi/cloudup/gcetasks/address.go b/upup/pkg/fi/cloudup/gcetasks/address.go index cee9af83af..82e24053a8 100644 --- a/upup/pkg/fi/cloudup/gcetasks/address.go +++ b/upup/pkg/fi/cloudup/gcetasks/address.go @@ -32,8 +32,12 @@ type Address struct { Name *string Lifecycle fi.Lifecycle - IPAddress *string - ForAPIServer bool + IPAddress *string + IPAddressType *string + Purpose *string + ForAPIServer bool + + Subnetwork *Subnet } var _ fi.CompareWithID = &ForwardingRule{} @@ -72,6 +76,8 @@ func findAddressByIP(cloud gce.GCECloud, ip string) (*Address, error) { actual := &Address{} actual.IPAddress = &addrs[0].Address + actual.IPAddressType = &addrs[0].AddressType + actual.Purpose = &addrs[0].Purpose actual.Name = &addrs[0].Name return actual, nil @@ -89,7 +95,14 @@ func (e *Address) find(cloud gce.GCECloud) (*Address, error) { actual := &Address{} actual.IPAddress = &r.Address + actual.IPAddressType = &r.AddressType + actual.Purpose = &r.Purpose actual.Name = &r.Name + if e.Subnetwork != nil { + actual.Subnetwork = &Subnet{ + Name: fi.PtrTo(lastComponent(r.Subnetwork)), + } + } return actual, nil } @@ -130,13 +143,19 @@ func (_ *Address) CheckChanges(a, e, changes *Address) error { func (_ *Address) RenderGCE(t *gce.GCEAPITarget, a, e, changes *Address) error { cloud := t.Cloud addr := &compute.Address{ - Name: *e.Name, - Address: fi.ValueOf(e.IPAddress), - Region: cloud.Region(), + Name: *e.Name, + Address: fi.ValueOf(e.IPAddress), + AddressType: fi.ValueOf(e.IPAddressType), + Purpose: fi.ValueOf(e.Purpose), + Region: cloud.Region(), + } + + if e.Subnetwork != nil { + addr.Subnetwork = e.Subnetwork.URL(t.Cloud.Project(), t.Cloud.Region()) } if a == nil { - klog.Infof("GCE creating address: %q", addr.Name) + klog.V(2).Infof("Creating Address: %q", addr.Name) op, err := cloud.Compute().Addresses().Insert(cloud.Project(), cloud.Region(), addr) if err != nil { @@ -154,12 +173,20 @@ func (_ *Address) RenderGCE(t *gce.GCEAPITarget, a, e, changes *Address) error { } type terraformAddress struct { - Name *string `cty:"name"` + Name *string `cty:"name"` + AddressType *string `cty:"address_type"` + Purpose *string `cty:"purpose"` + Subnetwork *terraformWriter.Literal `cty:"subnetwork"` } func (_ *Address) RenderTerraform(t *terraform.TerraformTarget, a, e, changes *Address) error { tf := &terraformAddress{ - Name: e.Name, + Name: e.Name, + AddressType: e.IPAddressType, + Purpose: e.Purpose, + } + if e.Subnetwork != nil { + tf.Subnetwork = e.Subnetwork.TerraformLink() } return t.RenderResource("google_compute_address", *e.Name, tf) } diff --git a/upup/pkg/fi/cloudup/gcetasks/backend_service.go b/upup/pkg/fi/cloudup/gcetasks/backend_service.go index d2068a8708..afeee3e990 100644 --- a/upup/pkg/fi/cloudup/gcetasks/backend_service.go +++ b/upup/pkg/fi/cloudup/gcetasks/backend_service.go @@ -124,7 +124,7 @@ func (_ *BackendService) RenderGCE(t *gce.GCEAPITarget, a, e, changes *BackendSe } if a == nil { - klog.Infof("GCE creating backend service: %q", bs.Name) + klog.V(2).Infof("Creating BackendService: %q", bs.Name) op, err := cloud.Compute().RegionBackendServices().Insert(cloud.Project(), cloud.Region(), bs) if err != nil { diff --git a/upup/pkg/fi/cloudup/gcetasks/healthcheck.go b/upup/pkg/fi/cloudup/gcetasks/healthcheck.go index 69b83f2e9b..f6bdd7a932 100644 --- a/upup/pkg/fi/cloudup/gcetasks/healthcheck.go +++ b/upup/pkg/fi/cloudup/gcetasks/healthcheck.go @@ -108,7 +108,7 @@ func (_ *HealthCheck) RenderGCE(t *gce.GCEAPITarget, a, e, changes *HealthCheck) } if a == nil { - klog.Infof("GCE creating healthcheck: %q", hc.Name) + klog.V(2).Infof("Creating HealthCheck %q", hc.Name) op, err := cloud.Compute().RegionHealthChecks().Insert(cloud.Project(), cloud.Region(), hc) if err != nil {