Repository: crossplane-contrib/provider-terraform
Pull request #5 (2021-07-03 01:46:59)

Garbage collect workdirs
<!--
Thank you for helping to improve Crossplane!

Please read through https://git.io/fj2m9 if this is your first time opening a
Crossplane pull request. Find us in https://slack.crossplane.io/messages/dev if
you need any help contributing.
-->

### Description of your changes

<!--
Briefly describe what this pull request does. Be sure to direct your reviewers'
attention to anything that needs special consideration.

We love pull requests that resolve an open Crossplane issue. If yours does, you
can uncomment the below line to indicate which issue your PR fixes, for example
"Fixes #500":

-->
This commit ensures that we don't leak the working directories created for each Terraform workspace when they're deleted. A garbage collector runs every hour, and will remove any directory that does not correspond to an extant workspace.

I have:

- [x] Read and followed Crossplane's [contribution process].
- [x] Run `make reviewable test` to ensure this PR is ready for review.

### How has this code been tested

<!--
Before reviewers can be confident in the correctness of this pull request, it
needs to tested and shown to be correct. Briefly describe the testing that has
already been done or which is planned for this change.
-->

[contribution process]: https://git.io/fj2m9

I've created two workspaces (derived from the inline example), deleted one, and confirmed that after an hour has passed only one workspace's workdir remains.


Patch:
diff --git a/README.md b/README.md
index 2ebf940..7320cf6 100644
--- a/README.md
+++ b/README.md
@@ -52,8 +52,5 @@ Known limitations:
   process will be killed. You will potentially lose state and leak resources.
 * The provider won't emit an event until _after_ it has successfully applied the
   Terraform module, which can take a long time.
-* Each `Workspace` is allocated a directory under the `/tf` directory inside the
-  provider container. These directories are not yet garbage collected when a
-  Workspace is deleted.
 
 [Kubernetes]: https://www.terraform.io/docs/language/settings/backends/kubernetes.html
\ No newline at end of file
diff --git a/internal/controller/template.go b/internal/controller/terraform.go
similarity index 100%
rename from internal/controller/template.go
rename to internal/controller/terraform.go
diff --git a/internal/controller/workspace/workspace.go b/internal/controller/workspace/workspace.go
index feeabef..a78d2fd 100644
--- a/internal/controller/workspace/workspace.go
+++ b/internal/controller/workspace/workspace.go
@@ -41,6 +41,7 @@ import (
 
 	"github.com/crossplane-contrib/provider-terraform/apis/v1alpha1"
 	"github.com/crossplane-contrib/provider-terraform/internal/terraform"
+	"github.com/crossplane-contrib/provider-terraform/internal/workdir"
 )
 
 const (
@@ -87,10 +88,14 @@ func Setup(mgr ctrl.Manager, l logging.Logger, rl workqueue.RateLimiter, poll, t
 		RateLimiter: ratelimiter.NewDefaultManagedRateLimiter(rl),
 	}
 
+	fs := afero.Afero{Fs: afero.NewOsFs()}
+	gc := workdir.NewGarbageCollector(mgr.GetClient(), tfDir, workdir.WithFs(fs), workdir.WithLogger(l))
+	go gc.Run(context.TODO())
+
 	c := &connector{
 		kube:      mgr.GetClient(),
 		usage:     resource.NewProviderConfigUsageTracker(mgr.GetClient(), &v1alpha1.ProviderConfigUsage{}),
-		fs:        afero.Afero{Fs: afero.NewOsFs()},
+		fs:        fs,
 		terraform: func(dir string) tfclient { return terraform.Harness{Path: tfPath, Dir: dir} },
 	}
 
@@ -123,7 +128,8 @@ func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.E
 		return nil, errors.New(errNotWorkspace)
 	}
 
