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.

Sign up for GitHub

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

feat: skip destroy for schematics resources #703

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+61 −33
Open

feat: skip destroy for schematics resources #703

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d01de0b
feat: skip destroy for schematics resources
Aashiq-J Nov 27, 2023
e9ea313
Merge branch 'main' into skip-destroy
Aashiq-J Nov 27, 2023
827f691
Merge branch 'main' of https://github.com/terraform-ibm-modules/ibmcl…
Aashiq-J Nov 28, 2023
014f158
fix: few review changes
Aashiq-J Nov 28, 2023
49eb4f1
fix: remove few empty spaces
Aashiq-J Nov 28, 2023
b393cda
Merge branch 'main' into skip-destroy
Aashiq-J Nov 28, 2023
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
4 changes: 3 additions & 1 deletion testschematic/example_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package testschematic_test
import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/terraform-ibm-modules/ibmcloud-terratest-wrapper/testschematic"
)

Expand Down Expand Up @@ -46,9 +47,10 @@ func Example_default() {
options.AddNetrcCredential("bitbucket.com", "bit-user", "bit-token")

// run the test
err := options.RunSchematicTest()
output, err := options.RunSchematicTest()
if err != nil {
t.Fail()
}
assert.NotNil(t, output, "Expected some output")
})
}
5 changes: 5 additions & 0 deletions testschematic/test_options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ type TestSchematicOptions struct {
// This array will be used to construct a valid `Variablestore` configuration for the Schematics Workspace Template
TerraformVars []TestSchematicTerraformVar

// If you want to skip teardown use these
SkipTestTearDown bool

// This value will set the `folder` attribute in the Schematics template, and will be used as the execution folder for terraform.
// Defaults to root directory of source, "." if not supplied.
//
Expand Down Expand Up @@ -189,6 +192,8 @@ func TestSchematicOptionsDefault(originalOptions *TestSchematicOptions) *TestSch
newOptions.SchematicsApiURL = DefaultSchematicsApiURL
}

newOptions.SkipTestTearDown = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should only default to false if the value is not set in originalOptions (if it is set to true).

In golang, the default for a bool is already false, so setting this here is probably optional.


