From 6871aa7148bd53fc0f44f758e832d4ffc1f3306a Mon Sep 17 00:00:00 2001 From: justinsb Date: Fri, 12 Jan 2024 22:00:23 -0500 Subject: [PATCH] Refactor: Plumb context through GCE firewallRule methods Helps with more coherent tracing/logging. --- cloudmock/gce/mockcompute/forwarding_rule.go | 6 +++--- pkg/resources/gce/gce.go | 6 ++++-- upup/pkg/fi/cloudup/gce/compute.go | 18 +++++++++--------- upup/pkg/fi/cloudup/gcetasks/forwardingrule.go | 8 +++++--- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/cloudmock/gce/mockcompute/forwarding_rule.go b/cloudmock/gce/mockcompute/forwarding_rule.go index bed4559993..d0580c1cfb 100644 --- a/cloudmock/gce/mockcompute/forwarding_rule.go +++ b/cloudmock/gce/mockcompute/forwarding_rule.go @@ -53,7 +53,7 @@ func (c *forwardingRuleClient) All() map[string]interface{} { return m } -func (c *forwardingRuleClient) Insert(project, region string, fr *compute.ForwardingRule) (*compute.Operation, error) { +func (c *forwardingRuleClient) Insert(ctx context.Context, project, region string, fr *compute.ForwardingRule) (*compute.Operation, error) { c.Lock() defer c.Unlock() regions, ok := c.forwardingRules[project] @@ -91,7 +91,7 @@ func (c *forwardingRuleClient) SetLabels(ctx context.Context, project, region, n return doneOperation(), nil } -func (c *forwardingRuleClient) Delete(project, region, name string) (*compute.Operation, error) { +func (c *forwardingRuleClient) Delete(ctx context.Context, project, region, name string) (*compute.Operation, error) { c.Lock() defer c.Unlock() regions, ok := c.forwardingRules[project] @@ -109,7 +109,7 @@ func (c *forwardingRuleClient) Delete(project, region, name string) (*compute.Op return doneOperation(), nil } -func (c *forwardingRuleClient) Get(project, region, name string) (*compute.ForwardingRule, error) { +func (c *forwardingRuleClient) Get(ctx context.Context, project, region, name string) (*compute.ForwardingRule, error) { c.Lock() defer c.Unlock() regions, ok := c.forwardingRules[project] diff --git a/pkg/resources/gce/gce.go b/pkg/resources/gce/gce.go index a5570c324d..5d3d5c9296 100644 --- a/pkg/resources/gce/gce.go +++ b/pkg/resources/gce/gce.go @@ -500,6 +500,8 @@ func (d *clusterDiscoveryGCE) listForwardingRules() ([]*resources.Resource, erro } func deleteForwardingRule(cloud fi.Cloud, r *resources.Resource) error { + ctx := context.TODO() + c := cloud.(gce.GCECloud) t := r.Obj.(*compute.ForwardingRule) @@ -509,7 +511,7 @@ func deleteForwardingRule(cloud fi.Cloud, r *resources.Resource) error { return err } - op, err := c.Compute().ForwardingRules().Delete(u.Project, u.Region, u.Name) + op, err := c.Compute().ForwardingRules().Delete(ctx, u.Project, u.Region, u.Name) if err != nil { if gce.IsNotFound(err) { klog.Infof("ForwardingRule not found, assuming deleted: %q", t.SelfLink) @@ -585,7 +587,7 @@ nextFirewallRule: // We lookup the forwarding rule by name, but we then validate that it points to one of our resources forwardingRuleName := strings.TrimPrefix(firewallRule.Name, "k8s-fw-") - forwardingRule, err := c.Compute().ForwardingRules().Get(c.Project(), c.Region(), forwardingRuleName) + forwardingRule, err := c.Compute().ForwardingRules().Get(ctx, c.Project(), c.Region(), forwardingRuleName) if err != nil { if gce.IsNotFound(err) { // We looked it up by name, so an error isn't unlikely diff --git a/upup/pkg/fi/cloudup/gce/compute.go b/upup/pkg/fi/cloudup/gce/compute.go index c5f6a5414b..bde7c2d757 100644 --- a/upup/pkg/fi/cloudup/gce/compute.go +++ b/upup/pkg/fi/cloudup/gce/compute.go @@ -362,9 +362,9 @@ func (c *routeClientImpl) List(ctx context.Context, project string) ([]*compute. } type ForwardingRuleClient interface { - Insert(project, region string, fr *compute.ForwardingRule) (*compute.Operation, error) - Delete(project, region, name string) (*compute.Operation, error) - Get(project, region, name string) (*compute.ForwardingRule, error) + Insert(ctx context.Context, project, region string, fr *compute.ForwardingRule) (*compute.Operation, error) + Delete(ctx context.Context, project, region, name string) (*compute.Operation, error) + Get(ctx context.Context, project, region, name string) (*compute.ForwardingRule, error) List(ctx context.Context, project, region string) ([]*compute.ForwardingRule, error) SetLabels(ctx context.Context, project, region, resource string, request *compute.RegionSetLabelsRequest) (*compute.Operation, error) } @@ -375,16 +375,16 @@ type forwardingRuleClientImpl struct { var _ ForwardingRuleClient = &forwardingRuleClientImpl{} -func (c *forwardingRuleClientImpl) Insert(project, region string, fr *compute.ForwardingRule) (*compute.Operation, error) { - return c.srv.Insert(project, region, fr).Do() +func (c *forwardingRuleClientImpl) Insert(ctx context.Context, project, region string, fr *compute.ForwardingRule) (*compute.Operation, error) { + return c.srv.Insert(project, region, fr).Context(ctx).Do() } -func (c *forwardingRuleClientImpl) Delete(project, region, name string) (*compute.Operation, error) { - return c.srv.Delete(project, region, name).Do() +func (c *forwardingRuleClientImpl) Delete(ctx context.Context, project, region, name string) (*compute.Operation, error) { + return c.srv.Delete(project, region, name).Context(ctx).Do() } -func (c *forwardingRuleClientImpl) Get(project, region, name string) (*compute.ForwardingRule, error) { - return c.srv.Get(project, region, name).Do() +func (c *forwardingRuleClientImpl) Get(ctx context.Context, project, region, name string) (*compute.ForwardingRule, error) { + return c.srv.Get(project, region, name).Context(ctx).Do() } func (c *forwardingRuleClientImpl) SetLabels(ctx context.Context, project string, region string, resource string, request *compute.RegionSetLabelsRequest) (*compute.Operation, error) { diff --git a/upup/pkg/fi/cloudup/gcetasks/forwardingrule.go b/upup/pkg/fi/cloudup/gcetasks/forwardingrule.go index fc3a549ec5..dbc79b44c2 100644 --- a/upup/pkg/fi/cloudup/gcetasks/forwardingrule.go +++ b/upup/pkg/fi/cloudup/gcetasks/forwardingrule.go @@ -65,10 +65,12 @@ func (e *ForwardingRule) CompareWithID() *string { } func (e *ForwardingRule) Find(c *fi.CloudupContext) (*ForwardingRule, error) { + ctx := c.Context() + cloud := c.T.Cloud.(gce.GCECloud) name := fi.ValueOf(e.Name) - r, err := cloud.Compute().ForwardingRules().Get(cloud.Project(), cloud.Region(), name) + r, err := cloud.Compute().ForwardingRules().Get(ctx, cloud.Project(), cloud.Region(), name) if err != nil { if gce.IsNotFound(err) { return nil, nil @@ -212,7 +214,7 @@ func (_ *ForwardingRule) RenderGCE(t *gce.GCEAPITarget, a, e, changes *Forwardin if a == nil { klog.V(4).Infof("Creating ForwardingRule %q", o.Name) - op, err := t.Cloud.Compute().ForwardingRules().Insert(t.Cloud.Project(), t.Cloud.Region(), o) + op, err := t.Cloud.Compute().ForwardingRules().Insert(ctx, t.Cloud.Project(), t.Cloud.Region(), o) if err != nil { return fmt.Errorf("error creating ForwardingRule %q: %v", o.Name, err) } @@ -223,7 +225,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 - r, err := t.Cloud.Compute().ForwardingRules().Get(t.Cloud.Project(), t.Cloud.Region(), name) + 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) }