Hotfix: Upgrade go-autorest to v9.5.2 #617

metacpp · 2017-12-11T21:07:38Z

No description provided.

…ing 429 errors.

sebastus · 2017-12-12T05:19:07Z

the part that I know about (sender.go) looks good.

On Mon, Dec 11, 2017 at 1:07 PM, Su Shi ***@***.***> wrote: @metacpp <https://github.com/metacpp> requested your review on: hashicorp/terraform-provider-azurerm#617 <#617> Hotfix: Upgrade go-autorest to v9.5.1. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#617 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABOrrLUUW8HE67bKZlRUBDAQwURiMms-ks5s_ZmtgaJpZM4Q-CNq> .

tombuildsstuff

LGTM - just running the tests now 👍

tombuildsstuff · 2017-12-12T13:16:44Z

Looks like there's a bug in this:

Test ended in panic.

------- Stdout: -------
=== RUN   TestAccAzureRMSqlElasticPool_basic

------- Stderr: -------
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x702133]

goroutine 414 [running]:
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/go-autorest/autorest.DoRetryForStatusCodes.func1.1(0xc4204b8000, 0xc4205e7300, 0x0, 0xc420054d08)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/go-autorest/autorest/sender.go:235 +0x1a3
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/go-autorest/autorest.SenderFunc.Do(0xc4205e7300, 0xc4204b8000, 0xc420054d08, 0x1, 0x1)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/go-autorest/autorest/sender.go:38 +0x30
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/go-autorest/autorest.SendWithSender(0x2243780, 0xc4203577c0, 0xc4204b8000, 0xc420054d08, 0x1, 0x1, 0x1280000000000, 0x1, 0xa)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/go-autorest/autorest/sender.go:80 +0x75
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/go-autorest/autorest/azure.DoRetryWithRegistration.func1.1(0xc4204b8000, 0xc4203a4000, 0x0, 0xc420054f70)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/go-autorest/autorest/azure/rp.go:40 +0x1d4
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/go-autorest/autorest.SenderFunc.Do(0xc4203a4000, 0xc4204b8000, 0xc420054f70, 0x1, 0x1)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/go-autorest/autorest/sender.go:38 +0x30
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/go-autorest/autorest.SendWithSender(0x2243780, 0xc4203577c0, 0xc4204b8000, 0xc420054f70, 0x1, 0x1, 0xdf8475800, 0xd18c2e2800, 0x3)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/go-autorest/autorest/sender.go:80 +0x75
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/azure-sdk-for-go/arm/resources/resources.ProvidersClient.ListSender(0x223e640, 0xc420346e60, 0x2243ec0, 0xc4202d2660, 0x0, 0x0, 0xdf8475800, 0xd18c2e2800, 0x3, 0x6fc23ac00, ...)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/azure-sdk-for-go/arm/resources/resources/providers.go:166 +0x110
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/azure-sdk-for-go/arm/resources/resources.ProvidersClient.List(0x223e640, 0xc420346e60, 0x2243ec0, 0xc4202d2660, 0x0, 0x0, 0xdf8475800, 0xd18c2e2800, 0x3, 0x6fc23ac00, ...)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/Azure/azure-sdk-for-go/arm/resources/resources/providers.go:123 +0x105
github.com/terraform-providers/terraform-provider-azurerm/azurerm.providerConfigure.func1(0xc42010c150, 0xc42010c150, 0xc42018c6c0, 0xc4202d2420, 0x0)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/azurerm/provider.go:226 +0x577
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/helper/schema.(*Provider).Configure(0xc42010de30, 0xc42018c6c0, 0x7, 0xc42018c6c0)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/helper/schema/provider.go:240 +0xdf
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform.(*BuiltinEvalContext).ConfigureProvider(0xc420368000, 0xc420018fa1, 0x7, 0xc42018c6c0, 0x0, 0x31)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform/eval_context_builtin.go:157 +0xe8
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform.(*EvalConfigProvider).Eval(0xc4202d2280, 0x224d620, 0xc420368000, 0x2, 0x2, 0x1778c6d, 0x4)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform/eval_provider.go:65 +0x53
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform.EvalRaw(0x223fbc0, 0xc4202d2280, 0x224d620, 0xc420368000, 0xc42006c0f0, 0x2, 0xc4205c8210, 0x2b)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform/eval.go:53 +0x173
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform.(*EvalSequence).Eval(0xc4202d22a0, 0x224d620, 0xc420368000, 0x2, 0x2, 0x1778c6d, 0x4)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform/eval_sequence.go:14 +0x7e
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform.EvalRaw(0x2240340, 0xc4202d22a0, 0x224d620, 0xc420368000, 0x2b, 0x0, 0x0, 0x2b)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform/eval.go:53 +0x173
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform.(*EvalOpFilter).Eval(0xc42018c420, 0x224d620, 0xc420368000, 0x2, 0x2, 0x1778c6d, 0x4)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform/eval_filter_operation.go:37 +0x4c
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform.EvalRaw(0x2240100, 0xc42018c420, 0x224d620, 0xc420368000, 0x0, 0x0, 0x0, 0x0)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform/eval.go:53 +0x173
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform.(*EvalSequence).Eval(0xc4202d22c0, 0x224d620, 0xc420368000, 0x2, 0x2, 0x1778c6d, 0x4)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform/eval_sequence.go:14 +0x7e
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform.EvalRaw(0x2240340, 0xc4202d22c0, 0x224d620, 0xc420368000, 0x15c68e0, 0x224e503, 0x1574b00, 0xc420346180)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform/eval.go:53 +0x173
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform.Eval(0x2240340, 0xc4202d22c0, 0x224d620, 0xc420368000, 0xc4202d22c0, 0x2240340, 0xc4202d22c0, 0x48e954)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform/eval.go:34 +0x4d
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform.(*Graph).walk.func1(0x16b68a0, 0xc42000f7b8, 0x0, 0x0)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/terraform/graph.go:126 +0xd0d
github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/dag.(*Walker).walkVertex(0xc4203cafc0, 0x16b68a0, 0xc42000f7b8, 0xc420694200)
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/dag/walk.go:387 +0x3c1
created by github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/dag.(*Walker).Update
    /opt/teamcity-agent/work/b5bd5badde638102/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/dag/walk.go:310 +0x1364

