fix(cleaner): remove the usage of watch api

Author: Keerthan Mala

boot.go

@@ -89,7 +89,7 @@ func main() {
 				log.Printf("Starting deleted app cleaner")
 				cleanerErrCh := make(chan error)
 				go func() {
-					if err := cleaner.Run(gitHomeDir, kubeClient.Namespaces(), fs); err != nil {
+					if err := cleaner.Run(gitHomeDir, kubeClient.Namespaces(), fs, cnf.CleanerPollSleepDuration()); err != nil {
 						cleanerErrCh <- err
 					}
 				}()

pkg/cleaner/cleaner.go

@@ -3,46 +3,110 @@
 package cleaner
 
 import (
+	"io/ioutil"
 	"log"
+	"path/filepath"
+	"strings"
+	"time"
 
 	"k8s.io/kubernetes/pkg/api"
 
 	"github.com/deis/builder/pkg/k8s"
 	"github.com/deis/builder/pkg/sys"
+	"k8s.io/kubernetes/pkg/fields"
+	"k8s.io/kubernetes/pkg/labels"
 )
 
 const (
 	dotGitSuffix = ".git"
 )
 
-// Run starts the deleted app cleaner. This function listens to the Kubernetes event stream for
-// events that indicate namespaces that were `DELETED`.
-// If the namespace name matches a folder on the local filesystem, this func deletes that folder.
-// Note that this function blocks until the watcher returned by `nsLister.Watch` is closed, so
-// you should launch it in a goroutine.
-func Run(gitHome string, nsLister k8s.NamespaceWatcher, fs sys.FS) error {
-	watcher, err := nsLister.Watch(nil, nil, "")
+// localDirs returns all of the local directories immediately under gitHome that filter returns true for.
+// filter will receive only the names of each of the top level directories (not their fully qualified paths), and should return true if it should be included in the output
+func localDirs(gitHome string, filter func(string) bool) ([]string, error) {
+	fileInfos, err := ioutil.ReadDir(gitHome)
 	if err != nil {
-		log.Printf("unable to get watch events (%s)", err)
+		return nil, err
 	}
+	var ret []string
+	for _, fileInfo := range fileInfos {
+		nm := fileInfo.Name()
+		if len(nm) <= 0 || nm == "." || !fileInfo.IsDir() {
+			continue
+		}
+		if filter(nm) {
+			ret = append(ret, nm)
+		}
+	}
+	return ret, nil
+}
+
+// getDiff gets the directories that are not in namespaceList
+func getDiff(namespaceList []api.Namespace, dirs []string) []string {
+	var ret []string
+
+	// create a set of lowercase namespace names
+	namespacesSet := make(map[string]struct{})
+	for _, ns := range namespaceList {
+		lowerName := strings.ToLower(ns.Name)
+		namespacesSet[lowerName] = struct{}{}
+	}
+
+	// get dirs not in the namespaces set
+	for _, dir := range dirs {
+		lowerName := strings.ToLower(dir)
+		if _, ok := namespacesSet[lowerName]; !ok {
+			ret = append(ret, lowerName)
+		}
+	}
+
+	return ret
+}
+
+func stripSuffixes(strs []string, suffix string) []string {
+	ret := make([]string, len(strs))
+	for i, str := range strs {
+		idx := strings.LastIndex(str, suffix)
+		if idx >= 0 {
+			ret[i] = str[:idx]
+		} else {
+			ret[i] = str
+		}
+	}
+	return ret
+}
+
+func dirHasGitSuffix(dir string) bool {
+	return strings.HasSuffix(dir, dotGitSuffix)
+}
+
+// Run starts the deleted app cleaner. Every pollSleepDuration, it compares the result of nsLister.List with the directories in the top level of gitHome on the local file system.
+// On any error, it uses log messages to output a human readable description of what happened.
+func Run(gitHome string, nsLister k8s.NamespaceLister, fs sys.FS, pollSleepDuration time.Duration) error {
 	for {
-		select {
-		case event, ok := <-watcher.ResultChan():
-			if !ok {
-				break
-			}
-			if event.Type == "DELETED" {
-				switch event.Object.(type) {
-				case (*api.Namespace):
-					namespace := event.Object.(*api.Namespace)
-					appToDelete := gitHome + "/" + namespace.ObjectMeta.Name + dotGitSuffix
-					if err := fs.RemoveAll(appToDelete); err != nil {
-						log.Printf("Cleaner error removing deleted app %s (%s)", appToDelete, err)
-					}
-				default:
-				}
+		nsList, err := nsLister.List(labels.Everything(), fields.Everything())
+		if err != nil {
+			log.Printf("Cleaner error listing namespaces (%s)", err)
+			continue
+		}
+
+		gitDirs, err := localDirs(gitHome, dirHasGitSuffix)
+		if err != nil {
+			log.Printf("Cleaner error listing local git directories (%s)", err)
+			continue
+		}
+
+		gitDirs = stripSuffixes(gitDirs, dotGitSuffix)
+
+		appsToDelete := getDiff(nsList.Items, gitDirs)
+
+		for _, appToDelete := range appsToDelete {
+			dirToDelete := filepath.Join(gitHome, appToDelete+dotGitSuffix)
+			if err := fs.RemoveAll(dirToDelete); err != nil {
+				log.Printf("Cleaner error removing deleted app %s (%s)", dirToDelete, err)
 			}
 		}
-	}
 
+		time.Sleep(pollSleepDuration)
+	}
 }

pkg/cleaner/cleaner_test.go

@@ -1,60 +1,82 @@
 package cleaner
 
 import (
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 
-	"time"
-
-	"github.com/deis/builder/pkg/sys"
+	"github.com/arschles/assert"
 	"k8s.io/kubernetes/pkg/api"
-	"k8s.io/kubernetes/pkg/fields"
-	"k8s.io/kubernetes/pkg/labels"
-	"k8s.io/kubernetes/pkg/watch"
 )
 
-type namespace struct {
+func TestGetDiff(t *testing.T) {
+	nsList := []api.Namespace{
+		api.Namespace{ObjectMeta: api.ObjectMeta{Name: "app1"}},
+		api.Namespace{ObjectMeta: api.ObjectMeta{Name: "app2"}},
+	}
+	dirList := []string{"app1", "app3"}
+	diff := getDiff(nsList, dirList)
+	assert.Equal(t, len(diff), 1, "number of items in the disjunction")
 }
 
-// Watch returns a watch.Interface that watches the requested namespaces.
-func (r *namespace) Watch(label labels.Selector, field fields.Selector, resourceversion string) (watch.Interface, error) {
-	nst := watch.NewFake()
-	go func() {
-		nst.Add(&api.Namespace{ObjectMeta: api.ObjectMeta{Name: "dir1"}})
-		nst.Modify(&api.Namespace{ObjectMeta: api.ObjectMeta{Name: "dir1"}})
-		nst.Delete(&api.Namespace{ObjectMeta: api.ObjectMeta{Name: "dir1"}})
-		nst.Modify(&api.Namespace{ObjectMeta: api.ObjectMeta{Name: "dir2"}})
-		nst.Delete(nil)
-		nst.Add(&api.Pod{ObjectMeta: api.ObjectMeta{Name: "dir3"}})
-		nst.Delete(&api.Pod{ObjectMeta: api.ObjectMeta{Name: "dir3"}})
-		nst.Stop()
-	}()
-	return nst, nil
+func TestDirHasGitSuffix(t *testing.T) {
+	assert.True(t, dirHasGitSuffix("a.git"), "'a.git' reported no git suffix")
+	assert.False(t, dirHasGitSuffix("abc"), "'a' reported git suffix")
 }
 
-func (r *namespace) IsAnAPIObject() {}
-
-func TestCleanerRun(t *testing.T) {
-	ns := &namespace{}
-	fs := sys.NewFakeFS()
-	dirhome := "/home/git"
-	fs.Files["/home/git/dir1.git"] = []byte{}
-	fs.Files["/home/git/dir2.git"] = []byte{}
-	fs.Files["/home/git/dir3.git"] = []byte{}
-	go Run(dirhome, ns, fs)
-	time.Sleep(5 * time.Second)
-	// Namespace with name dir1 got deleted directory should be deleted
-	_, ok := fs.Files["/home/git/dir1.git"]
-	if ok {
-		t.Fatal("Test failed: Deleting a namespace should delete respective direcotry")
+func TestLocalDirs(t *testing.T) {
+	wd, err := os.Getwd()
+	assert.NoErr(t, err)
+	pkgDir, err := filepath.Abs(wd + "/..")
+	assert.NoErr(t, err)
+	lDirs, err := localDirs(pkgDir, func(dir string) bool {
+		// no directories with any dots in them
+		return len(strings.Split(dir, ".")) == 1
+	})
+	assert.NoErr(t, err)
+
+	expectedPackages := map[string]int{
+		"cleaner":    1,
+		"conf":       1,
+		"controller": 1,
+		"env":        1,
+		"git":        1,
+		"gitreceive": 1,
+		"healthsrv":  1,
+		"k8s":        1,
+		"sshd":       1,
+		"storage":    1,
+		"sys":        1,
 	}
-	// Namespace with name dir2 got modified directory should not be deleted
-	_, ok = fs.Files["/home/git/dir2.git"]
-	if !ok {
-		t.Fatal("Test failed: Modyfiying a namespace should not delete respective direcotry")
+
+	actualPackages := map[string]int{}
+	for _, lDir := range lDirs {
+		actualPackages[lDir]++
 	}
-	// Pod with name dir3 got deleted directory should not be deleted
-	_, ok = fs.Files["/home/git/dir3.git"]
-	if !ok {
-		t.Fatal("Test failed: Deleting a pod should not delete respective direcotry")
+	assert.Equal(t, len(actualPackages), len(expectedPackages), "number of packages")
+	for actualPackageName, actualNum := range actualPackages {
+		if actualNum != 1 {
+			t.Errorf("found %d %s packages", actualNum, actualPackageName)
+			continue
+		}
+		expectedNum, ok := expectedPackages[actualPackageName]
+		if !ok {
+			t.Errorf("found unexpected package %s", actualPackageName)
+			continue
+		}
+		if actualNum != expectedNum {
+			t.Errorf("found %d %s packages, expected %d", actualNum, actualPackageName, expectedNum)
+			continue
+		}
+	}
+}
+
+func TestStripSuffixes(t *testing.T) {
+	strs := []string{"a.git", "b.git", "c.git", "d"}
+	newStrs := stripSuffixes(strs, dotGitSuffix)
+	assert.Equal(t, len(newStrs), len(strs), "number of strings")
+	for _, str := range newStrs {
+		assert.False(t, strings.HasSuffix(str, dotGitSuffix), "string %s has suffix %s", str, dotGitSuffix)
 	}
 }

pkg/sshd/config.go

@@ -10,7 +10,7 @@ type Config struct {
 	SSHHostPort                  int    `envconfig:"SSH_HOST_PORT" default:"2223" required:"true"`
 	HealthSrvPort                int    `envconfig:"HEALTH_SERVER_PORT" default:"8092"`
 	HealthSrvTestStorageRegion   string `envconfig:"STORAGE_REGION" default:"us-east-1"`
-	CleanerPollSleepDurationSec  int    `envconfig:"CLEANER_POLL_SLEEP_DURATION_SEC" default:"1"`
+	CleanerPollSleepDurationSec  int    `envconfig:"CLEANER_POLL_SLEEP_DURATION_SEC" default:"5"`
 	StorageType                  string `envconfig:"BUILDER_STORAGE" default:"minio"`
 	SlugBuilderImagePullPolicy   string `envconfig:"SLUG_BUILDER_IMAGE_PULL_POLICY" default:"Always"`
 	DockerBuilderImagePullPolicy string `envconfig:"DOCKER_BUILDER_IMAGE_PULL_POLICY" default:"Always"`