-
Notifications
You must be signed in to change notification settings - Fork 232
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
helper/resource: Various small fixes #1091
Conversation
Reference: #1063 Reference: hashicorp/go-plugin#220 This includes the following small bug fixes: - Clarifies/fixes the trace logging during `terraform show` commands - Improves performance slightly for `terraform show` raw plan usage (terraform-exec already captures and returns stdout for raw plans) - Prevents a goroutine leak specific to the detached stop context in `schema.GRPCProvider` Previously, leak detection on testing would contain an entry per Terraform command such as: ``` [Goroutine 276 in state select, with github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.mergeStop on top of the stack: goroutine 276 [select]: github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.mergeStop({0x103a5f298?, 0x14000390c00?}, 0x1400033e5f0, 0x140004bb560) /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/grpc_provider.go:46 +0x64 created by github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).StopContext /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/grpc_provider.go:63 +0x178 ] ``` Leaked goroutines such as: ``` [Goroutine 53 in state select, with github.com/hashicorp/go-plugin.(*gRPCBrokerServer).Recv on top of the stack: goroutine 53 [select]: github.com/hashicorp/go-plugin.(*gRPCBrokerServer).Recv(0x29?) /Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_broker.go:121 +0x58 github.com/hashicorp/go-plugin.(*GRPCBroker).Run(0x140003ca190) /Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_broker.go:411 +0x40 created by github.com/hashicorp/go-plugin.(*GRPCServer).Init /Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_server.go:85 +0x424 ] ``` Will likely require a fix upstream in go-plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
// For legacy providers, call the StopProvider RPC so the StopContext | ||
// goroutine is cleaned up properly. | ||
for _, legacyProviderServer := range legacyProviderServers { | ||
legacyProviderServer.StopProvider(ctx, nil) //nolint:errcheck // does not return errors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For what it is worth, we could consider calling this RPC on all providers started by the testing framework as a better approximation to normal Terraform executions, but it only makes sense for sdk/framework-based providers if we enable them to implement close/stop logic that gets called during that RPC.
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Reference: #1063
Reference: hashicorp/go-plugin#220
This includes the following small bug fixes:
terraform show
commandsterraform show
raw plan usage (terraform-exec already captures and returns stdout for raw plans)schema.GRPCProvider
Previously, leak detection on testing would contain an entry per Terraform command such as:
Leaked goroutines such as:
Will likely require a fix upstream in go-plugin.