Conversation
add tests update changelog
03462d8 to
3bc1359
Compare
Maed223
left a comment
There was a problem hiding this comment.
Smoke testing is looking good from the perspective of the Stacks plugin 🚀 ! Mostly just one point about about an additional bit of needed info in the StacksPluginConfig
Otherwise I'm realizing that we aren't fully honoring the -json flag here when we have diagnostics returned. Though it's an awkward situation since not all stacks subcommands implement a json view. Don't think it's a massive problem since we'd only return diags here if the hostname/token env vars aren't configured, or if the plugin dev override is in use. Wanted to call that out, but I don't have a strong opinion on how/if to handle it.
|
Found out the cause (and the fixes) of the occasional nil panics we were seeing in the prototype, stemming from services here not being properly managed. One note is that in addition to what we were seeing in Accessing the service before it's fully set upIn This looks to be a bit of a race condition since we start the services within asynchronous go routines: dependenciesBrokerID := c.Broker.NextId()
go c.Broker.AcceptAndServe(dependenciesBrokerID, dependenciesServerFunc)
packagesBrokerID := c.Broker.NextId()
go c.Broker.AcceptAndServe(packagesBrokerID, packagesServerFunc)
stacksBrokerID := c.Broker.NextId()
go c.Broker.AcceptAndServe(stacksBrokerID, stacksServerFunc)but SolutionImplement a mechanism to ensure all broker services are fully initialized before proceeding: func (c GRPCStacksClient) registerBrokers(stdout, stderr io.Writer) brokerIDs {
// ... existing code ...
// Create channels to signal when each service is ready
dependenciesReady := make(chan struct{})
packagesReady := make(chan struct{})
stacksReady := make(chan struct{})
dependenciesServerFunc := func(opts []grpc.ServerOption) *grpc.Server {
s = grpc.NewServer(opts...)
dependencies.RegisterDependenciesServer(s, dependenciesServer)
dependenciesServer.ActivateRPCServer(newDependenciesServer(handles, c.Services))
close(dependenciesReady) // Signal that this service is ready
return s
}
// Similar modifications for other services...
// Wait for all services to be ready
<-dependenciesReady
<-packagesReady
<-stacksReady
return brokerIDs{...}
}The service closing before the complete execution of the commandThis was the class of problem we were seeing in the In SolutionGet rid of the dependenciesServerFunc := func(opts []grpc.ServerOption) *grpc.Server {
s := grpc.NewServer(opts...) // the := creating a new variable
// ... other code ...
return s
} |
| ctx: ctx, | ||
| serviceURL: serviceURL, | ||
| httpClient: retryableClient, | ||
| pluginName: "cloudplugin", |
There was a problem hiding this comment.
It looks like you are scrubbing the term "cloudplugin" from this package, which I think is a good idea. But it's not clear to me if you went as far as you could go or if you just missed a few of them. This is just one example.
IMO, it's OK to remove or manipulate all cloudplugin features from terraform and shift those over to stacks
There was a problem hiding this comment.
I went about the rearrangement by creating a base client so that each plugin could use an instance of the base client, that's why there is still both cloudclient and stacksclient in the package.
Maed223
left a comment
There was a problem hiding this comment.
Smoke tested, and looks great from my perspective 🚀
| var serverWG sync.WaitGroup | ||
| // wait for all 3 servers to start | ||
| serverWG.Add(3) | ||
|
|
|
I'm getting the following error when I use |
|
@henrythor - Could you please open a bug report describing the issue you've encountered? Then your report can go through our triage process. |
|
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. |
Fixes #
terraform stacksterraform stacksexposes some Terraform Stack operations through the cli. The available subcommands depend on the stacks plugin implementation.NOTE: the
terraform stacks -helpwill be implemented in a separate PRTarget Release
1.13.x
CHANGELOG entry
The new command
terraform stacksexposes some Terraform Stack operations through the cli. The available subcommands depend on the stacks plugin implementation. Useterraform stacks -helpto see available commands.This change is user-facing and I added a changelog entry.
This change is not user-facing.
NOTE: Test steps are written in the project slack channel