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

@@ -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

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
+}