return newOptions

}
Expand Down
107 changes: 61 additions & 46 deletions testschematic/tests.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,19 @@ import (
// 3. configure supplied test variables in workspace
// 4. run PLAN/APPLY/DESTROY steps on workspace to provision and destroy resources
// 5. delete the test workspace
func (options *TestSchematicOptions) RunSchematicTest() error {

// create new schematic service with authenticator
var svc *SchematicsTestService

func (options *TestSchematicOptions) RunSchematicTest() (*SchematicsTestService, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this change (creating package global for the service pointer and returning it from the Run function) is necessary. The original design was to create a new service object, only if not initially provided in options struct, and then store that pointer in the options struct to be used later if needed.

The change that is likely needed here would be to make options.schematicsTestSvc a public struct attribute by capatalizing the name: options.SchematicsTestSvc. By making that attribute public you can then reference it inside this package AND outside in the terraform unit test, and avoid having a package global containing a pointer (which is sometimes avoided due to complexity).

I would remove the global svc, remove the return value, and just change the options attribute to public (simply capatalize the attribute name).


// WORKSPACE SETUP
// In this section of the test we are setting up the workspace.
// Any errors in this section will be considerd "unexpected" and returned to the calling unit test
// to short-circuit and quit the test.
// The official start of the unit test, with assertions, will begin AFTER workspace is properly created.

// create new schematic service with authenticator, set pointer of service in options for use later
var svc *SchematicsTestService
// set pointer of service in options for use later
if options.schematicsTestSvc == nil {
svc = &SchematicsTestService{}
} else {
Expand All @@ -47,59 +50,59 @@ func (options *TestSchematicOptions) RunSchematicTest() error {
} else {
svcErr := svc.InitializeSchematicsService()
if svcErr != nil {
return fmt.Errorf("error creating schematics sdk service: %w", svcErr)
return nil, fmt.Errorf("error creating schematics sdk service: %w", svcErr)
}
}

// get the root path of this project
projectPath, pathErr := common.GitRootPath(".")
if pathErr != nil {
return fmt.Errorf("error getting root path of git project: %w", pathErr)
return nil, fmt.Errorf("error getting root path of git project: %w", pathErr)
}

// create a new tar file for the project
options.Testing.Log("[SCHEMATICS] Creating TAR file")
tarballName, tarballErr := CreateSchematicTar(projectPath, &options.TarIncludePatterns)
if tarballErr != nil {
return fmt.Errorf("error creating tar file: %w", tarballErr)
return nil, fmt.Errorf("error creating tar file: %w", tarballErr)
}
defer os.Remove(tarballName) // just to cleanup

// create a new empty workspace, resulting in "draft" status
options.Testing.Log("[SCHEMATICS] Creating Test Workspace")
_, wsErr := svc.CreateTestWorkspace(options.Prefix, options.ResourceGroup, options.TemplateFolder, options.TerraformVersion, options.Tags)
if wsErr != nil {
return fmt.Errorf("error creating new schematic workspace: %w", wsErr)
return nil, fmt.Errorf("error creating new schematic workspace: %w", wsErr)
}

options.Testing.Logf("[SCHEMATICS] Workspace Created: %s (%s)", svc.WorkspaceName, svc.WorkspaceID)
// can be used in error messages to repeat workspace name
workspaceNameString := fmt.Sprintf("[ %s (%s) ]", svc.WorkspaceName, svc.WorkspaceID)

// since workspace is now created, always call the teardown to remove
defer testTearDown(svc, options)
defer testTearDown(options)

// upload the terraform code
options.Testing.Log("[SCHEMATICS] Uploading TAR file")
uploadErr := svc.UploadTarToWorkspace(tarballName)
if uploadErr != nil {
return fmt.Errorf("error uploading tar file to workspace: %w - %s", uploadErr, workspaceNameString)
return nil, fmt.Errorf("error uploading tar file to workspace: %w - %s", uploadErr, workspaceNameString)
}

// -------- UPLOAD TAR FILE ----------
// find the tar upload job
uploadJob, uploadJobErr := svc.FindLatestWorkspaceJobByName(SchematicsJobTypeUpload)
if uploadJobErr != nil {
return fmt.Errorf("error finding the upload tar action: %w - %s", uploadJobErr, workspaceNameString)
return nil, fmt.Errorf("error finding the upload tar action: %w - %s", uploadJobErr, workspaceNameString)
}
// wait for it to finish
uploadJobStatus, uploadJobStatusErr := svc.WaitForFinalJobStatus(*uploadJob.ActionID)
if uploadJobStatusErr != nil {
return fmt.Errorf("error waiting for upload of tar to finish: %w - %s", uploadJobStatusErr, workspaceNameString)
return nil, fmt.Errorf("error waiting for upload of tar to finish: %w - %s", uploadJobStatusErr, workspaceNameString)
}
// check if complete
if uploadJobStatus != SchematicsJobStatusCompleted {
return fmt.Errorf("tar upload has failed with status %s - %s", uploadJobStatus, workspaceNameString)
return nil, fmt.Errorf("tar upload has failed with status %s - %s", uploadJobStatus, workspaceNameString)
}

// ------ FINAL WORKSPACE CONFIG ------
Expand All @@ -109,7 +112,7 @@ func (options *TestSchematicOptions) RunSchematicTest() error {
options.Testing.Log("[SCHEMATICS] Updating Workspace Variablestore")
updateErr := svc.UpdateTestTemplateVars(options.TerraformVars)
if updateErr != nil {
return fmt.Errorf("error updating template with Variablestore: %w - %s", updateErr, workspaceNameString)
return nil, fmt.Errorf("error updating template with Variablestore: %w - %s", updateErr, workspaceNameString)
}

// TERRAFORM TESTING BEGINS
Expand Down Expand Up @@ -143,45 +146,57 @@ func (options *TestSchematicOptions) RunSchematicTest() error {
}
}

// ------ DESTROY ------
// only run destroy if we had potentially created resources
if svc.TerraformResourcesCreated {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are moving the resource destroy from this block, you need to still replace it with your new TestTearDown() or similar.

Keep in mind thought that in this test case there are TWO different destroy that take place:

  1. The destroy of the applied resources
  2. The delete of the workspace

The block removed here is covering item 1, the destroy of the applied resources, NOT the deletion of the workspace.

Perhaps a short meeting or slack conversation can explain this better.

// Check if "DO_NOT_DESTROY_ON_FAILURE" is set
envVal, _ := os.LookupEnv("DO_NOT_DESTROY_ON_FAILURE")
if options.Testing.Failed() && strings.ToLower(envVal) == "true" {
options.Testing.Log("[SCHEMATICS] Schematics APPLY failed. Debug the Test and delete resources manually.")
} else {
destroyResponse, destroyErr := svc.CreateDestroyJob()
if assert.NoErrorf(options.Testing, destroyErr, "error creating DESTROY - %s", workspaceNameString) {
options.Testing.Log("[SCHEMATICS] Starting DESTROY job ...")
destroyJobStatus, destroyStatusErr := svc.WaitForFinalJobStatus(*destroyResponse.Activityid)
if assert.NoErrorf(options.Testing, destroyStatusErr, "error waiting for DESTROY to finish - %s", workspaceNameString) {
assert.Equalf(options.Testing, SchematicsJobStatusCompleted, destroyJobStatus, "DESTROY has failed with status %s - %s", destroyJobStatus, workspaceNameString)
}
}
}
}
return svc, nil
}

return nil
// Function to destroy all resources. Resources are not destroyed if tests failed and "DO_NOT_DESTROY_ON_FAILURE" environment variable is true.
func (options *TestSchematicOptions) TestTearDown() {
options.SkipTestTearDown = false
defer testTearDown(options)
}

// testTearDown is a helper function, typically called via golang "defer", that will clean up and remove any existing resources that were
// created for the test.
// The removal of some resources may be influenced by certain conditions or optional settings.
func testTearDown(svc *SchematicsTestService, options *TestSchematicOptions) {
// ------ DELETE WORKSPACE ------
// only delete workspace if one of these is true:
// * terraform hasn't been started yet
// * no failures
// * failed and DeleteWorkspaceOnFail is true
if !svc.TerraformTestStarted ||
!options.Testing.Failed() ||
(options.Testing.Failed() && options.DeleteWorkspaceOnFail) {

options.Testing.Log("[SCHEMATICS] Deleting Workspace")
_, deleteWsErr := svc.DeleteWorkspace()
if deleteWsErr != nil {
options.Testing.Logf("[SCHEMATICS] WARNING: Schematics WORKSPACE DELETE failed! Remove manually if required. Name: %s (%s)", svc.WorkspaceName, svc.WorkspaceID)
func testTearDown(options *TestSchematicOptions) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old testTearDown ONLY covered the deletion of the workspace, NOT the destroy of the applied resources.

You are replacing it and combining the destruction of applied resources AND the delete of the workspace. This may alter how this works, as you still need to call the destroy of resources in RunSchematicsTest if the skip is not set, and keep seperate the destruction of the workspace (which was done on purpose to leave workspace around on failed test due to logs only existing there).

This might have to be split into two functions. Perhaps further explanation can be done on a schematics thread or short meeting.


if !options.SkipTestTearDown {
workspaceNameString := fmt.Sprintf("[ %s (%s) ]", svc.WorkspaceName, svc.WorkspaceID)
// ------ DESTROY ------
// only run destroy if we had potentially created resources
if svc.TerraformResourcesCreated {
// Check if "DO_NOT_DESTROY_ON_FAILURE" is set
envVal, _ := os.LookupEnv("DO_NOT_DESTROY_ON_FAILURE")
if options.Testing.Failed() && strings.ToLower(envVal) == "true" {
options.Testing.Log("[SCHEMATICS] Schematics APPLY failed. Debug the Test and delete resources manually.")
} else {
destroyResponse, destroyErr := svc.CreateDestroyJob()
if assert.NoErrorf(options.Testing, destroyErr, "error creating DESTROY - %s", workspaceNameString) {
options.Testing.Log("[SCHEMATICS] Starting DESTROY job ...")
destroyJobStatus, destroyStatusErr := svc.WaitForFinalJobStatus(*destroyResponse.Activityid)
if assert.NoErrorf(options.Testing, destroyStatusErr, "error waiting for DESTROY to finish - %s", workspaceNameString) {
assert.Equalf(options.Testing, SchematicsJobStatusCompleted, destroyJobStatus, "DESTROY has failed with status %s - %s", destroyJobStatus, workspaceNameString)
}
}
}
}

// ------ DELETE WORKSPACE ------
// only delete workspace if one of these is true:
// * terraform hasn't been started yet
// * no failures
// * failed and DeleteWorkspaceOnFail is true
if !svc.TerraformTestStarted ||
!options.Testing.Failed() ||
(options.Testing.Failed() && options.DeleteWorkspaceOnFail) {

options.Testing.Log("[SCHEMATICS] Deleting Workspace")
_, deleteWsErr := svc.DeleteWorkspace()
if deleteWsErr != nil {
options.Testing.Logf("[SCHEMATICS] WARNING: Schematics WORKSPACE DELETE failed! Remove manually if required. Name: %s (%s)", svc.WorkspaceName, svc.WorkspaceID)
}
}
} else {
options.Testing.Log("Skipping automatic Test Teardown")
}
}
3 changes: 2 additions & 1 deletion testschematic/tests_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ func TestSchematicFullTest(t *testing.T) {
}

t.Run("CleanRun", func(t *testing.T) {
err := options.RunSchematicTest()
output, err := options.RunSchematicTest()
assert.NoError(t, err)
assert.NotNil(t, output, "Expected some output")
assert.True(t, schematicSvc.applyComplete)
assert.True(t, schematicSvc.destroyComplete)
assert.True(t, schematicSvc.workspaceDeleteComplete)
Expand Down