Skip to content

Commit

Permalink
Initialize tfexec.Terraform once per request
Browse files Browse the repository at this point in the history
Signed-off-by: Karishma Chawla <kachawla@microsoft.com>
  • Loading branch information
kachawla committed Nov 2, 2023
1 parent a330410 commit 0248a0d
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 74 deletions.
18 changes: 9 additions & 9 deletions pkg/cli/cmd/recipe/show/show_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
"github.com/radius-project/radius/pkg/cli/output"
"github.com/radius-project/radius/pkg/cli/workspaces"
"github.com/radius-project/radius/pkg/corerp/api/v20231001preview"
datastorerp "github.com/radius-project/radius/pkg/datastoresrp/frontend/controller"
datastoresrp "github.com/radius-project/radius/pkg/datastoresrp/frontend/controller"
"github.com/radius-project/radius/pkg/recipes"
"github.com/radius-project/radius/pkg/to"
"github.com/radius-project/radius/test/radcli"
Expand All @@ -46,7 +46,7 @@ func Test_Validate(t *testing.T) {
testcases := []radcli.ValidateInput{
{
Name: "Valid Show Command",
Input: []string{"recipeName", "--resource-type", datastorerp.RedisCachesResourceType},
Input: []string{"recipeName", "--resource-type", datastoresrp.RedisCachesResourceType},
ExpectedValid: true,
ConfigHolder: framework.ConfigHolder{
ConfigFilePath: "",
Expand All @@ -55,7 +55,7 @@ func Test_Validate(t *testing.T) {
},
{
Name: "Show Command with incorrect fallback workspace",
Input: []string{"-e", "my-env", "-g", "my-env", "recipeName", "--resource-type", datastorerp.RedisCachesResourceType},
Input: []string{"-e", "my-env", "-g", "my-env", "recipeName", "--resource-type", datastoresrp.RedisCachesResourceType},
ExpectedValid: false,
ConfigHolder: framework.ConfigHolder{
ConfigFilePath: "",
Expand All @@ -64,7 +64,7 @@ func Test_Validate(t *testing.T) {
},
{
Name: "Show Command with too many positional args",
Input: []string{"recipeName", "arg2", "--resource-type", datastorerp.RedisCachesResourceType},
Input: []string{"recipeName", "arg2", "--resource-type", datastoresrp.RedisCachesResourceType},
ExpectedValid: false,
ConfigHolder: framework.ConfigHolder{
ConfigFilePath: "",
Expand All @@ -73,7 +73,7 @@ func Test_Validate(t *testing.T) {
},
{
Name: "Show Command with fallback workspace",
Input: []string{"-e", "my-env", "-w", "test-workspace", "recipeName", "--resource-type", datastorerp.RedisCachesResourceType},
Input: []string{"-e", "my-env", "-w", "test-workspace", "recipeName", "--resource-type", datastoresrp.RedisCachesResourceType},
ExpectedValid: true,
ConfigHolder: framework.ConfigHolder{
ConfigFilePath: "",
Expand Down Expand Up @@ -111,7 +111,7 @@ func Test_Run(t *testing.T) {
}
recipe := types.EnvironmentRecipe{
Name: "cosmosDB",
ResourceType: datastorerp.MongoDatabasesResourceType,
ResourceType: datastoresrp.MongoDatabasesResourceType,
TemplateKind: recipes.TemplateKindBicep,
TemplatePath: "ghcr.io/testpublicrecipe/bicep/modules/mongodatabases:v1",
}
Expand Down Expand Up @@ -145,7 +145,7 @@ func Test_Run(t *testing.T) {
Workspace: &workspaces.Workspace{},
Format: "table",
RecipeName: "cosmosDB",
ResourceType: datastorerp.MongoDatabasesResourceType,
ResourceType: datastoresrp.MongoDatabasesResourceType,
}

err := runner.Run(context.Background())
Expand Down Expand Up @@ -187,7 +187,7 @@ func Test_Run(t *testing.T) {
}
recipe := types.EnvironmentRecipe{
Name: "cosmosDB",
ResourceType: datastorerp.MongoDatabasesResourceType,
ResourceType: datastoresrp.MongoDatabasesResourceType,
TemplateKind: recipes.TemplateKindTerraform,
TemplatePath: "Azure/cosmosdb/azurerm",
TemplateVersion: "1.1.0",
Expand Down Expand Up @@ -222,7 +222,7 @@ func Test_Run(t *testing.T) {
Workspace: &workspaces.Workspace{},
Format: "table",
RecipeName: "cosmosDB",
ResourceType: datastorerp.MongoDatabasesResourceType,
ResourceType: datastoresrp.MongoDatabasesResourceType,
}

err := runner.Run(context.Background())
Expand Down
55 changes: 33 additions & 22 deletions pkg/recipes/terraform/execute.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"time"

install "github.com/hashicorp/hc-install"
"github.com/hashicorp/terraform-exec/tfexec"
tfjson "github.com/hashicorp/terraform-json"
"github.com/radius-project/radius/pkg/metrics"
"github.com/radius-project/radius/pkg/recipes"
Expand Down Expand Up @@ -95,14 +96,20 @@ func (e *executor) Deploy(ctx context.Context, options Options) (*tfjson.State,
return nil, err
}

// Create a new instance of tfexec.Terraform with current the Terraform installation path
tf, err := NewTerraform(ctx, workingDir, execPath)
if err != nil {
return nil, err
}

// Create Terraform config in the working directory
kubernetesBackendSuffix, err := e.generateConfig(ctx, workingDir, execPath, options)
kubernetesBackendSuffix, err := e.generateConfig(ctx, tf, workingDir, options)
if err != nil {
return nil, err
}

// Run TF Init and Apply in the working directory
state, err := initAndApply(ctx, workingDir, execPath)
state, err := initAndApply(ctx, tf, workingDir)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -144,8 +151,14 @@ func (e *executor) Delete(ctx context.Context, options Options) error {
return err
}

// Create a new instance of tfexec.Terraform with current the Terraform installation path
tf, err := NewTerraform(ctx, workingDir, execPath)
if err != nil {
return err
}

// Create Terraform config in the working directory
kubernetesBackendSuffix, err := e.generateConfig(ctx, workingDir, execPath, options)
kubernetesBackendSuffix, err := e.generateConfig(ctx, tf, workingDir, options)
if err != nil {
return err
}
Expand All @@ -165,7 +178,7 @@ func (e *executor) Delete(ctx context.Context, options Options) error {
}

// Run TF Destroy in the working directory to delete the resources deployed by the recipe
err = initAndDestroy(ctx, workingDir, execPath)
err = initAndDestroy(ctx, tf)
if err != nil {
return err
}
Expand Down Expand Up @@ -204,12 +217,18 @@ func (e *executor) GetRecipeMetadata(ctx context.Context, options Options) (map[
return nil, err
}

// Create a new instance of tfexec.Terraform with current the Terraform installation path
tf, err := NewTerraform(ctx, workingDir, execPath)
if err != nil {
return nil, err
}

_, err = getTerraformConfig(ctx, workingDir, options)
if err != nil {
return nil, err
}

result, err := downloadAndInspect(ctx, workingDir, execPath, options)
result, err := downloadAndInspect(ctx, tf, workingDir, options)
if err != nil {
return nil, err
}
Expand All @@ -232,15 +251,15 @@ func createWorkingDir(ctx context.Context, tfDir string) (string, error) {
}

// generateConfig generates Terraform configuration with required inputs for the module, providers and backend to be initialized and applied.
func (e *executor) generateConfig(ctx context.Context, workingDir, execPath string, options Options) (string, error) {
func (e *executor) generateConfig(ctx context.Context, tf *tfexec.Terraform, workingDir string, options Options) (string, error) {
logger := ucplog.FromContextOrDiscard(ctx)

tfConfig, err := getTerraformConfig(ctx, workingDir, options)
if err != nil {
return "", err
}

loadedModule, err := downloadAndInspect(ctx, workingDir, execPath, options)
loadedModule, err := downloadAndInspect(ctx, tf, workingDir, options)
if err != nil {
return "", err
}
Expand Down Expand Up @@ -298,13 +317,13 @@ func (e *executor) generateConfig(ctx context.Context, workingDir, execPath stri
}

// downloadAndInspect handles downloading the TF module and retrieving the necessary information
func downloadAndInspect(ctx context.Context, workingDir string, execPath string, options Options) (*moduleInspectResult, error) {
func downloadAndInspect(ctx context.Context, tf *tfexec.Terraform, workingDir string, options Options) (*moduleInspectResult, error) {
logger := ucplog.FromContextOrDiscard(ctx)

// Download the Terraform module to the working directory.
logger.Info(fmt.Sprintf("Downloading Terraform module: %s", options.EnvRecipe.TemplatePath))
downloadStartTime := time.Now()
if err := downloadModule(ctx, workingDir, execPath, options.EnvRecipe.TemplatePath); err != nil {
if err := downloadModule(ctx, tf, options.EnvRecipe.TemplatePath); err != nil {
metrics.DefaultRecipeEngineMetrics.RecordRecipeDownloadDuration(ctx, downloadStartTime,
metrics.NewRecipeAttributes(metrics.RecipeEngineOperationDownloadRecipe, options.EnvRecipe.Name,
options.EnvRecipe, recipes.RecipeDownloadFailed))
Expand Down Expand Up @@ -350,16 +369,11 @@ func getTerraformConfig(ctx context.Context, workingDir string, options Options)
}

// initAndApply runs Terraform init and apply in the provided working directory.
func initAndApply(ctx context.Context, workingDir, execPath string) (*tfjson.State, error) {
func initAndApply(ctx context.Context, tf *tfexec.Terraform, workingDir string) (*tfjson.State, error) {
logger := ucplog.FromContextOrDiscard(ctx)

tf, err := NewTerraform(ctx, workingDir, execPath)
if err != nil {
return nil, err
}
// Initialize Terraform
logger.Info("Initializing Terraform")

terraformInitStartTime := time.Now()
if err := tf.Init(ctx); err != nil {
metrics.DefaultRecipeEngineMetrics.RecordTerraformInitializationDuration(ctx, terraformInitStartTime,
Expand All @@ -382,19 +396,16 @@ func initAndApply(ctx context.Context, workingDir, execPath string) (*tfjson.Sta
}

// initAndDestroy runs Terraform init and destroy in the provided working directory.
func initAndDestroy(ctx context.Context, workingDir, execPath string) error {
func initAndDestroy(ctx context.Context, tf *tfexec.Terraform) error {
logger := ucplog.FromContextOrDiscard(ctx)

tf, err := NewTerraform(ctx, workingDir, execPath)
if err != nil {
return err
}

// Initialize Terraform
logger.Info("Initializing Terraform")

terraformInitStartTime := time.Now()
if err := tf.Init(ctx); err != nil {
metrics.DefaultRecipeEngineMetrics.RecordTerraformInitializationDuration(ctx, terraformInitStartTime,
[]attribute.KeyValue{metrics.OperationStateAttrKey.String(metrics.FailedOperationState)})

return fmt.Errorf("terraform init failure: %w", err)
}
metrics.DefaultRecipeEngineMetrics.RecordTerraformInitializationDuration(ctx, terraformInitStartTime, nil)
Expand Down
30 changes: 6 additions & 24 deletions pkg/recipes/terraform/execute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"path/filepath"
"testing"

"github.com/hashicorp/terraform-exec/tfexec"
"github.com/radius-project/radius/pkg/recipes"
"github.com/radius-project/radius/pkg/recipes/terraform/config"
"github.com/radius-project/radius/test/testcontext"
Expand Down Expand Up @@ -57,25 +58,15 @@ func TestCreateWorkingDir_Error(t *testing.T) {
require.Contains(t, err.Error(), "failed to create working directory")
}

func TestInitAndApply_EmptyWorkingDirPath(t *testing.T) {
// Create a temporary directory for testing.
testDir := t.TempDir()
execPath := filepath.Join(testDir, "terraform")

_, err := initAndApply(testcontext.New(t), "", execPath)
require.Error(t, err)
require.Contains(t, err.Error(), "Terraform cannot be initialised with empty workdir")
}

func TestGeneratedConfig(t *testing.T) {
func TestGenerateConfig(t *testing.T) {
configTests := []struct {
name string
workingDir string
opts Options
err string
}{
{
name: "empty recipe name",
name: "empty recipe name error",
opts: Options{
EnvRecipe: &recipes.EnvironmentDefinition{
TemplatePath: "test/module/source",
Expand All @@ -93,16 +84,6 @@ func TestGeneratedConfig(t *testing.T) {
ResourceRecipe: &recipes.ResourceMetadata{},
},
err: "error creating file: open /invalid-dir/main.tf.json",
}, {
name: "invalid exec path",
opts: Options{
EnvRecipe: &recipes.EnvironmentDefinition{
Name: "test-recipe",
TemplatePath: "test/module/source",
},
ResourceRecipe: &recipes.ResourceMetadata{},
},
err: "/terraform: no such file or directory",
},
}

Expand All @@ -112,9 +93,10 @@ func TestGeneratedConfig(t *testing.T) {
if tc.workingDir == "" {
tc.workingDir = t.TempDir()
}
execPath := filepath.Join(tc.workingDir, "terraform")
tf, _ := tfexec.NewTerraform(tc.workingDir, filepath.Join(tc.workingDir, "terraform"))

e := executor{}
_, err := e.generateConfig(ctx, tc.workingDir, execPath, tc.opts)
_, err := e.generateConfig(ctx, tf, tc.workingDir, tc.opts)
require.Error(t, err)
require.ErrorContains(t, err, tc.err)
})
Expand Down
10 changes: 3 additions & 7 deletions pkg/recipes/terraform/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"path/filepath"

"github.com/hashicorp/terraform-config-inspect/tfconfig"
"github.com/hashicorp/terraform-exec/tfexec"
"github.com/radius-project/radius/pkg/recipes"
"github.com/radius-project/radius/pkg/recipes/recipecontext"
)
Expand Down Expand Up @@ -98,13 +99,8 @@ func inspectModule(workingDir, localModuleName string) (*moduleInspectResult, er
// downloadModule downloads the module to the workingDir from the module source specified in the Terraform configuration.
// It uses Terraform's Get command to download the module using the Terraform executable available at execPath.
// An error is returned if the module could not be downloaded.
func downloadModule(ctx context.Context, workingDir, execPath, templatePath string) error {
tf, err := NewTerraform(ctx, workingDir, execPath)
if err != nil {
return err
}

if err = tf.Get(ctx); err != nil {
func downloadModule(ctx context.Context, tf *tfexec.Terraform, templatePath string) error {
if err := tf.Get(ctx); err != nil {
return fmt.Errorf("failed to run terraform get to download the module from source %q: %w", templatePath, err)
}

Expand Down
12 changes: 0 additions & 12 deletions pkg/recipes/terraform/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@ limitations under the License.
package terraform

import (
"path/filepath"
"testing"

"github.com/hashicorp/terraform-config-inspect/tfconfig"
"github.com/radius-project/radius/test/testcontext"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -89,13 +87,3 @@ func Test_InspectTFModuleConfig(t *testing.T) {
})
}
}

func Test_DownloadModule_EmptyWorkingDirPath_Error(t *testing.T) {
// Create a temporary test directory.
testDir := t.TempDir()
execPath := filepath.Join(testDir, "terraform")

err := downloadModule(testcontext.New(t), "", execPath, "test/module/source")
require.Error(t, err)
require.Contains(t, err.Error(), "Terraform cannot be initialised with empty workdir")
}
Loading

0 comments on commit 0248a0d

Please sign in to comment.