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

Initialize tfexec.Terraform once per request + refactoring/cleanup #6630

Merged
merged 3 commits into from
Nov 3, 2023
Merged

Initialize tfexec.Terraform once per request + refactoring/cleanup #6630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0248a0d
Initialize tfexec.Terraform once per request
kachawla Oct 30, 2023
2dde25c
Move working directory creation to newTerraform
kachawla Nov 2, 2023
f7666c2
Merge branch 'main' into kachawla/terraform-improvements
kachawla Nov 3, 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
18 changes: 9 additions & 9 deletions pkg/cli/cmd/recipe/show/show_test.go
View file
Open in desktop
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
74 changes: 24 additions & 50 deletions pkg/recipes/terraform/execute.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,10 @@ import (
"context"
"errors"
"fmt"
"io/fs"
"os"
"path/filepath"
"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 All @@ -42,11 +40,6 @@ import (
"k8s.io/client-go/kubernetes"
)

const (
executionSubDir = "deploy"
workingDirFileMode fs.FileMode = 0700
)

var (
// ErrRecipeNameEmpty is the error when the recipe name is empty.
ErrRecipeNameEmpty = errors.New("recipe name cannot be empty")
Expand Down Expand Up @@ -89,20 +82,20 @@ func (e *executor) Deploy(ctx context.Context, options Options) (*tfjson.State,
return nil, err
}

// Create Working Directory
workingDir, err := createWorkingDir(ctx, options.RootDir)
// Create a new instance of tfexec.Terraform with current Terraform installation path
tf, err := NewTerraform(ctx, options.RootDir, execPath)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could working in the Terraform installation path cause clashes with other files that will take place in the same directory?

Before we were creating a working directory (workingDir) in the root directory, now we will not create a working directory but will work in the execution path (execPath) -- the working directory and execution path will be the same. Would that cause any problems?

Copy link
Contributor Author

@kachawla kachawla Nov 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We are still creating the working directory inside the root directory

radius/pkg/recipes/terraform/types.go

Line 67 in 2dde25c

workingDir, err := createWorkingDir(ctx, tfRootDir)
, this PR is just moving creation of working directory closer to the initialization of terraform-exec. Did you mean something else?

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, 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)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -138,14 +131,14 @@ func (e *executor) Delete(ctx context.Context, options Options) error {
return err
}

// Create Working Directory
workingDir, err := createWorkingDir(ctx, options.RootDir)
// Create a new instance of tfexec.Terraform with current Terraform installation path
tf, err := NewTerraform(ctx, options.RootDir, 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, options)
if err != nil {
return err
}
Expand All @@ -165,7 +158,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 @@ -198,18 +191,18 @@ func (e *executor) GetRecipeMetadata(ctx context.Context, options Options) (map[
return nil, err
}

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

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

result, err := downloadAndInspect(ctx, workingDir, execPath, options)
result, err := downloadAndInspect(ctx, tf, options)
if err != nil {
return nil, err
}
Expand All @@ -219,28 +212,17 @@ func (e *executor) GetRecipeMetadata(ctx context.Context, options Options) (map[
}, nil
}

func createWorkingDir(ctx context.Context, tfDir string) (string, error) {
logger := ucplog.FromContextOrDiscard(ctx)

workingDir := filepath.Join(tfDir, executionSubDir)
logger.Info(fmt.Sprintf("Creating Terraform working directory: %q", workingDir))
if err := os.MkdirAll(workingDir, workingDirFileMode); err != nil {
return "", fmt.Errorf("failed to create working directory for terraform execution: %w", err)
}

return workingDir, nil
}

// 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, options Options) (string, error) {
logger := ucplog.FromContextOrDiscard(ctx)
workingDir := tf.WorkingDir()

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

loadedModule, err := downloadAndInspect(ctx, workingDir, execPath, options)
loadedModule, err := downloadAndInspect(ctx, tf, options)
if err != nil {
return "", err
}
Expand Down Expand Up @@ -298,13 +280,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, 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 All @@ -317,7 +299,7 @@ func downloadAndInspect(ctx context.Context, workingDir string, execPath string,
// Load the downloaded module to retrieve providers and variables required by the module.
// This is needed to add the appropriate providers config and populate the value of recipe context variable.
logger.Info(fmt.Sprintf("Inspecting the downloaded Terraform module: %s", options.EnvRecipe.TemplatePath))
loadedModule, err := inspectModule(workingDir, options.EnvRecipe.Name)
loadedModule, err := inspectModule(tf.WorkingDir(), options.EnvRecipe.Name)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -350,16 +332,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) (*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 +359,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
Loading
Loading