Allow force deleting agent pools #435
Conversation
komalali
force-pushed
the
komal/allow-force-deleting-agent-pool
branch
from
8c35ce5
to
d931b1d
Compare
komalali
force-pushed
the
komal/allow-force-deleting-agent-pool
branch
from
d931b1d
to
15dfae1
Compare
komalali
force-pushed
the
komal/allow-force-deleting-agent-pool
branch
from
69dba5e
to
1d8d156
Compare
| Urn: "urn:beep-boop", | ||
| } | ||
|
|
||
| resp, err := provider.Delete(&req) |
There was a problem hiding this comment.
isn't this just testing that the mock returns what you told it to return? 😞
I don't think this is useful
There was a problem hiding this comment.
I think a better test would be verifying that the force=true param gets passed through when destroying the stack that has the ForceDestroy input set
There was a problem hiding this comment.
Yeah that's fair. Anything that's not a full integration test has been somewhat tricky to test in the provider. Let me spend some time figuring this out.
There was a problem hiding this comment.
it seems like most of the other tests in the package are the same way so I don't want to block you 🤷♀️
but if you figure out something I'd love to know for #436 !
There was a problem hiding this comment.
Yeah I think I'm going to merge this just to unblock Scala but this is a good question that we do need an answer to beyond E2E tests
| @@ -39,7 +39,7 @@ const stackWebhook = new service.Webhook("stack-webhook", { | |||
| organizationName: serviceOrg, | |||
| projectName: pulumi.getProject(), | |||
| stackName: pulumi.getStack(), | |||
| payloadUrl: "https://example.com", | |||
| payloadUrl: "https://hooks.slack.com/blahblah", | |||
There was a problem hiding this comment.
Needed an update due to new URL validation for webhooks
| @@ -106,18 +107,23 @@ func (c *Client) UpdateAgentPool(ctx context.Context, agentPoolId, orgName, name | |||
| return nil | |||
| } | |||
|
|
|||
| func (c *Client) DeleteAgentPool(ctx context.Context, agentPoolId, orgName string) error { | |||
| func (c *Client) DeleteAgentPool(ctx context.Context, agentPoolId, orgName string, forceDestroy bool) error { | |||
There was a problem hiding this comment.
super nit: should we consider making options like this a struct in case we need to add more later?
| @@ -31,6 +32,9 @@ func GenerateAgentPoolProperties(input PulumiServiceAgentPoolInput, agentPool pu | |||
| if input.Description != "" { | |||
| inputMap["description"] = resource.NewPropertyValue(input.Description) | |||
| } | |||
| if input.ForceDestroy { | |||
| inputMap["forceDestroy"] = resource.NewPropertyValue(input.ForceDestroy) | |||
There was a problem hiding this comment.
Might want to make forceDestroy a constant
There was a problem hiding this comment.
Hmm, not sure it makes sense to make one input a constant and not any of the others... so I'd leave that to a future optimization since this is not the pattern anywhere in the provider.
|
This PR has been shipped in release v0.27.0. |
Sometimes, a user may want to force delete an agent pool, even if it is associated with existing deployment settings. This is already supported in the API, bringing the functionality to PSP.