juju · jujubot · Jun 22, 2019 · Jun 18, 2019 · Jun 19, 2019 · Jun 20, 2019
diff --git a/provider/ec2/environ.go b/provider/ec2/environ.go
@@ -583,7 +583,7 @@ func (e *environ) StartInstance(ctx context.ProviderCallContext, args environs.S
 	}
 
 	switch {
-	case subnetErr != nil && errors.IsNotFound(subnetErr):
+	case isNotFoundError(subnetErr):
 		return nil, errors.Trace(subnetErr)
 	case subnetErr != nil:
 		return nil, errors.Annotatef(maybeConvertCredentialError(subnetErr, ctx), "getting subnets for zone %q", availabilityZone)

diff --git a/provider/gce/google/conn_instance.go b/provider/gce/google/conn_instance.go
@@ -90,16 +90,19 @@ func (gce *Connection) Instances(prefix string, statuses ...string) ([]Instance,
 func (gce *Connection) removeInstance(id, zone string) error {
 	err := gce.raw.RemoveInstance(gce.projectID, zone, id)
 	if err != nil {
+		if IsNotFound(err) {
+			return nil
+		}
 		// TODO(ericsnow) Try removing the firewall anyway?
 		return errors.Trace(err)
 	}
 
 	fwname := id
 	err = gce.raw.RemoveFirewall(gce.projectID, fwname)
-	if errors.IsNotFound(err) {
-		return nil
-	}
 	if err != nil {
+		if IsNotFound(err) {
+			return nil
+		}
 		return errors.Trace(err)
 	}
 	return nil

diff --git a/provider/gce/google/conn_instance_test.go b/provider/gce/google/conn_instance_test.go
@@ -118,12 +118,13 @@ func (s *connSuite) TestConnectionAddInstanceFailed(c *gc.C) {
 func (s *connSuite) TestConnectionAddInstanceWaitFailed(c *gc.C) {
 	s.FakeConn.Instance = &s.RawInstanceFull
 
-	failure := s.NewWaitError(nil, errors.New("unknown"))
+	cause := errors.New("unknown")
+	failure := s.NewWaitError(nil, cause)
 	s.FakeConn.Err = failure
 
 	err := google.ConnAddInstance(s.Conn, &s.RawInstance, "mtype", "a-zone")
 
-	c.Check(errors.Cause(err), gc.Equals, failure)
+	c.Check(errors.Cause(err), gc.Equals, cause)
 }
 
 func (s *connSuite) TestConnectionAddInstanceGetFailed(c *gc.C) {

diff --git a/provider/gce/google/conn_network.go b/provider/gce/google/conn_network.go
@@ -21,7 +21,7 @@ import (
 // returned.
 func (gce Connection) firewallRules(fwname string) (ruleSet, error) {
 	firewalls, err := gce.raw.GetFirewalls(gce.projectID, fwname)
-	if errors.IsNotFound(err) {
+	if IsNotFound(err) {
 		return make(ruleSet), nil
 	}
 	if err != nil {

diff --git a/provider/gce/google/errors.go b/provider/gce/google/errors.go
@@ -10,6 +10,7 @@ import (
 	"strings"
 
 	"github.com/juju/errors"
+	"google.golang.org/api/googleapi"
 
 	"github.com/juju/juju/environs/context"
 )
@@ -90,20 +91,24 @@ func HasDenialStatusCode(err error) bool {
 		return false
 	}
 
-	// http/url.Error is constructed with status code in mind and, at the time of writing for go-1.10,
-	// contains response status code and description in error.Error.
-	// We have to examine the error message to determine whether the error is related to authentication failure.
-	if cause, ok := errors.Cause(err).(*url.Error); ok {
-		for code, descs := range AuthorisationFailureStatusCodes {
-			for _, desc := range descs {
-				if strings.Contains(cause.Error(), fmt.Sprintf(": %v %v", code, desc)) {
-					return true
-				}
+	var cause error
+	switch e := errors.Cause(err).(type) {
+	case *url.Error:
+		cause = e
+	case *googleapi.Error:
+		cause = e
+	default:
+		return false
+	}
+
+	for code, descs := range AuthorisationFailureStatusCodes {
+		for _, desc := range descs {
+			if strings.Contains(cause.Error(), fmt.Sprintf(": %v %v", code, desc)) {
+				return true
 			}
 		}
 	}
 	return false
-
 }
 
 // AuthorisationFailureStatusCodes contains http status code and
@@ -120,3 +125,14 @@ var AuthorisationFailureStatusCodes = map[int][]string{
 	// https://tools.ietf.org/html/rfc6749#section-5.2
 	http.StatusBadRequest: {"Bad Request"},
 }
+
+// IsNotFound reports if given error is of 'not found' type.
+func IsNotFound(err error) bool {
+	if err == nil {
+		return false
+	}
+	if gerr, ok := errors.Cause(err).(*googleapi.Error); ok {
+		return gerr.Code == http.StatusNotFound
+	}
+	return errors.IsNotFound(errors.Cause(err))
+}
diff --git a/provider/gce/google/raw.go b/provider/gce/google/raw.go
@@ -100,8 +100,7 @@ func (rc *rawConn) AddInstance(projectID, zoneName string, spec *compute.Instanc
 		// We are guaranteed the insert failed at the point.
 		return errors.Annotate(err, "sending new instance request")
 	}
-
-	err = rc.waitOperation(projectID, operation, attemptsLong)
+	err = rc.waitOperation(projectID, operation, attemptsLong, logOperationErrors)
 	return errors.Trace(err)
 }
 
@@ -111,8 +110,7 @@ func (rc *rawConn) RemoveInstance(projectID, zone, id string) error {
 	if err != nil {
 		return errors.Trace(err)
 	}
-
-	err = rc.waitOperation(projectID, operation, attemptsLong)
+	err = rc.waitOperation(projectID, operation, attemptsLong, returnNotFoundOperationErrors)
 	return errors.Trace(err)
 }
 
@@ -145,8 +143,7 @@ func (rc *rawConn) AddFirewall(projectID string, firewall *compute.Firewall) err
 	if err != nil {
 		return errors.Trace(err)
 	}
-
-	err = rc.waitOperation(projectID, operation, attemptsLong)
+	err = rc.waitOperation(projectID, operation, attemptsLong, logOperationErrors)
 	return errors.Trace(err)
 }
 
@@ -156,19 +153,45 @@ func (rc *rawConn) UpdateFirewall(projectID, name string, firewall *compute.Fire
 	if err != nil {
 		return errors.Trace(err)
 	}
-
-	err = rc.waitOperation(projectID, operation, attemptsLong)
+	err = rc.waitOperation(projectID, operation, attemptsLong, logOperationErrors)
 	return errors.Trace(err)
 }
 
+type handleOperationErrors func(operation *compute.Operation) error
+
+func returnNotFoundOperationErrors(operation *compute.Operation) error {
+	if operation.Error != nil {
+		result := waitError{operation, nil}
+		for _, err := range operation.Error.Errors {
+			if err.Code == "RESOURCE_NOT_FOUND" {
+				result.cause = errors.NotFoundf("resource", err.Message)
+				continue
+			}
+			logger.Errorf("GCE operation error: (%s) %s", err.Code, err.Message)
+		}
+		return result
+	}
+	return nil
+}
+
+func logOperationErrors(operation *compute.Operation) error {
+	if operation.Error != nil {
+		for _, err := range operation.Error.Errors {
+			logger.Errorf("GCE operation error: (%s) %s", err.Code, err.Message)
+		}
+		return waitError{operation, nil}
+	}
+	return nil
+}
+
 func (rc *rawConn) RemoveFirewall(projectID, name string) error {
 	call := rc.Firewalls.Delete(projectID, name)
 	operation, err := call.Do()
 	if err != nil {
 		return errors.Trace(convertRawAPIError(err))
 	}
 
-	err = rc.waitOperation(projectID, operation, attemptsLong)
+	err = rc.waitOperation(projectID, operation, attemptsLong, returnNotFoundOperationErrors)
 	return errors.Trace(convertRawAPIError(err))
 }
 
@@ -216,7 +239,7 @@ func (rc *rawConn) CreateDisk(project, zone string, spec *compute.Disk) error {
 	if err != nil {
 		return errors.Annotate(err, "could not create a new disk")
 	}
-	return errors.Trace(rc.waitOperation(project, op, attemptsLong))
+	return errors.Trace(rc.waitOperation(project, op, attemptsLong, logOperationErrors))
 }
 
 func (rc *rawConn) ListDisks(project string) ([]*compute.Disk, error) {
@@ -246,7 +269,7 @@ func (rc *rawConn) RemoveDisk(project, zone, id string) error {
 	if err != nil {
 		return errors.Annotatef(err, "could not delete disk %q", id)
 	}
-	return errors.Trace(rc.waitOperation(project, op, attemptsLong))
+	return errors.Trace(rc.waitOperation(project, op, attemptsLong, returnNotFoundOperationErrors))
 }
 
 func (rc *rawConn) GetDisk(project, zone, id string) (*compute.Disk, error) {
@@ -307,6 +330,13 @@ func (err waitError) Error() string {
 	return fmt.Sprintf("GCE operation %q failed", err.op.Name)
 }
 
+func (err waitError) Cause() error {
+	if err.cause != nil {
+		return err.cause
+	}
+	return err
+}
+
 func isWaitError(err error) bool {
 	_, ok := err.(*waitError)
 	return ok
@@ -347,7 +377,7 @@ var doOpCall = func(call opDoer) (*compute.Operation, error) {
 // attempts) and may time out.
 //
 // TODO(katco): 2016-08-09: lp:1611427
-func (rc *rawConn) waitOperation(projectID string, op *compute.Operation, attempts utils.AttemptStrategy) error {
+func (rc *rawConn) waitOperation(projectID string, op *compute.Operation, attempts utils.AttemptStrategy, f handleOperationErrors) error {
 	// TODO(perrito666) 2016-05-02 lp:1558657
 	started := time.Now()
 	logger.Infof("GCE operation %q, waiting...", op.Name)
@@ -367,11 +397,8 @@ func (rc *rawConn) waitOperation(projectID string, op *compute.Operation, attemp
 		err := errors.Errorf("timed out after %d seconds", time.Now().Sub(started)/time.Second)
 		return waitError{op, err}
 	}
-	if op.Error != nil {
-		for _, err := range op.Error.Errors {
-			logger.Errorf("GCE operation error: (%s) %s", err.Code, err.Message)
-		}
-		return waitError{op, nil}
+	if err := f(op); err != nil {
+		return err
 	}
 
 	logger.Infof("GCE operation %q finished", op.Name)
@@ -394,7 +421,7 @@ func (rc *rawConn) SetMetadata(projectID, zone, instanceID string, metadata *com
 	if err != nil {
 		return errors.Trace(err)
 	}
-	err = rc.waitOperation(projectID, op, attemptsLong)
+	err = rc.waitOperation(projectID, op, attemptsLong, logOperationErrors)
 	return errors.Trace(err)
 }
 

diff --git a/provider/gce/google/raw_test.go b/provider/gce/google/raw_test.go
@@ -19,8 +19,9 @@ type rawConnSuite struct {
 	// TODO(katco): 2016-08-09: lp:1611427
 	strategy utils.AttemptStrategy
 
-	callCount int
-	opCallErr error
+	handleOperationErrorsF handleOperationErrors
+	callCount              int
+	opCallErr              error
 }
 
 var _ = gc.Suite(&rawConnSuite{})
@@ -45,6 +46,7 @@ func (s *rawConnSuite) SetUpTest(c *gc.C) {
 		s.callCount++
 		return s.op, s.opCallErr
 	})
+	s.handleOperationErrorsF = logOperationErrors
 }
 
 func (s *rawConnSuite) TestConnectionCheckOperationError(c *gc.C) {
@@ -88,7 +90,7 @@ func (s *rawConnSuite) TestConnectionCheckOperationGlobal(c *gc.C) {
 
 func (s *rawConnSuite) TestConnectionWaitOperation(c *gc.C) {
 	original := &compute.Operation{}
-	err := s.rawConn.waitOperation("proj", original, s.strategy)
+	err := s.rawConn.waitOperation("proj", original, s.strategy, s.handleOperationErrorsF)
 
 	c.Check(err, jc.ErrorIsNil)
 	c.Check(s.callCount, gc.Equals, 1)
@@ -98,7 +100,7 @@ func (s *rawConnSuite) TestConnectionWaitOperationAlreadyDone(c *gc.C) {
 	original := &compute.Operation{
 		Status: StatusDone,
 	}
-	err := s.rawConn.waitOperation("proj", original, s.strategy)
+	err := s.rawConn.waitOperation("proj", original, s.strategy, s.handleOperationErrorsF)
 
 	c.Check(err, jc.ErrorIsNil)
 	c.Check(s.callCount, gc.Equals, 0)
@@ -115,15 +117,15 @@ func (s *rawConnSuite) TestConnectionWaitOperationWaiting(c *gc.C) {
 	})
 
 	original := &compute.Operation{}
-	err := s.rawConn.waitOperation("proj", original, s.strategy)
+	err := s.rawConn.waitOperation("proj", original, s.strategy, s.handleOperationErrorsF)
 
 	c.Check(err, jc.ErrorIsNil)
 	c.Check(s.callCount, gc.Equals, 2)
 }
 
 func (s *rawConnSuite) TestConnectionWaitOperationTimeout(c *gc.C) {
 	s.op.Status = StatusRunning
-	err := s.rawConn.waitOperation("proj", s.op, s.strategy)
+	err := s.rawConn.waitOperation("proj", s.op, s.strategy, s.handleOperationErrorsF)
 
 	c.Check(err, gc.ErrorMatches, ".* timed out .*")
 	c.Check(s.callCount, gc.Equals, 4)
@@ -133,7 +135,7 @@ func (s *rawConnSuite) TestConnectionWaitOperationFailure(c *gc.C) {
 	s.opCallErr = errors.New("<unknown>")
 
 	original := &compute.Operation{}
-	err := s.rawConn.waitOperation("proj", original, s.strategy)
+	err := s.rawConn.waitOperation("proj", original, s.strategy, s.handleOperationErrorsF)
 
 	c.Check(err, gc.ErrorMatches, ".*<unknown>")
 	c.Check(s.callCount, gc.Equals, 1)
@@ -144,7 +146,7 @@ func (s *rawConnSuite) TestConnectionWaitOperationError(c *gc.C) {
 	s.op.Name = "testing-wait-operation-error"
 
 	original := &compute.Operation{}
-	err := s.rawConn.waitOperation("proj", original, s.strategy)
+	err := s.rawConn.waitOperation("proj", original, s.strategy, s.handleOperationErrorsF)
 
 	c.Check(err, gc.ErrorMatches, `.* "testing-wait-operation-error" .*`)
 	c.Check(s.callCount, gc.Equals, 1)

diff --git a/provider/oci/storage_volumes.go b/provider/oci/storage_volumes.go
@@ -428,8 +428,8 @@ func (v *volumeSource) attachVolume(ctx envcontext.ProviderCallContext, param st
 			req := ociCore.DetachVolumeRequest{
 				VolumeAttachmentId: volAttach.GetId(),
 			}
-			_, nestedErr := v.computeAPI.DetachVolume(context.Background(), req)
-			if nestedErr != nil {
+			res, nestedErr := v.computeAPI.DetachVolume(context.Background(), req)
+			if nestedErr != nil && !v.env.isNotFound(res.RawResponse) {
 				logger.Warningf("failed to cleanup volume attachment: %v", volAttach.GetId())
 				return
 			}
@@ -625,8 +625,8 @@ func (v *volumeSource) DetachVolumes(ctx envcontext.ProviderCallContext, params
 						VolumeAttachmentId: attachment.Id,
 					}
 
-					_, err := v.computeAPI.DetachVolume(context.Background(), request)
-					if err != nil {
+					res, err := v.computeAPI.DetachVolume(context.Background(), request)
+					if err != nil && !v.env.isNotFound(res.RawResponse) {
 						if isAuthFailure(err, ctx) {
 							credErr = err
 							common.HandleCredentialError(err, ctx)