Skip to content

Commit d60f910

Browse files
jackofallopsjackofallops
authored
sdk - rework LRO and ProvisioningState pollers to match ARM spec for terminal statuses (#1221)
* remove brittle custom statuses and rely on API defined terminal statuses * linting fixes * update provisioningstatepoller for terminal state processing
1 parent 7599a56 commit d60f910

14 files changed

+133
-245
lines changed

‎sdk/client/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,7 @@ func (c *Client) retryableClient(ctx context.Context, checkRetry retryablehttp.C
720720
// In case the context has deadline defined, adjust the retry count to a value
721721
// that the total time spent for retrying is right before the deadline exceeded.
722722
if deadline, ok := ctx.Deadline(); ok {
723-
r.RetryMax = safeRetryNumber(deadline.Sub(time.Now()))
723+
r.RetryMax = safeRetryNumber(time.Until(deadline))
724724
}
725725

726726
tlsConfig := tls.Config{

‎sdk/client/msgraph/client.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ type Client struct {
3232
apiVersion ApiVersion
3333

3434
// tenantId is the tenant ID to use in requests
35-
tenantId string
35+
tenantId string // nolint: unused
3636
}
3737

3838
func NewClient(api environments.Api, serviceName string, apiVersion ApiVersion) (*Client, error) {
@@ -96,7 +96,7 @@ func (c *Client) NewRequest(ctx context.Context, input client.RequestOptions) (*
9696
}
9797

9898
req.URL.RawQuery = query.Encode()
99-
//req.RetryFunc = client.RequestRetryAny(defaultRetryFunctions...)
99+
// req.RetryFunc = client.RequestRetryAny(defaultRetryFunctions...)
100100
req.ValidStatusCodes = input.ExpectedStatusCodes
101101

102102
return req, nil

‎sdk/client/pollers/interface.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func (e PollingCancelledError) Error() string {
7373
return fmt.Sprintf("polling was cancelled: %+v", e.Message)
7474
}
7575

76-
return fmt.Sprintf("polling was cancelled")
76+
return "polling was cancelled"
7777
}
7878

7979
var _ error = PollingDroppedConnectionError{}
@@ -89,7 +89,7 @@ func (e PollingDroppedConnectionError) Error() string {
8989
return fmt.Sprintf("experienced a dropped connection when polling: %+v", e.Message)
9090
}
9191

92-
return fmt.Sprintf("experienced a dropped connection when polling")
92+
return "experienced a dropped connection when polling"
9393
}
9494

9595
var _ error = PollingFailedError{}
@@ -108,5 +108,5 @@ func (e PollingFailedError) Error() string {
108108
return fmt.Sprintf("polling failed: %+v", e.Message)
109109
}
110110

111-
return fmt.Sprintf("polling failed")
111+
return "polling failed"
112112
}

‎sdk/client/pollers/poller.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,8 @@ func (p *Poller) PollUntilDone(ctx context.Context) error {
114114
retryDuration = p.latestResponse.PollInterval
115115
}
116116
endTime := time.Now().Add(retryDuration)
117-
select {
117+
118+
select { // nolint: gosimple
118119
case <-time.After(time.Until(endTime)):
119120
{
120121
break
@@ -160,24 +161,20 @@ func (p *Poller) PollUntilDone(ctx context.Context) error {
160161
case PollingStatusCancelled:
161162
p.latestError = fmt.Errorf("internal-error: a polling status of `Cancelled` should be surfaced as a PollingCancelledError")
162163
done = true
163-
break
164164

165165
case PollingStatusFailed:
166166
p.latestError = fmt.Errorf("internal-error: a polling status of `Failed` should be surfaced as a PollingFailedError")
167167
done = true
168-
break
169168

170169
case PollingStatusInProgress:
171170
continue
172171

173172
case PollingStatusSucceeded:
174173
done = true
175-
break
176174

177175
default:
178176
p.latestError = fmt.Errorf("internal-error: unimplemented polling status %q", string(response.Status))
179177
done = true
180-
break
181178
}
182179

183180
if done {

‎sdk/client/resourcemanager/errors.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ API Response:
4343

4444
// parseErrorFromApiResponse parses the error from the API Response
4545
// into an Error type, which allows for better surfacing of errors
46-
func parseErrorFromApiResponse(response http.Response) (*Error, error) {
46+
func parseErrorFromApiResponse(response *http.Response) (*Error, error) {
4747
respBody, err := io.ReadAll(response.Body)
4848
if err != nil {
4949
return nil, fmt.Errorf("parsing response body: %+v", err)
@@ -125,7 +125,6 @@ func parseErrorFromApiResponse(response http.Response) (*Error, error) {
125125
if v.ActivityId != "" {
126126
activityId = v.ActivityId
127127
}
128-
break
129128
}
130129
return &Error{
131130
ActivityId: activityId,

‎sdk/client/resourcemanager/errors_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func TestParseErrorFromApiResponse_LongRunningOperation(t *testing.T) {
2424
Message: "The specified name is already in use.",
2525
Status: "Failed",
2626
}
27-
actual, err := parseErrorFromApiResponse(input)
27+
actual, err := parseErrorFromApiResponse(&input)
2828
if err != nil {
2929
t.Fatalf("parsing error from api response: %+v", err)
3030
}
@@ -49,7 +49,7 @@ func TestParseErrorFromApiResponse_ResourceManagerType1(t *testing.T) {
4949
Message: "Cannot update the site 'func-tfbug-b78d100425' because it uses AlwaysOn feature which is not allowed in the target compute mode.",
5050
Status: "Unknown",
5151
}
52-
actual, err := parseErrorFromApiResponse(input)
52+
actual, err := parseErrorFromApiResponse(&input)
5353
if err != nil {
5454
t.Fatalf("parsing error from api response: %+v", err)
5555
}
@@ -74,7 +74,7 @@ func TestParseErrorFromApiResponse_ResourceManagerType2(t *testing.T) {
7474
Message: "Failed to start operation. Verify input and try operation again.\nFailed to start operation. Verify input and try operation again.\nPossible Causes: \"Invalid parameters were specified.\"\nRecommended Action: \"Verify the input and try again.\"",
7575
Status: "BadRequest",
7676
}
77-
actual, err := parseErrorFromApiResponse(input)
77+
actual, err := parseErrorFromApiResponse(&input)
7878
if err != nil {
7979
t.Fatalf("parsing error from api response: %+v", err)
8080
}
@@ -98,7 +98,7 @@ func TestParseErrorFromApiResponse_ResourceManagerType2Alt(t *testing.T) {
9898
Message: "The request is not valid.\nResource name is not valid. It must be between 3 and 26 characters and can only include alphanumeric characters and hyphens.",
9999
Status: "ValidationError",
100100
}
101-
actual, err := parseErrorFromApiResponse(input)
101+
actual, err := parseErrorFromApiResponse(&input)
102102
if err != nil {
103103
t.Fatalf("parsing error from api response: %+v", err)
104104
}
@@ -123,7 +123,7 @@ func TestParseErrorFromApiResponse_ResourceManagerType3(t *testing.T) {
123123
Message: "Outbound Trust/Untrust certificate can not be deleted from rule stack SUBSCRIPTION~XXXXXXXX-XXXX-XXXX-XXXXXXXXXXXX~RG~acctestRG-PAN-230725055238618023~STACK~testAcc-palrs-230725055238618023 as associated rule list(s) already has SSL outbound inspection set with status code BadRequest",
124124
Status: "Failed",
125125
}
126-
actual, err := parseErrorFromApiResponse(input)
126+
actual, err := parseErrorFromApiResponse(&input)
127127
if err != nil {
128128
t.Fatalf("parsing error from api response: %+v", err)
129129
}
@@ -147,7 +147,7 @@ func TestParseErrorFromApiResponse_EmptyResponseShouldOutputHttpResponseAnyway(t
147147
Message: "Couldn't parse Azure API Response into a friendly error - please see the original HTTP Response for more details (and file a bug so we can fix this!).",
148148
Status: "Unknown",
149149
}
150-
actual, err := parseErrorFromApiResponse(input)
150+
actual, err := parseErrorFromApiResponse(&input)
151151
if err != nil {
152152
t.Fatalf("parsing error from api response: %+v", err)
153153
}
@@ -172,7 +172,7 @@ func TestParseErrorFromApiResponse_UnexpectedHtmlShouldOutputHttpResponseAnyway(
172172
Message: "Couldn't parse Azure API Response into a friendly error - please see the original HTTP Response for more details (and file a bug so we can fix this!).",
173173
Status: "Unknown",
174174
}
175-
actual, err := parseErrorFromApiResponse(input)
175+
actual, err := parseErrorFromApiResponse(&input)
176176
if err != nil {
177177
t.Fatalf("parsing error from api response: %+v", err)
178178
}
@@ -197,7 +197,7 @@ func TestParseErrorFromApiResponse_UnexpectedLiteralStringShouldOutputHttpRespon
197197
Message: "Couldn't parse Azure API Response into a friendly error - please see the original HTTP Response for more details (and file a bug so we can fix this!).",
198198
Status: "Unknown",
199199
}
200-
actual, err := parseErrorFromApiResponse(input)
200+
actual, err := parseErrorFromApiResponse(&input)
201201
if err != nil {
202202
t.Fatalf("parsing error from api response: %+v", err)
203203
}
@@ -222,7 +222,7 @@ func TestParseErrorFromApiResponse_UnexpectedLiteralQuotedStringShouldOutputHttp
222222
Message: "Couldn't parse Azure API Response into a friendly error - please see the original HTTP Response for more details (and file a bug so we can fix this!).",
223223
Status: "Unknown",
224224
}
225-
actual, err := parseErrorFromApiResponse(input)
225+
actual, err := parseErrorFromApiResponse(&input)
226226
if err != nil {
227227
t.Fatalf("parsing error from api response: %+v", err)
228228
}
@@ -246,7 +246,7 @@ func TestParseErrorFromApiResponse_UnexpectedShouldOutputHttpResponseAnyway(t *t
246246
Message: "Couldn't parse Azure API Response into a friendly error - please see the original HTTP Response for more details (and file a bug so we can fix this!).",
247247
Status: "Unknown",
248248
}
249-
actual, err := parseErrorFromApiResponse(input)
249+
actual, err := parseErrorFromApiResponse(&input)
250250
if err != nil {
251251
t.Fatalf("parsing error from api response: %+v", err)
252252
}

‎sdk/client/resourcemanager/poller_helpers_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ func responseWithHttpStatusCode(input int) expectedResponse {
4444
}
4545
}
4646

47+
func responseWithNoTerminalState() expectedResponse {
48+
return expectedResponse{
49+
status: pointer.ToEnum[status](""),
50+
}
51+
}
52+
4753
func dropConnection(t *testing.T, w http.ResponseWriter) {
4854
httpHijacker, ok := w.(http.Hijacker)
4955
if !ok {

‎sdk/client/resourcemanager/poller_lro.go

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ type longRunningOperationPoller struct {
3434
}
3535

3636
func pollingUriForLongRunningOperation(resp *client.Response) string {
37-
pollingUrl := resp.Header.Get(http.CanonicalHeaderKey("Azure-AsyncOperation"))
37+
pollingUrl := resp.Header.Get("Azure-AsyncOperation")
3838
if pollingUrl == "" {
3939
pollingUrl = resp.Header.Get("Location")
4040
}
@@ -174,23 +174,8 @@ func (p *longRunningOperationPoller) Poll(ctx context.Context) (result *pollers.
174174
return nil, fmt.Errorf("internal-error: polling support for the Content-Type %q was not implemented: %+v", contentType, err)
175175
}
176176

177-
if op.Properties.ProvisioningState == "" && op.Status == "" {
178-
return nil, fmt.Errorf("expected either `provisioningState` or `status` to be returned from the LRO API but both were empty")
179-
}
180-
181-
for k, v := range longRunningOperationCustomStatuses {
182-
if strings.EqualFold(string(op.Properties.ProvisioningState), string(k)) {
183-
result.Status = v
184-
break
185-
}
186-
if strings.EqualFold(string(op.Status), string(k)) {
187-
result.Status = v
188-
break
189-
}
190-
}
191-
192-
if result.Status == pollers.PollingStatusFailed {
193-
lroError, parseError := parseErrorFromApiResponse(*result.HttpResponse.Response)
177+
if result.Status == pollers.PollingStatusFailed || op.Status == statusFailed || op.Properties.ProvisioningState == statusFailed {
178+
lroError, parseError := parseErrorFromApiResponse(result.HttpResponse.Response)
194179
if parseError != nil {
195180
return nil, parseError
196181
}
@@ -201,8 +186,8 @@ func (p *longRunningOperationPoller) Poll(ctx context.Context) (result *pollers.
201186
}
202187
}
203188

204-
if result.Status == pollers.PollingStatusCancelled {
205-
lroError, parseError := parseErrorFromApiResponse(*result.HttpResponse.Response)
189+
if result.Status == pollers.PollingStatusCancelled || op.Status == statusCanceled || op.Properties.ProvisioningState == statusCanceled {
190+
lroError, parseError := parseErrorFromApiResponse(result.HttpResponse.Response)
206191
if parseError != nil {
207192
return nil, parseError
208193
}
@@ -213,12 +198,16 @@ func (p *longRunningOperationPoller) Poll(ctx context.Context) (result *pollers.
213198
}
214199
}
215200

216-
if result.Status == "" {
217-
err = fmt.Errorf("`result.Status` was nil/empty - `op.Status` was %q / `op.Properties.ProvisioningState` was %q", string(op.Status), string(op.Properties.ProvisioningState))
201+
if result.Status == pollers.PollingStatusSucceeded || op.Status == statusSucceeded || op.Properties.ProvisioningState == statusSucceeded {
202+
result.Status = pollers.PollingStatusSucceeded
203+
return
218204
}
205+
206+
// If we don't have a terminal status anywhere, we must assume we're still in progress
207+
result.Status = pollers.PollingStatusInProgress
219208
}
220209

221-
return
210+
return result, nil
222211
}
223212

224213
type operationResult struct {
@@ -249,3 +238,11 @@ const (
249238
statusInProgress status = "InProgress"
250239
statusSucceeded status = "Succeeded"
251240
)
241+
242+
func statusIsTerminal(s status) bool {
243+
switch s {
244+
case statusCanceled, statusCancelled, statusFailed, statusSucceeded, statusInProgress:
245+
return true
246+
}
247+
return false
248+
}

0 commit comments

Comments
 (0)