Skip to content

Implement cleanup of temporary files after execution #62

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

Merged
mumoshu merged 3 commits into from
Sep 5, 2021
Merged

Implement cleanup of temporary files after execution #62

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
628b4b4
Move temporary helmfile creation logic for both helmfile_release and …
Jul 19, 2021
8b70013
Added TmpHelfilePath field to ReleaseSet strucutre to store info abou…
Jul 19, 2021
23fdcc9
Reverting change making tmp helmfiles created in .terraform/helmfile,…
Jul 19, 2021
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
77 changes: 25 additions & 52 deletions pkg/helmfile/release_set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ import (
)

type ReleaseSet struct {
Bin string
Values []interface{}
ValuesFiles []interface{}
HelmBin string
Path string
Content string
DiffOutput string
ApplyOutput string
Environment string
Bin string
Values []interface{}
ValuesFiles []interface{}
HelmBin string
Content string
DiffOutput string
ApplyOutput string
Environment string
TmpHelmFilePath string

// Selector is a helmfile label selector that is a AND list of label key-value pairs
Selector map[string]interface{}
Expand Down Expand Up @@ -69,12 +69,6 @@ func NewReleaseSet(d ResourceRead) (*ReleaseSet, error) {
if env := d.Get(KeyEnvironment); env != nil {
f.Environment = env.(string)
}
// environment defaults to "" for helmfile_release_set but it's always nil for helmfile_release.
// This nil-check is required to handle the latter case. Otherwise it ends up with:
// panic: interface conversion: interface {} is nil, not string
if path := d.Get(KeyPath); path != nil {
f.Path = path.(string)
}

@mumoshu mumoshu Sep 5, 2021

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm going to merge this and make necessary changes(if any) later but one thing to ask here- What was the rationale behind removing this feature (specifying an existing path) in this pull request?

@dm3ch dm3ch Sep 6, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mumoshu When I started finding root cause of persisting tmp files, i found that there's two separate pieces of code creating tmp files for releases:

  1. Creation of helmfile for standalone helm release
  2. Copying existing helmfile before apply

This cases had completely different places where tmp files were created, so I refactored both cases to pass helmfile content same way and make same tmp file creation code for them.

When I sent this PR I tested that both terraform resources works fine with this PR, so my idea was unification of tmp file creation without breaking how terraform resources works


if content := d.Get(KeyContent); content != nil {
f.Content = content.(string)
Expand Down Expand Up @@ -121,20 +115,6 @@ func NewReleaseSet(d ResourceRead) (*ReleaseSet, error) {

logf("Printing raw working directory for %q: %s", d.Id(), f.WorkingDirectory)

if f.Path != "" {
if info, err := os.Stat(f.Path); err != nil {
if !os.IsNotExist(err) {
return nil, fmt.Errorf("verifying working_directory %q: %w", f.Path, err)
}
} else if info != nil && info.IsDir() {
f.WorkingDirectory = f.Path
} else {
f.WorkingDirectory = filepath.Dir(f.Path)
}
}

logf("Printing computed working directory for %q: %s", d.Id(), f.WorkingDirectory)

if environmentVariables := d.Get(KeyEnvironmentVariables); environmentVariables != nil {
f.EnvironmentVariables = environmentVariables.(map[string]interface{})
}
Expand All @@ -146,31 +126,22 @@ func NewReleaseSet(d ResourceRead) (*ReleaseSet, error) {
}

func NewCommandWithKubeconfig(fs *ReleaseSet, args ...string) (*exec.Cmd, error) {
if fs.Content != "" && fs.Path != "" && fs.Path != HelmfileDefaultPath {
return nil, fmt.Errorf("content and path can't be specified together: content=%q, path=%q", fs.Content, fs.Path)
}

if fs.WorkingDirectory != "" {
if err := os.MkdirAll(fs.WorkingDirectory, 0755); err != nil {
return nil, fmt.Errorf("creating working directory %q: %w", fs.WorkingDirectory, err)
}
}

var path string
if fs.Content != "" {
bs := []byte(fs.Content)
first := sha256.New()
first.Write(bs)
path = fmt.Sprintf("helmfile-%x.yaml", first.Sum(nil))
if err := ioutil.WriteFile(filepath.Join(fs.WorkingDirectory, path), bs, 0700); err != nil {
return nil, err
}
} else {
path = fs.Path
bs := []byte(fs.Content)
first := sha256.New()
first.Write(bs)
fs.TmpHelmFilePath = fmt.Sprintf("helmfile-%x.yaml", first.Sum(nil))
if err := ioutil.WriteFile(filepath.Join(fs.WorkingDirectory, fs.TmpHelmFilePath), bs, 0700); err != nil {
return nil, err
}

flags := []string{
"--file", path,
"--file", fs.TmpHelmFilePath,
"--no-color",
}

Expand Down Expand Up @@ -307,6 +278,8 @@ func CreateReleaseSet(ctx *sdk.Context, fs *ReleaseSet, d ResourceReadWrite) err
if err != nil {
return err
}
defer os.Remove(fs.TmpHelmFilePath)

//obtain exclusive lock
mutexKV.Lock(fs.WorkingDirectory)
defer mutexKV.Unlock(fs.WorkingDirectory)
Expand Down Expand Up @@ -372,6 +345,7 @@ func runBuild(ctx *sdk.Context, fs *ReleaseSet, flags ...string) (*State, error)
if err != nil {
return nil, err
}
defer os.Remove(fs.TmpHelmFilePath)

//obtain exclusive lock
mutexKV.Lock(fs.WorkingDirectory)
Expand All @@ -390,6 +364,7 @@ func getHelmfileVersion(ctx *sdk.Context, fs *ReleaseSet) (*semver.Version, erro
if err != nil {
return nil, fmt.Errorf("creating command: %w", err)
}
defer os.Remove(fs.TmpHelmFilePath)

//obtain exclusive lock
mutexKV.Lock(fs.WorkingDirectory)
Expand Down Expand Up @@ -423,6 +398,7 @@ func runTemplate(ctx *sdk.Context, fs *ReleaseSet) (*State, error) {
if err != nil {
return nil, err
}
defer os.Remove(fs.TmpHelmFilePath)

//obtain exclusive lock
mutexKV.Lock(fs.WorkingDirectory)
Expand Down Expand Up @@ -467,6 +443,7 @@ func runDiff(ctx *sdk.Context, fs *ReleaseSet, conf DiffConfig) (*State, error)
if err != nil {
return nil, err
}
defer os.Remove(fs.TmpHelmFilePath)

// Use the stable directory for storing temporary charts and values files
// so that helmfile-diff output becomes stables and terraform plan doesn't break.
Expand All @@ -493,6 +470,7 @@ func runDiff(ctx *sdk.Context, fs *ReleaseSet, conf DiffConfig) (*State, error)
if err := os.MkdirAll(abspath, 0755); err != nil {
return nil, xerrors.Errorf("creating temp directory for helmfile and chartify %s: %w", abspath, err)
}
defer os.Remove(abspath)

cmd.Env = append(cmd.Env, "HELMFILE_TEMPDIR="+abspath)
cmd.Env = append(cmd.Env, "CHARTIFY_TEMPDIR="+abspath)
Expand Down Expand Up @@ -648,13 +626,6 @@ func DiffReleaseSet(ctx *sdk.Context, fs *ReleaseSet, d ResourceReadWrite, opts
o(&diffConf)
}

if fs.Path != "" {
_, err := os.Stat(fs.Path)
if err != nil {
return "", fmt.Errorf("verifying path %q: %w", fs.Path, err)
}
}

diff, err := readDiffFile(ctx, fs)
if err != nil {
state, err := runDiff(ctx, fs, diffConf)
Expand Down Expand Up @@ -854,6 +825,7 @@ func UpdateReleaseSet(ctx *sdk.Context, fs *ReleaseSet, d ResourceReadWrite) err
if err != nil {
return err
}
defer os.Remove(fs.TmpHelmFilePath)

//obtain exclusive lock
mutexKV.Lock(fs.WorkingDirectory)
Expand All @@ -876,6 +848,7 @@ func DeleteReleaseSet(ctx *sdk.Context, fs *ReleaseSet, d ResourceReadWrite) err
if err != nil {
return err
}
defer os.Remove(fs.TmpHelmFilePath)

//obtain exclusive lock
mutexKV.Lock(fs.WorkingDirectory)
Expand Down
10 changes: 1 addition & 9 deletions pkg/helmfile/resource_release.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
package helmfile

import (
"crypto/sha256"
"encoding/json"
"fmt"
"github.com/mumoshu/terraform-provider-eksctl/pkg/sdk/tfsdk"
"github.com/rs/xid"
"golang.org/x/xerrors"
"io/ioutil"
"runtime/debug"

"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
Expand Down Expand Up @@ -299,17 +297,11 @@ func NewReleaseSetWithSingleRelease(d ResourceRead) (*ReleaseSet, error) {
if err != nil {
return nil, err
}
first := sha256.New()
first.Write(bs)
path := fmt.Sprintf("helmfile-%x.yaml", first.Sum(nil))
if err := ioutil.WriteFile(path, bs, 0755); err != nil {
return nil, err
}

rs := &ReleaseSet{
Bin: r.Bin,
HelmBin: r.HelmBin,
Path: path,
Content: string(bs),
Environment: "default",
WorkingDirectory: r.WorkingDirectory,
Kubeconfig: r.Kubeconfig,
Expand Down