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

helper/resource: Various small fixes #1091

Merged
merged 2 commits into from
Nov 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/1091.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
helper/resource: Prevented goroutine leak per Terraform command when testing terraform-plugin-sdk based providers via `Providers` or `ProviderFactories`
```
21 changes: 20 additions & 1 deletion helper/resource/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,13 @@ func runProviderCommand(ctx context.Context, t testing.T, f func() error, wd *pl
host = v
}

// schema.Provider have a global stop context that is created outside
// the server context and have their own associated goroutine. Since
// Terraform does not call the StopProvider RPC to stop the server in
// reattach mode, ensure that we save these servers to later call that
// RPC and end those goroutines.
legacyProviderServers := make([]*schema.GRPCProviderServer, 0, len(factories.legacy))

// Spin up gRPC servers for every provider factory, start a
// WaitGroup to listen for all of the close channels.
var wg sync.WaitGroup
Expand All @@ -180,13 +187,19 @@ func runProviderCommand(ctx context.Context, t testing.T, f func() error, wd *pl
// shut down.
wg.Add(1)

grpcProviderServer := schema.NewGRPCProviderServer(provider)
legacyProviderServers = append(legacyProviderServers, grpcProviderServer)

// Ensure StopProvider is always called when returning early.
defer grpcProviderServer.StopProvider(ctx, nil) //nolint:errcheck // does not return errors

// configure the settings our plugin will be served with
// the GRPCProviderFunc wraps a non-gRPC provider server
// into a gRPC interface, and the logger just discards logs
// from go-plugin.
opts := &plugin.ServeOpts{
GRPCProviderFunc: func() tfprotov5.ProviderServer {
return schema.NewGRPCProviderServer(provider)
return grpcProviderServer
},
Logger: hclog.New(&hclog.LoggerOptions{
Name: "plugintest",
Expand Down Expand Up @@ -430,6 +443,12 @@ func runProviderCommand(ctx context.Context, t testing.T, f func() error, wd *pl
// get closed, and we'll hang here.
cancel()

// 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
Copy link
Contributor Author

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.

}

logging.HelperResourceTrace(ctx, "Waiting for providers to stop")

// wait for the servers to actually shut down; it may take a moment for
Expand Down
63 changes: 63 additions & 0 deletions helper/resource/plugin_test.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package resource

import (
"context"
"fmt"
"os"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform-plugin-go/tfprotov5"
"github.com/hashicorp/terraform-plugin-go/tfprotov6"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/internal/plugintest"
)

func TestProtoV5ProviderFactoriesMerge(t *testing.T) {
Expand Down Expand Up @@ -234,3 +238,62 @@ func TestSdkProviderFactoriesMerge(t *testing.T) {
})
}
}

func TestRunProviderCommand(t *testing.T) {
currentDir, err := os.Getwd()

if err != nil {
t.Fatalf("unable to get working directory: %s", err)
}

ctx := context.Background()
funcCalled := false
helper := plugintest.AutoInitProviderHelper(ctx, currentDir)

err = runProviderCommand(
ctx,
t,
func() error {
funcCalled = true
return nil
},
helper.RequireNewWorkingDir(ctx, t),
&providerFactories{
legacy: map[string]func() (*schema.Provider, error){
"examplecloud": func() (*schema.Provider, error) { //nolint:unparam // required signature
return &schema.Provider{
ResourcesMap: map[string]*schema.Resource{
"examplecloud_thing": {
CreateContext: func(ctx context.Context, d *schema.ResourceData, i interface{}) diag.Diagnostics {
d.SetId("id")

return nil
},
DeleteContext: func(_ context.Context, _ *schema.ResourceData, _ interface{}) diag.Diagnostics {
return nil
},
ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics {
return nil
},
Schema: map[string]*schema.Schema{
"id": {
Computed: true,
Type: schema.TypeString,
},
},
},
},
}, nil
},
},
},
)

if err != nil {
t.Fatal(err)
}

if !funcCalled {
t.Error("expected func to be called")
}
}
23 changes: 8 additions & 15 deletions internal/plugintest/working_dir.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package plugintest

import (
"bytes"
"context"
"encoding/json"
"fmt"
"io"
"os"
"path/filepath"

Expand Down Expand Up @@ -281,11 +279,11 @@ func (wd *WorkingDir) SavedPlan(ctx context.Context) (*tfjson.Plan, error) {
return nil, fmt.Errorf("there is no current saved plan")
}

logging.HelperResourceTrace(ctx, "Calling Terraform CLI apply command")
logging.HelperResourceTrace(ctx, "Calling Terraform CLI show command for JSON plan")

plan, err := wd.tf.ShowPlanFile(context.Background(), wd.planFilename(), tfexec.Reattach(wd.reattachInfo))

logging.HelperResourceTrace(ctx, "Calling Terraform CLI apply command")
logging.HelperResourceTrace(ctx, "Calling Terraform CLI show command for JSON plan")

return plan, err
}
Expand All @@ -299,34 +297,29 @@ func (wd *WorkingDir) SavedPlanRawStdout(ctx context.Context) (string, error) {
return "", fmt.Errorf("there is no current saved plan")
}

var ret bytes.Buffer

wd.tf.SetStdout(&ret)
defer wd.tf.SetStdout(io.Discard)

logging.HelperResourceTrace(ctx, "Calling Terraform CLI show command")
logging.HelperResourceTrace(ctx, "Calling Terraform CLI show command for stdout plan")

_, err := wd.tf.ShowPlanFileRaw(context.Background(), wd.planFilename(), tfexec.Reattach(wd.reattachInfo))
stdout, err := wd.tf.ShowPlanFileRaw(context.Background(), wd.planFilename(), tfexec.Reattach(wd.reattachInfo))

logging.HelperResourceTrace(ctx, "Called Terraform CLI show command")
logging.HelperResourceTrace(ctx, "Called Terraform CLI show command for stdout plan")

if err != nil {
return "", err
}

return ret.String(), nil
return stdout, nil
}

// State returns an object describing the current state.
//

// If the state cannot be read, State returns an error.
func (wd *WorkingDir) State(ctx context.Context) (*tfjson.State, error) {
logging.HelperResourceTrace(ctx, "Calling Terraform CLI show command")
logging.HelperResourceTrace(ctx, "Calling Terraform CLI show command for JSON state")

state, err := wd.tf.Show(context.Background(), tfexec.Reattach(wd.reattachInfo))

logging.HelperResourceTrace(ctx, "Called Terraform CLI show command")
logging.HelperResourceTrace(ctx, "Called Terraform CLI show command for JSON state")

return state, err
}
Expand Down