Skip to content

Commit

Permalink
Implement Disconnect on ExternalClient interface
Browse files Browse the repository at this point in the history
Signed-off-by: smcavallo <[email protected]>
  • Loading branch information
smcavallo authored and mergenci committed Jan 8, 2025
1 parent 9f51ffe commit b6820b0
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 23 deletions.
4 changes: 4 additions & 0 deletions pkg/controller/external.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ type external struct {
logger logging.Logger
}

func (e *external) Disconnect(ctx context.Context) error {
return nil
}

func (e *external) scheduleProvider(name string) (bool, error) {
if e.providerScheduler == nil || e.workspace == nil {
return false, nil
Expand Down
14 changes: 9 additions & 5 deletions pkg/controller/external_async_tfpluginfw.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,13 +224,13 @@ func (n *terraformPluginFrameworkAsyncExternalClient) Update(_ context.Context,
return managed.ExternalUpdate{}, n.opTracker.LastOperation.Error()
}

func (n *terraformPluginFrameworkAsyncExternalClient) Delete(_ context.Context, mg xpresource.Managed) error { //nolint:contextcheck // we intentionally use a fresh context for the async operation
func (n *terraformPluginFrameworkAsyncExternalClient) Delete(_ context.Context, mg xpresource.Managed) (managed.ExternalDelete, error) { //nolint:contextcheck // we intentionally use a fresh context for the async operation
switch {
case n.opTracker.LastOperation.Type == "delete":
n.opTracker.logger.Debug("The previous delete operation is still ongoing")
return nil
return managed.ExternalDelete{}, nil
case !n.opTracker.LastOperation.MarkStart("delete"):
return errors.Errorf("%s operation that started at %s is still running", n.opTracker.LastOperation.Type, n.opTracker.LastOperation.StartTime().String())
return managed.ExternalDelete{}, errors.Errorf("%s operation that started at %s is still running", n.opTracker.LastOperation.Type, n.opTracker.LastOperation.StartTime().String())
}

ctx, cancel := context.WithDeadline(context.Background(), n.opTracker.LastOperation.StartTime().Add(defaultAsyncTimeout))
Expand All @@ -255,8 +255,12 @@ func (n *terraformPluginFrameworkAsyncExternalClient) Delete(_ context.Context,
defer ph.recoverIfPanic()

n.opTracker.logger.Debug("Async delete starting...")
ph.err = n.terraformPluginFrameworkExternalClient.Delete(ctx, mg)
_, ph.err = n.terraformPluginFrameworkExternalClient.Delete(ctx, mg)
}()

return n.opTracker.LastOperation.Error()
return managed.ExternalDelete{}, n.opTracker.LastOperation.Error()
}

func (n *terraformPluginFrameworkAsyncExternalClient) Disconnect(_ context.Context) error {
return nil
}
14 changes: 9 additions & 5 deletions pkg/controller/external_async_tfpluginsdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,13 +202,13 @@ func (n *terraformPluginSDKAsyncExternal) Update(_ context.Context, mg xpresourc
return managed.ExternalUpdate{}, n.opTracker.LastOperation.Error()
}

func (n *terraformPluginSDKAsyncExternal) Delete(_ context.Context, mg xpresource.Managed) error { //nolint:contextcheck // we intentionally use a fresh context for the async operation
func (n *terraformPluginSDKAsyncExternal) Delete(_ context.Context, mg xpresource.Managed) (managed.ExternalDelete, error) { //nolint:contextcheck // we intentionally use a fresh context for the async operation
switch {
case n.opTracker.LastOperation.Type == "delete":
n.opTracker.logger.Debug("The previous delete operation is still ongoing", "tfID", n.opTracker.GetTfID())
return nil
return managed.ExternalDelete{}, nil
case !n.opTracker.LastOperation.MarkStart("delete"):
return errors.Errorf("%s operation that started at %s is still running", n.opTracker.LastOperation.Type, n.opTracker.LastOperation.StartTime().String())
return managed.ExternalDelete{}, errors.Errorf("%s operation that started at %s is still running", n.opTracker.LastOperation.Type, n.opTracker.LastOperation.StartTime().String())
}

ctx, cancel := context.WithDeadline(context.Background(), n.opTracker.LastOperation.StartTime().Add(defaultAsyncTimeout))
Expand All @@ -233,8 +233,12 @@ func (n *terraformPluginSDKAsyncExternal) Delete(_ context.Context, mg xpresourc
defer ph.recoverIfPanic()

n.opTracker.logger.Debug("Async delete starting...", "tfID", n.opTracker.GetTfID())
ph.err = n.terraformPluginSDKExternal.Delete(ctx, mg)
_, ph.err = n.terraformPluginSDKExternal.Delete(ctx, mg)
}()

return n.opTracker.LastOperation.Error()
return managed.ExternalDelete{}, n.opTracker.LastOperation.Error()
}

func (n *terraformPluginSDKAsyncExternal) Disconnect(_ context.Context) error {
return nil
}
2 changes: 1 addition & 1 deletion pkg/controller/external_async_tfpluginsdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ func TestAsyncTerraformPluginSDKDelete(t *testing.T) {
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
terraformPluginSDKAsyncExternal := prepareTerraformPluginSDKAsyncExternal(tc.args.r, tc.args.cfg, tc.args.fns)
err := terraformPluginSDKAsyncExternal.Delete(context.TODO(), tc.args.obj)
_, err := terraformPluginSDKAsyncExternal.Delete(context.TODO(), tc.args.obj)
if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
t.Errorf("\n%s\nConnect(...): -want error, +got error:\n", diff)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/external_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ func TestDelete(t *testing.T) {
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
e := &external{workspace: tc.w, callback: tc.c, config: tc.cfg}
err := e.Delete(context.TODO(), tc.args.obj)
_, err := e.Delete(context.TODO(), tc.args.obj)
if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
t.Errorf("\n%s\nCreate(...): -want error, +got error:\n%s", tc.reason, diff)
}
Expand Down
18 changes: 11 additions & 7 deletions pkg/controller/external_tfpluginfw.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,17 +520,17 @@ func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg
return managed.ExternalUpdate{}, nil
}

func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ xpresource.Managed) error {
func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ xpresource.Managed) (managed.ExternalDelete, error) {
n.logger.Debug("Deleting the external resource")

tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceValueTerraformType)
if err != nil {
return errors.Wrap(err, "cannot construct dynamic value for TF Config")
return managed.ExternalDelete{}, errors.Wrap(err, "cannot construct dynamic value for TF Config")
}
// set an empty planned state, this corresponds to deleting
plannedState, err := tfprotov5.NewDynamicValue(n.resourceValueTerraformType, tftypes.NewValue(n.resourceValueTerraformType, nil))
if err != nil {
return errors.Wrap(err, "cannot set the planned state for deletion")
return managed.ExternalDelete{}, errors.Wrap(err, "cannot set the planned state for deletion")
}

applyRequest := &tfprotov5.ApplyResourceChangeRequest{
Expand All @@ -542,22 +542,22 @@ func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ x
start := time.Now()
applyResponse, err := n.server.ApplyResourceChange(ctx, applyRequest)
if err != nil {
return errors.Wrap(err, "cannot delete resource")
return managed.ExternalDelete{}, errors.Wrap(err, "cannot delete resource")
}
metrics.ExternalAPITime.WithLabelValues("delete").Observe(time.Since(start).Seconds())
if fatalDiags := getFatalDiagnostics(applyResponse.Diagnostics); fatalDiags != nil {
return errors.Wrap(fatalDiags, "resource deletion call returned error diags")
return managed.ExternalDelete{}, errors.Wrap(fatalDiags, "resource deletion call returned error diags")
}
n.opTracker.SetFrameworkTFState(applyResponse.NewState)

newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(n.resourceValueTerraformType)
if err != nil {
return errors.Wrap(err, "cannot unmarshal state after deletion")
return managed.ExternalDelete{}, errors.Wrap(err, "cannot unmarshal state after deletion")
}
// mark the resource as logically deleted if the TF call clears the state
n.opTracker.SetDeleted(newStateAfterApplyVal.IsNull())

return nil
return managed.ExternalDelete{}, nil
}

func (n *terraformPluginFrameworkExternalClient) setExternalName(mg xpresource.Managed, stateValueMap map[string]interface{}) (bool, error) {
Expand Down Expand Up @@ -713,3 +713,7 @@ func protov5DynamicValueFromMap(data map[string]any, terraformType tftypes.Type)

return &dynamicValue, nil
}

func (n *terraformPluginFrameworkExternalClient) Disconnect(_ context.Context) error {
return nil
}
2 changes: 1 addition & 1 deletion pkg/controller/external_tfpluginfw_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ func TestTPFDelete(t *testing.T) {
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
tpfExternal := prepareTPFExternalWithTestConfig(tc.testConfiguration)
err := tpfExternal.Delete(context.TODO(), &tc.testConfiguration.obj)
_, err := tpfExternal.Delete(context.TODO(), &tc.testConfiguration.obj)
if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
t.Errorf("\n%s\nConnect(...): -want error, +got error:\n", diff)
}
Expand Down
8 changes: 6 additions & 2 deletions pkg/controller/external_tfpluginsdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ func (n *terraformPluginSDKExternal) Update(ctx context.Context, mg xpresource.M
return managed.ExternalUpdate{}, nil
}

func (n *terraformPluginSDKExternal) Delete(ctx context.Context, _ xpresource.Managed) error {
func (n *terraformPluginSDKExternal) Delete(ctx context.Context, _ xpresource.Managed) (managed.ExternalDelete, error) {
n.logger.Debug("Deleting the external resource")
if n.instanceDiff == nil {
n.instanceDiff = tf.NewInstanceDiff()
Expand All @@ -714,11 +714,15 @@ func (n *terraformPluginSDKExternal) Delete(ctx context.Context, _ xpresource.Ma
newState, diag := n.resourceSchema.Apply(ctx, n.opTracker.GetTfState(), n.instanceDiff, n.ts.Meta)
metrics.ExternalAPITime.WithLabelValues("delete").Observe(time.Since(start).Seconds())
if diag != nil && diag.HasError() {
return errors.Errorf("failed to delete the resource: %v", diag)
return managed.ExternalDelete{}, errors.Errorf("failed to delete the resource: %v", diag)
}
n.opTracker.SetTfState(newState)
// mark the resource as logically deleted if the TF call clears the state
n.opTracker.SetDeleted(newState == nil)
return managed.ExternalDelete{}, nil
}

func (n *terraformPluginSDKExternal) Disconnect(_ context.Context) error {
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/external_tfpluginsdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ func TestTerraformPluginSDKDelete(t *testing.T) {
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
terraformPluginSDKExternal := prepareTerraformPluginSDKExternal(tc.args.r, tc.args.cfg)
err := terraformPluginSDKExternal.Delete(context.TODO(), &tc.args.obj)
_, err := terraformPluginSDKExternal.Delete(context.TODO(), &tc.args.obj)
if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
t.Errorf("\n%s\nConnect(...): -want error, +got error:\n", diff)
}
Expand Down

0 comments on commit b6820b0

Please sign in to comment.