Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add wait_for_instances field to IGM and self_link option to the IG data source #1222

Merged
merged 2 commits into from
Mar 20, 2018

Conversation

danawillow
Copy link
Contributor

Fixes #1213.

(even though it's two changes, doing it in a single PR enables the test I wanted to write)

$ make testacc TEST=./google TESTARGS='-run=TestAccDataSourceGoogleComputeInstanceGroup_fromIGM'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v -run=TestAccDataSourceGoogleComputeInstanceGroup_fromIGM -timeout 120m
=== RUN   TestAccDataSourceGoogleComputeInstanceGroup_fromIGM
=== PAUSE TestAccDataSourceGoogleComputeInstanceGroup_fromIGM
=== CONT  TestAccDataSourceGoogleComputeInstanceGroup_fromIGM
--- PASS: TestAccDataSourceGoogleComputeInstanceGroup_fromIGM (121.64s)
PASS
ok  	github.com/terraform-providers/terraform-provider-google/google	121.848s

Timeout: d.Timeout(schema.TimeoutCreate),
}
_, err := conf.WaitForState()
// If err is nil, success.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone later adds some code below this, it'll get missed - might be good to add a comment saying not to do that? Not sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just updated it to return the error only if there is one, that seemed like a much more standard way to go about this.

@@ -257,17 +257,17 @@ func waitForInstancesRefreshFunc(f getInstanceManagerFunc, d *schema.ResourceDat
log.Printf("[WARNING] Error in fetching manager while waiting for instances to come up: %s\n", err)
return nil, "error", err
}
if creatingCount := m.CurrentActions.Creating + m.CurrentActions.CreatingWithoutRetries; creatingCount > 0 {
return creatingCount, "creating", nil
if done := m.CurrentActions.None; done < m.TargetSize {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slaps forehead

I think I remember writing this code, so thanks for catching my mistake there - obviously the instances shouldn't report ready if they're FAILED.

@danawillow danawillow merged commit 28efae5 into hashicorp:master Mar 20, 2018
@danawillow danawillow deleted the is-1213 branch March 20, 2018 21:20
chrisst pushed a commit to chrisst/terraform-provider-google that referenced this pull request Nov 9, 2018
…ta source (hashicorp#1222)

* Add wait_for_instances field to IGM and self_link option to the IG data source

* don't be clever with errors
modular-magician added a commit to modular-magician/terraform-provider-google that referenced this pull request Oct 10, 2019
@ghost
Copy link

ghost commented Mar 29, 2020

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!

@ghost ghost locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add operation to wait for MIG resize before completion
2 participants