tombuildsstuff

The acceptance tests have showed that this panics when the connection is reset/dropped (whereas the current version of Go-AutoRest will instead return an error, which can be handled gracefully).

Can we update this to check that resp isn't nil before accessing it, so that we can handle this in a more graceful fashion?

tombuildsstuff · 2017-12-12T13:20:25Z

vendor/github.com/Azure/go-autorest/autorest/sender.go

 					return resp, err
 				}
 				delayed := DelayWithRetryAfter(resp, r.Cancel)
 				if !delayed {
 					DelayForBackoff(backoff, attempt, r.Cancel)
 				}
+				// don't count a 429 against the number of attempts
+				// so that we continue to retry until it succeeds
+				if resp.StatusCode != http.StatusTooManyRequests {


👋🏻 @jhendrixMSFT @marstr we're seeing a crash here - we handle checking for this internally to see if there was a dropped connection from Azure (which happens often enough that we need to handle it) - any chance that we could check if resp != nil before checking the status code?

I will get this fixed and release a v9.5.2 today.

awesome - thanks @jhendrixMSFT :)

The fix is in an the release has been tagged. https://github.com/Azure/go-autorest/releases/tag/v9.5.2

mbfrahry · 2017-12-12T19:43:56Z

The above test passes now.

make testacc TEST=./azurerm TESTARGS="-run=TestAccAzureRMSqlElasticPool_basic"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./azurerm -v -run=TestAccAzureRMSqlElasticPool_basic -timeout 180m
=== RUN   TestAccAzureRMSqlElasticPool_basic
--- PASS: TestAccAzureRMSqlElasticPool_basic (134.18s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	134.206s

mbfrahry

LGTM

tombuildsstuff · 2017-12-13T10:54:27Z

CHANGELOG.md

+IMPROVEMENTS:
+
+* Too Many Request: Upgrade go-autorest to v9.5.2 to keep retrying all the time while meeting 429 status code. ([#617](https://github.com/terraform-providers/terraform-provider-azurerm/pull/617))
+


@metacpp heads up that we don't manage this file through PR's (also there is no v0.3.4 of the Provider) - so I'm going to push a commit to revert this file/change (and update it as we normally would)

code changes

tombuildsstuff

LGTM - thanks for re-vendoring this 👍

tombuildsstuff · 2017-12-13T12:47:52Z

Meant to add - ran the test suite and this seems fine with the changes in AutoRest v9.5.2 👍

ghost · 2020-04-01T14:57:29Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

Hotfix: Upgrade go-autorest to v9.5.1 to have more retries after meet…

f263b13

…ing 429 errors.

metacpp requested review from tombuildsstuff, sebastus and JunyiYi December 11, 2017 21:07

tombuildsstuff added the dependencies label Dec 11, 2017

tombuildsstuff approved these changes Dec 12, 2017

View reviewed changes

tombuildsstuff previously requested changes Dec 12, 2017

View reviewed changes

Upgrade to go-autorest v9.5.2 to contain the fix.

76e3108

mbfrahry approved these changes Dec 12, 2017

View reviewed changes

metacpp changed the title ~~Hotfix: Upgrade go-autorest to v9.5.1~~ Hotfix: Upgrade go-autorest to v9.5.2 Dec 12, 2017

tombuildsstuff reviewed Dec 13, 2017

View reviewed changes

tombuildsstuff approved these changes Dec 13, 2017

View reviewed changes

tombuildsstuff merged commit a8dca99 into hashicorp:master Dec 13, 2017

tombuildsstuff added a commit that referenced this pull request Dec 13, 2017

Updating to include #617

7c49a37

ghost locked and limited conversation to collaborators Apr 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hotfix: Upgrade go-autorest to v9.5.2 #617

Hotfix: Upgrade go-autorest to v9.5.2 #617

metacpp commented Dec 11, 2017

sebastus commented Dec 12, 2017 via email

tombuildsstuff left a comment

tombuildsstuff commented Dec 12, 2017 •

edited

Loading

tombuildsstuff left a comment

tombuildsstuff Dec 12, 2017

jhendrixMSFT Dec 12, 2017

tombuildsstuff Dec 12, 2017

jhendrixMSFT Dec 12, 2017

mbfrahry commented Dec 12, 2017

mbfrahry left a comment

tombuildsstuff Dec 13, 2017

tombuildsstuff left a comment

tombuildsstuff commented Dec 13, 2017

ghost commented Apr 1, 2020

		IMPROVEMENTS:

		* Too Many Request: Upgrade go-autorest to v9.5.2 to keep retrying all the time while meeting 429 status code. ([#617](https://github.com/terraform-providers/terraform-provider-azurerm/pull/617))

Hotfix: Upgrade go-autorest to v9.5.2 #617

Hotfix: Upgrade go-autorest to v9.5.2 #617

Conversation

metacpp commented Dec 11, 2017

sebastus commented Dec 12, 2017 via email

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff commented Dec 12, 2017 • edited Loading

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff Dec 12, 2017

Choose a reason for hiding this comment

jhendrixMSFT Dec 12, 2017

Choose a reason for hiding this comment

tombuildsstuff Dec 12, 2017

Choose a reason for hiding this comment

jhendrixMSFT Dec 12, 2017

Choose a reason for hiding this comment

mbfrahry commented Dec 12, 2017

mbfrahry left a comment

Choose a reason for hiding this comment

tombuildsstuff Dec 13, 2017

Choose a reason for hiding this comment

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff commented Dec 13, 2017

ghost commented Apr 1, 2020

tombuildsstuff commented Dec 12, 2017 •

edited

Loading