-	// TODO(negz): Garbage collect this directory.
+	// NOTE(negz): This directory will be garbage collected by the workdir
+	// garbage collector that is started in Setup.
 	dir := filepath.Join(tfDir, string(cr.GetUID()))
 	if err := c.fs.MkdirAll(dir, 0700); resource.Ignore(os.IsExist, err) != nil {
 		return nil, errors.Wrap(err, errMkdir)
diff --git a/internal/workdir/workdir.go b/internal/workdir/workdir.go
new file mode 100644
index 0000000..3866c65
--- /dev/null
+++ b/internal/workdir/workdir.go
@@ -0,0 +1,137 @@
+/*
+Copyright 2021 The Crossplane Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package workdir
+
+import (
+	"context"
+	"path/filepath"
+	"strings"
+	"time"
+
+	"github.com/crossplane-contrib/provider-terraform/apis/v1alpha1"
+	"github.com/crossplane/crossplane-runtime/pkg/logging"
+	"github.com/pkg/errors"
+	"github.com/spf13/afero"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// Error strings.
+const (
+	errListWorkspaces = "cannot list workspaces"
+	errFmtReadDir     = "cannot read directory %q"
+)
+
+// A GarbageCollector garbage collects the working directories of Terraform
+// workspaces that no longer exist.
+type GarbageCollector struct {
+	kube      client.Client
+	parentDir string
+	fs        afero.Afero
+	interval  time.Duration
+	log       logging.Logger
+}
+
+// A GarbageCollectorOption configures a new GarbageCollector.
+type GarbageCollectorOption func(*GarbageCollector)
+
+// WithFs configures the afero filesystem implementation in which work dirs will
+// be garbage collected. The default is the real operating system filesystem.
+func WithFs(fs afero.Afero) GarbageCollectorOption {
+	return func(gc *GarbageCollector) { gc.fs = fs }
+}
+
+// WithInterval configures how often garbage collection will run. The default
+// interval is one hour.
+func WithInterval(i time.Duration) GarbageCollectorOption {
+	return func(gc *GarbageCollector) { gc.interval = i }
+}
+
+// WithLogger configures the logger that will be used. The default is a no-op
+// logger never emits logs.
+func WithLogger(l logging.Logger) GarbageCollectorOption {
+	return func(gc *GarbageCollector) { gc.log = l }
+}
+
+// NewGarbageCollector returns a garbage collector that garbage collects the
+// working directories of Terraform workspaces.
+func NewGarbageCollector(c client.Client, parentDir string, o ...GarbageCollectorOption) *GarbageCollector {
+	gc := &GarbageCollector{
+		kube:      c,
+		parentDir: parentDir,
+		fs:        afero.Afero{Fs: afero.NewOsFs()},
+		interval:  1 * time.Hour,
+		log:       logging.NewNopLogger(),
+	}
+
+	for _, fn := range o {
+		fn(gc)
+	}
+
+	return gc
+}
+
+// Run the garbage collector. Blocks until the supplied context is done.
+func (gc *GarbageCollector) Run(ctx context.Context) {
+	t := time.NewTicker(gc.interval)
+	for {
+		select {
+		case <-ctx.Done():
+			return
+		case <-t.C:
+			if err := gc.collect(ctx); err != nil {
+				gc.log.Info("Garbage collection failed", "error", err)
+			}
+		}
+	}
+}
+
+func (gc *GarbageCollector) collect(ctx context.Context) error {
+	l := &v1alpha1.WorkspaceList{}
+	if err := gc.kube.List(ctx, l); err != nil {
+		return errors.Wrap(err, errListWorkspaces)
+	}
+
+	exists := map[string]bool{}
+	for _, ws := range l.Items {
+		exists[string(ws.GetUID())] = true
+	}
+
+	fis, err := gc.fs.ReadDir(gc.parentDir)
+	if err != nil {
+		return errors.Wrapf(err, errFmtReadDir, gc.parentDir)
+	}
+
+	failed := make([]string, 0)
+	for _, fi := range fis {
+		if !fi.IsDir() {
+			continue
+		}
+		if exists[fi.Name()] {
+			continue
+		}
+		path := filepath.Join(gc.parentDir, fi.Name())
+		if err := gc.fs.RemoveAll(path); err != nil {
+			failed = append(failed, path)
+		}
+	}
+
+	if len(failed) > 0 {
+		return errors.Errorf("could not delete directories: %v", strings.Join(failed, ", "))
+	}
+
+	return nil
+}


Test patch:
diff --git a/internal/workdir/workdir_test.go b/internal/workdir/workdir_test.go
new file mode 100644
index 0000000..59e3860
--- /dev/null
+++ b/internal/workdir/workdir_test.go
@@ -0,0 +1,139 @@
+/*
+Copyright 2021 The Crossplane Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package workdir
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/pkg/errors"
+	"github.com/spf13/afero"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/crossplane/crossplane-runtime/pkg/test"
+
+	"github.com/crossplane-contrib/provider-terraform/apis/v1alpha1"
+)
+
+func withDirs(fs afero.Afero, dir ...string) afero.Afero {
+	for _, d := range dir {
+		_ = fs.Mkdir(d, os.ModePerm)
+	}
+	return fs
+}
+
+func getDirs(fs afero.Afero, parentDir string) []string {
+	dirs := make([]string, 0)
+	fis, _ := fs.ReadDir(parentDir)
+	for _, fi := range fis {
+		if !fi.IsDir() {
+			continue
+		}
+		dirs = append(dirs, fi.Name())
+	}
+	return dirs
+}
+
+func TestCollect(t *testing.T) {
+	parentDir := "/test"
+
+	type fields struct {
+		kube       client.Client
+		parentdDir string
+		fs         afero.Afero
+	}
+	type args struct {
+		ctx context.Context
+	}
+	type want struct {
+		dirs []string
+		err  error
+	}
+	cases := map[string]struct {
+		reason string
+		fields fields
+		args   args
+		want   want
+	}{
+		"ErrNoParentDir": {
+			reason: "Garbage collection should fail when the parent directory does not exist.",
+			fields: fields{
+				kube:       &test.MockClient{MockList: test.NewMockListFn(nil)},
+				parentdDir: parentDir,
+				fs:         afero.Afero{Fs: afero.NewMemMapFs()},
+			},
+			want: want{
+				err: errors.Wrapf(errors.Errorf("open %s: file does not exist", parentDir), errFmtReadDir, parentDir),
+			},
+		},
+		"NoOp": {
+			reason: "Garbage collection should succeed when there are no workspaces or workdirs.",
+			fields: fields{
+				kube:       &test.MockClient{MockList: test.NewMockListFn(nil)},
+				parentdDir: parentDir,
+				fs:         withDirs(afero.Afero{Fs: afero.NewMemMapFs()}, parentDir),
+			},
+			want: want{
+				err: nil,
+			},
+		},
+		"Success": {
+			reason: "Workdirs belonging to workspaces that no longer exist should be successfully garbage collected.",
+			fields: fields{
+				kube: &test.MockClient{MockList: test.NewMockListFn(nil, func(obj client.ObjectList) error {
+					*obj.(*v1alpha1.WorkspaceList) = v1alpha1.WorkspaceList{Items: []v1alpha1.Workspace{
+						{ObjectMeta: metav1.ObjectMeta{UID: types.UID("a")}},
+						{ObjectMeta: metav1.ObjectMeta{UID: types.UID("b")}},
+					}}
+					return nil
+				})},
+				parentdDir: parentDir,
+				fs: withDirs(afero.Afero{Fs: afero.NewMemMapFs()},
+					parentDir,
+					filepath.Join(parentDir, "a"),
+					filepath.Join(parentDir, "b"),
+					filepath.Join(parentDir, "c"),
+				),
+			},
+			want: want{
+				dirs: []string{"a", "b"},
+			},
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			gc := NewGarbageCollector(tc.fields.kube, tc.fields.parentdDir, WithFs(tc.fields.fs))
+			err := gc.collect(tc.args.ctx)
+			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
+				t.Errorf("gc.collect(...): -want error, +got error:\n%s", diff)
+			}
+
+			got := getDirs(tc.fields.fs, tc.fields.parentdDir)
+			if diff := cmp.Diff(tc.want.dirs, got, cmpopts.EquateEmpty()); diff != "" {
+				t.Errorf("gc.collect(...): -want dirs, +got dirs:\n%s", diff)
+			}
+		})
+	}
+
+}