Merge pull request #58566 from bowei/cp-op-fix

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

GCE: invalid location was used in regional and zonal operations

Location was set to the full URL instead of the short name. Adds
logging to the operations to make future issues easier to debug.

Big thanks to nick for root causing this bug! I merged your patch in with the logging changes and chose to pass key instead of manipulating the compute API struct directly.

/assign @nicksardo 

```release-note
NONE
```
This commit is contained in:
Kubernetes Submit Queue 2018-01-20 02:25:10 -08:00 committed by GitHub
commit db8335f4a9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 64 additions and 25 deletions

View File

@ -18,6 +18,9 @@ package cloud
import (
"context"
"fmt"
"github.com/golang/glog"
alpha "google.golang.org/api/compute/v0.alpha"
beta "google.golang.org/api/compute/v0.beta"
@ -40,6 +43,11 @@ type gaOperation struct {
s *Service
op *ga.Operation
projectID string
key *meta.Key
}
func (o *gaOperation) String() string {
return fmt.Sprintf("gaOperation{%q, %v}", o.projectID, o.key)
}
func (o *gaOperation) isDone(ctx context.Context) (bool, error) {
@ -48,13 +56,18 @@ func (o *gaOperation) isDone(ctx context.Context) (bool, error) {
err error
)
switch {
case o.op.Region != "":
op, err = o.s.GA.RegionOperations.Get(o.projectID, o.op.Region, o.op.Name).Context(ctx).Do()
case o.op.Zone != "":
op, err = o.s.GA.ZoneOperations.Get(o.projectID, o.op.Zone, o.op.Name).Context(ctx).Do()
default:
switch o.key.Type() {
case meta.Regional:
op, err = o.s.GA.RegionOperations.Get(o.projectID, o.key.Region, o.op.Name).Context(ctx).Do()
glog.V(5).Infof("GA.RegionOperations.Get(%v, %v, %v) = %+v, %v; ctx = %v", o.projectID, o.key.Region, o.op.Name, op, err, ctx)
case meta.Zonal:
op, err = o.s.GA.ZoneOperations.Get(o.projectID, o.key.Zone, o.op.Name).Context(ctx).Do()
glog.V(5).Infof("GA.ZoneOperations.Get(%v, %v, %v) = %+v, %v; ctx = %v", o.projectID, o.key.Zone, o.op.Name, op, err, ctx)
case meta.Global:
op, err = o.s.GA.GlobalOperations.Get(o.projectID, o.op.Name).Context(ctx).Do()
glog.V(5).Infof("GA.GlobalOperations.Get(%v, %v) = %+v, %v; ctx = %v", o.projectID, o.op.Name, op, err, ctx)
default:
return false, fmt.Errorf("invalid key type: %#v", o.key)
}
if err != nil {
return false, err
@ -75,6 +88,11 @@ type alphaOperation struct {
s *Service
op *alpha.Operation
projectID string
key *meta.Key
}
func (o *alphaOperation) String() string {
return fmt.Sprintf("alphaOperation{%q, %v}", o.projectID, o.key)
}
func (o *alphaOperation) isDone(ctx context.Context) (bool, error) {
@ -83,13 +101,18 @@ func (o *alphaOperation) isDone(ctx context.Context) (bool, error) {
err error
)
switch {
case o.op.Region != "":
op, err = o.s.Alpha.RegionOperations.Get(o.projectID, o.op.Region, o.op.Name).Context(ctx).Do()
case o.op.Zone != "":
op, err = o.s.Alpha.ZoneOperations.Get(o.projectID, o.op.Zone, o.op.Name).Context(ctx).Do()
default:
switch o.key.Type() {
case meta.Regional:
op, err = o.s.Alpha.RegionOperations.Get(o.projectID, o.key.Region, o.op.Name).Context(ctx).Do()
glog.V(5).Infof("Alpha.RegionOperations.Get(%v, %v, %v) = %+v, %v; ctx = %v", o.projectID, o.key.Region, o.op.Name, op, err, ctx)
case meta.Zonal:
op, err = o.s.Alpha.ZoneOperations.Get(o.projectID, o.key.Zone, o.op.Name).Context(ctx).Do()
glog.V(5).Infof("Alpha.ZoneOperations.Get(%v, %v, %v) = %+v, %v; ctx = %v", o.projectID, o.key.Zone, o.op.Name, op, err, ctx)
case meta.Global:
op, err = o.s.Alpha.GlobalOperations.Get(o.projectID, o.op.Name).Context(ctx).Do()
glog.V(5).Infof("Alpha.GlobalOperations.Get(%v, %v) = %+v, %v; ctx = %v", o.projectID, o.op.Name, op, err, ctx)
default:
return false, fmt.Errorf("invalid key type: %#v", o.key)
}
if err != nil {
return false, err
@ -110,6 +133,11 @@ type betaOperation struct {
s *Service
op *beta.Operation
projectID string
key *meta.Key
}
func (o *betaOperation) String() string {
return fmt.Sprintf("betaOperation{%q, %v}", o.projectID, o.key)
}
func (o *betaOperation) isDone(ctx context.Context) (bool, error) {
@ -118,13 +146,18 @@ func (o *betaOperation) isDone(ctx context.Context) (bool, error) {
err error
)
switch {
case o.op.Region != "":
op, err = o.s.Beta.RegionOperations.Get(o.projectID, o.op.Region, o.op.Name).Context(ctx).Do()
case o.op.Zone != "":
op, err = o.s.Beta.ZoneOperations.Get(o.projectID, o.op.Zone, o.op.Name).Context(ctx).Do()
default:
switch o.key.Type() {
case meta.Regional:
op, err = o.s.Beta.RegionOperations.Get(o.projectID, o.key.Region, o.op.Name).Context(ctx).Do()
glog.V(5).Infof("Beta.RegionOperations.Get(%v, %v, %v) = %+v, %v; ctx = %v", o.projectID, o.key.Region, o.op.Name, op, err, ctx)
case meta.Zonal:
op, err = o.s.Beta.ZoneOperations.Get(o.projectID, o.key.Zone, o.op.Name).Context(ctx).Do()
glog.V(5).Infof("Beta.ZoneOperations.Get(%v, %v, %v) = %+v, %v; ctx = %v", o.projectID, o.key.Zone, o.op.Name, op, err, ctx)
case meta.Global:
op, err = o.s.Beta.GlobalOperations.Get(o.projectID, o.op.Name).Context(ctx).Do()
glog.V(5).Infof("Beta.GlobalOperations.Get(%v, %v) = %+v, %v; ctx = %v", o.projectID, o.op.Name, op, err, ctx)
default:
return false, fmt.Errorf("invalid key type: %#v", o.key)
}
if err != nil {
return false, err

View File

@ -20,6 +20,8 @@ import (
"context"
"fmt"
"github.com/golang/glog"
alpha "google.golang.org/api/compute/v0.alpha"
beta "google.golang.org/api/compute/v0.beta"
ga "google.golang.org/api/compute/v1"
@ -36,26 +38,26 @@ type Service struct {
}
// wrapOperation wraps a GCE anyOP in a version generic operation type.
func (g *Service) wrapOperation(anyOp interface{}) (operation, error) {
func (s *Service) wrapOperation(anyOp interface{}) (operation, error) {
switch o := anyOp.(type) {
case *ga.Operation:
r, err := ParseResourceURL(o.SelfLink)
if err != nil {
return nil, err
}
return &gaOperation{g, o, r.ProjectID}, nil
return &gaOperation{s, o, r.ProjectID, r.Key}, nil
case *alpha.Operation:
r, err := ParseResourceURL(o.SelfLink)
if err != nil {
return nil, err
}
return &alphaOperation{g, o, r.ProjectID}, nil
return &alphaOperation{s, o, r.ProjectID, r.Key}, nil
case *beta.Operation:
r, err := ParseResourceURL(o.SelfLink)
if err != nil {
return nil, err
}
return &betaOperation{g, o, r.ProjectID}, nil
return &betaOperation{s, o, r.ProjectID, r.Key}, nil
default:
return nil, fmt.Errorf("invalid type %T", anyOp)
}
@ -64,16 +66,20 @@ func (g *Service) wrapOperation(anyOp interface{}) (operation, error) {
// WaitForCompletion of a long running operation. This will poll the state of
// GCE for the completion status of the given operation. genericOp can be one
// of alpha, beta, ga Operation types.
func (g *Service) WaitForCompletion(ctx context.Context, genericOp interface{}) error {
op, err := g.wrapOperation(genericOp)
func (s *Service) WaitForCompletion(ctx context.Context, genericOp interface{}) error {
op, err := s.wrapOperation(genericOp)
if err != nil {
glog.Errorf("wrapOperation(%+v) error: %v", genericOp, err)
return err
}
for done, err := op.isDone(ctx); !done; done, err = op.isDone(ctx) {
if err != nil {
glog.V(4).Infof("op.isDone(%v) error; op = %v, err = %v", ctx, op, err)
return err
}
g.RateLimiter.Accept(ctx, op.rateLimitKey())
glog.V(5).Infof("op.isDone(%v) waiting; op = %v", ctx, op)
s.RateLimiter.Accept(ctx, op.rateLimitKey())
}
glog.V(5).Infof("op.isDone(%v) complete; op = %v", ctx, op)
return nil
}