From da233efe11ee0e8e29132bc081488f0b0687d65c Mon Sep 17 00:00:00 2001 From: justinsb Date: Fri, 12 Jan 2024 21:44:06 -0500 Subject: [PATCH] gce: Prune old forwarding rules Now that we create an new forwarding rule for kops-controller, we want to remove the old one after the rolling-update. --- pkg/model/gcemodel/api_loadbalancer.go | 25 ++--- .../pkg/fi/cloudup/gcetasks/forwardingrule.go | 92 +++++++++++++++++++ 2 files changed, 98 insertions(+), 19 deletions(-) diff --git a/pkg/model/gcemodel/api_loadbalancer.go b/pkg/model/gcemodel/api_loadbalancer.go index 1808abcad9..7d97d10801 100644 --- a/pkg/model/gcemodel/api_loadbalancer.go +++ b/pkg/model/gcemodel/api_loadbalancer.go @@ -87,23 +87,6 @@ func (b *APILoadBalancerBuilder) createPublicLB(c *fi.CloudupModelBuilderContext "name": "api", }, }) - if b.Cluster.UsesNoneDNS() { - ipAddress.WellKnownServices = append(ipAddress.WellKnownServices, wellknownservices.KopsController) - - c.AddTask(&gcetasks.ForwardingRule{ - Name: s(b.NameForForwardingRule("kops-controller")), - Lifecycle: b.Lifecycle, - PortRange: s(strconv.Itoa(wellknownports.KopsControllerPort) + "-" + strconv.Itoa(wellknownports.KopsControllerPort)), - TargetPool: targetPool, - IPAddress: ipAddress, - IPProtocol: "TCP", - LoadBalancingScheme: s("EXTERNAL"), - Labels: map[string]string{ - clusterLabel.Key: clusterLabel.Value, - "name": "kops-controller", - }, - }) - } return nil } @@ -231,7 +214,7 @@ func (b *APILoadBalancerBuilder) createInternalLB(c *fi.CloudupModelBuilderConte if b.Cluster.UsesNoneDNS() { ipAddress.WellKnownServices = append(ipAddress.WellKnownServices, wellknownservices.KopsController) - c.AddTask(&gcetasks.ForwardingRule{ + fr := &gcetasks.ForwardingRule{ Name: s(b.NameForForwardingRule("kops-controller-" + sn.Name)), Lifecycle: b.Lifecycle, BackendService: bs, @@ -245,7 +228,11 @@ func (b *APILoadBalancerBuilder) createInternalLB(c *fi.CloudupModelBuilderConte clusterLabel.Key: clusterLabel.Value, "name": "kops-controller-" + sn.Name, }, - }) + } + // We previously created a forwarding rule which was external; prune it + fr.PruneForwardingRulesWithName(b.NameForForwardingRule("kops-controller")) //, "Removing legacy external load balancer for kops-controller") + + c.AddTask(fr) } } return nil diff --git a/upup/pkg/fi/cloudup/gcetasks/forwardingrule.go b/upup/pkg/fi/cloudup/gcetasks/forwardingrule.go index dbc79b44c2..3ac1bf5623 100644 --- a/upup/pkg/fi/cloudup/gcetasks/forwardingrule.go +++ b/upup/pkg/fi/cloudup/gcetasks/forwardingrule.go @@ -56,6 +56,13 @@ type ForwardingRule struct { // Fingerprint of the labels, used to avoid race-conditions on updates. // Only set on the actual resource returned by Find. labelFingerprint string + + // pruneForwardingRules will prune any forwarding rules found with the specified names + pruneForwardingRules []forwardingRulePruneSpec +} + +type forwardingRulePruneSpec struct { + Name string } var _ fi.CompareWithID = &ForwardingRule{} @@ -64,6 +71,10 @@ func (e *ForwardingRule) CompareWithID() *string { return e.Name } +func (e *ForwardingRule) PruneForwardingRulesWithName(name string) { + e.pruneForwardingRules = append(e.pruneForwardingRules, forwardingRulePruneSpec{Name: name}) +} + func (e *ForwardingRule) Find(c *fi.CloudupContext) (*ForwardingRule, error) { ctx := c.Context() @@ -225,6 +236,7 @@ func (_ *ForwardingRule) RenderGCE(t *gce.GCEAPITarget, a, e, changes *Forwardin if e.Labels != nil { // We can't set labels on creation; we have to read the object to get the fingerprint + // TODO: We could get it from the operation! r, err := t.Cloud.Compute().ForwardingRules().Get(ctx, t.Cloud.Project(), t.Cloud.Region(), name) if err != nil { return fmt.Errorf("reading created ForwardingRule %q: %v", name, err) @@ -325,3 +337,83 @@ func (e *ForwardingRule) TerraformLink() *terraformWriter.Literal { return terraformWriter.LiteralSelfLink("google_compute_forwarding_rule", name) } + +var _ fi.CloudupProducesDeletions = &ForwardingRule{} + +// FindDeletions implements fi.HasDeletions +func (e *ForwardingRule) FindDeletions(c *fi.CloudupContext) ([]fi.CloudupDeletion, error) { + var removals []fi.CloudupDeletion + + if len(e.pruneForwardingRules) != 0 { + ctx := c.Context() + cloud := c.T.Cloud.(gce.GCECloud) + + forwardingRules, err := cloud.Compute().ForwardingRules().List(ctx, cloud.Project(), cloud.Region()) + if err != nil { + return nil, fmt.Errorf("listing forwardingRules: %w", err) + } + + for _, forwardingRule := range forwardingRules { + prune := false + + for _, rule := range e.pruneForwardingRules { + if rule.Name == forwardingRule.Name { + prune = true + } + } + + if prune { + removals = append(removals, &deleteForwardingRule{forwardingRule: forwardingRule}) + } + } + } + + return removals, nil +} + +// deleteForwardingRule tracks a ForwardingRule that we're going to delete +// It implements fi.Deletion +type deleteForwardingRule struct { + forwardingRule *compute.ForwardingRule +} + +var _ fi.CloudupDeletion = &deleteForwardingRule{} + +// TaskName returns the task name +func (d *deleteForwardingRule) TaskName() string { + return "ForwardingRule" +} + +// Item returns the forwarding rule name +func (d *deleteForwardingRule) Item() string { + return d.forwardingRule.Name +} + +func (d *deleteForwardingRule) Delete(t fi.CloudupTarget) error { + ctx := context.TODO() + + gceTarget, ok := t.(*gce.GCEAPITarget) + if !ok { + return fmt.Errorf("unexpected target type for deletion: %T", t) + } + + cloud := gceTarget.Cloud + name := d.forwardingRule.Name + + if _, err := cloud.Compute().ForwardingRules().Delete(ctx, cloud.Project(), cloud.Region(), name); err != nil { + return fmt.Errorf("deleting forwardingRule %s: %w", name, err) + } + + return nil +} + +// String returns a string representation of the task +func (d *deleteForwardingRule) String() string { + return d.TaskName() + "-" + d.Item() +} + +func (d *deleteForwardingRule) DeferDeletion() bool { + // We want to defer deletion, in case new nodes are launched with + // the old configuration during the rolling-update operation. + return true +}