Improve workdir cleanup logic #195
Conversation
| @@ -656,7 +672,7 @@ func (sess *reconcileStackSession) SetupPulumiWorkdir(gitAuth *auto.GitAuth) err | |||
| return errors.Wrap(err, "installing project dependencies") | |||
| } | |||
|
|
|||
| return nil | |||
| return err | |||
There was a problem hiding this comment.
I can't quite grasp this change...
There was a problem hiding this comment.
I can't quite grasp this change...
Its actually unnecessary. Removed.
| }() | ||
|
|
||
| var w auto.Workspace | ||
| w, err = auto.NewLocalWorkspace(context.Background(), auto.WorkDir(dir), auto.Repo(repo), secretsProvider) |
There was a problem hiding this comment.
What's the default behavior of our automation API before this change? Does it create a temporary folder internally but does not clean it up correctly? What is our advice to 3rd party users of automation API?
There was a problem hiding this comment.
What's the default behavior of our automation API before this change? Does it create a temporary folder internally but does not clean it up correctly? What is our advice to 3rd party users of automation API?
Yes it creates a temporary directory internally. We tried to handle cleanup through the sess.CleanupPulumiWorkdir() call elsewhere but we do many operations as part of the stack setup which may fail independently and we leak the work dir on failure.
This seems to be more a function of the way the operator is currently written than anything specific to the automation API.
| @@ -633,7 +648,8 @@ func (sess *reconcileStackSession) SetupPulumiWorkdir(gitAuth *auto.GitAuth) err | |||
| sess.autoStack = &a | |||
| sess.logger.Debug("Setting autostack", "autostack", sess.autoStack) | |||
|
|
|||
| c, err := sess.autoStack.GetAllConfig(ctx) | |||
| var c auto.ConfigMap | |||
| c, err = sess.autoStack.GetAllConfig(ctx) | |||
Fixes #104.
Manually verified that the work directory is deleted on failure while retaining existing behavior on success.