diff --git a/provider/pkg/azure/client_azcore.go b/provider/pkg/azure/client_azcore.go index d5b37a7ab9cb..fb9835d1b431 100644 --- a/provider/pkg/azure/client_azcore.go +++ b/provider/pkg/azure/client_azcore.go @@ -202,34 +202,35 @@ func (c *azCoreClient) Delete(ctx context.Context, id, apiVersion, asyncStyle st return err } - resp, err := c.pipeline.Do(req) - if err != nil { - return err - } - - // Some APIs are explicitly marked `x-ms-long-running-operation` and we should only do the - // poll for the deletion result in that case. - if asyncStyle != "" { - pt, err := runtime.NewPoller[any](resp, c.pipeline, nil) + err = func() error { + resp, err := c.pipeline.Do(req) if err != nil { return err } - _, err = pt.PollUntilDone(ctx, &runtime.PollUntilDoneOptions{ - Frequency: time.Duration(c.deletePollingIntervalSeconds * int64(time.Second)), - }) - if err != nil { - respErr := err.(*azcore.ResponseError) - if resp.StatusCode == 202 && respErr.StatusCode == 404 && strings.Contains(respErr.Error(), "ResourceNotFound") { - // Consider this specific error to be a success of deletion. - // Work around https://github.com/pulumi/pulumi-azure-nextgen/issues/120 - return nil - } + if !runtime.HasStatusCode(resp, http.StatusOK, http.StatusAccepted, http.StatusNoContent) { + return runtime.NewResponseError(resp) } - return err - } - if !runtime.HasStatusCode(resp, http.StatusOK, http.StatusAccepted, http.StatusNoContent, http.StatusNotFound) { - return newResponseError(resp) + // Some APIs are explicitly marked `x-ms-long-running-operation` and we should only do the + // poll for the deletion result in that case. + if asyncStyle != "" { + pt, err := runtime.NewPoller[any](resp, c.pipeline, nil) + if err != nil { + return err + } + _, err = pt.PollUntilDone(ctx, &runtime.PollUntilDoneOptions{ + Frequency: time.Duration(c.deletePollingIntervalSeconds * int64(time.Second)), + }) + return err + } + return nil + }() + if err, ok := err.(*azcore.ResponseError); ok { + if err.StatusCode == http.StatusNotFound { + // If the resource is already deleted, we don't want to return an error. + return nil + } + return newResponseError(err.RawResponse) } return err } @@ -297,6 +298,9 @@ func (c *azCoreClient) putOrPatch(ctx context.Context, method string, id string, Frequency: time.Duration(c.updatePollingIntervalSeconds * int64(time.Second)), }) if err != nil { + if err, ok := err.(*azcore.ResponseError); ok { + return nil, created, newResponseError(err.RawResponse) + } return nil, created, err } } diff --git a/provider/pkg/azure/client_azcore_test.go b/provider/pkg/azure/client_azcore_test.go index b378980ff69f..f43d359ecead 100644 --- a/provider/pkg/azure/client_azcore_test.go +++ b/provider/pkg/azure/client_azcore_test.go @@ -418,7 +418,7 @@ func TestErrorStatusCodes(t *testing.T) { { StatusCode: 503, // temporary failure Header: http.Header{"Location": []string{"https://management.azure.com/operation"}}, - Body: io.NopCloser(strings.NewReader(`{"status": "Unavailable"}`)), + Body: io.NopCloser(strings.NewReader(`{"error": {"code": "Unavailable"}}`)), }, { StatusCode: 200, @@ -429,6 +429,17 @@ func TestErrorStatusCodes(t *testing.T) { require.NoError(t, err) }) + t.Run("DELETE initial success on 404 when asyncStyle is given", func(t *testing.T) { + client := newClientWithPreparedResponses([]*http.Response{ + { + StatusCode: 404, + Body: io.NopCloser(strings.NewReader(`{"error": {"code": "ResourceNotFound"}}`)), + }, + }) + err := client.Delete(context.Background(), "/subscriptions/123/rg/rg", "2022-09-01", "the actual value doesn't matter!", nil) + require.NoError(t, err) + }) + t.Run("DELETE polling success on 404 when asyncStyle is given", func(t *testing.T) { client := newClientWithPreparedResponses([]*http.Response{ { @@ -438,13 +449,26 @@ func TestErrorStatusCodes(t *testing.T) { }, { StatusCode: 404, - Body: io.NopCloser(strings.NewReader(`{"error": "ResourceNotFound"}`)), + Body: io.NopCloser(strings.NewReader(`{"error": {"code": "ResourceNotFound"}}`)), }, }) err := client.Delete(context.Background(), "/subscriptions/123/rg/rg", "2022-09-01", "the actual value doesn't matter!", nil) require.NoError(t, err) }) + t.Run("DELETE initial failure when asyncStyle is given", func(t *testing.T) { + client := newClientWithPreparedResponses([]*http.Response{ + { + StatusCode: 400, + Body: io.NopCloser(strings.NewReader(`{"error": {"code": "BadRequest"}}`)), + }, + }) + err := client.Delete(context.Background(), "/subscriptions/123/rg/rg", "2022-09-01", "the actual value doesn't matter!", nil) + require.Error(t, err) + require.IsType(t, &PulumiAzcoreResponseError{}, err) + require.Equal(t, "BadRequest", err.(*PulumiAzcoreResponseError).ErrorCode) + }) + t.Run("DELETE polling failure when asyncStyle is given", func(t *testing.T) { client := newClientWithPreparedResponses([]*http.Response{ { @@ -455,16 +479,18 @@ func TestErrorStatusCodes(t *testing.T) { { StatusCode: 503, // temporary failure Header: http.Header{"Location": []string{"https://management.azure.com/operation"}}, - Body: io.NopCloser(strings.NewReader(`{"status": "Unavailable"}`)), + Body: io.NopCloser(strings.NewReader(`{"error": {"code": "Unavailable"}}`)), }, { StatusCode: 400, Header: http.Header{"Location": []string{"https://management.azure.com/operation"}}, - Body: io.NopCloser(strings.NewReader(`{"status": "Unavailable"}`)), + Body: io.NopCloser(strings.NewReader(`{"error": {"code": "BadRequest"}}`)), }, }) err := client.Delete(context.Background(), "/subscriptions/123/rg/rg", "2022-09-01", "the actual value doesn't matter!", nil) require.Error(t, err) + require.IsType(t, &PulumiAzcoreResponseError{}, err) + require.Equal(t, "BadRequest", err.(*PulumiAzcoreResponseError).ErrorCode) }) }