Repository: googleforgames/open-match
Pull request #705 (2019-07-30 18:56:15)

Refactor ci-reap-cluster to ci-reap-namespaces
This commit together with #701 removes the `Test: Create Cluster` step from cloudbuild to speed up CI. ci-reaper is now responsible for deleting staled namespaces to make sure resources in a broken build will be freed


Patch:
diff --git a/Makefile b/Makefile
index 9796dcbc..fcebb00f 100644
--- a/Makefile
+++ b/Makefile
@@ -861,8 +861,8 @@ else
 	@echo "Not deploying build artifacts to open-match.dev because this is not a post commit change."
 endif
 
-ci-reap-clusters: build/toolchain/bin/reaper$(EXE_EXTENSION)
-	$(TOOLCHAIN_BIN)/reaper -project=$(GCP_PROJECT_ID) -age=30m -location=$(GCP_ZONE) -label=$(OPEN_MATCH_CI_LABEL)
+ci-reap-namespaces: build/toolchain/bin/reaper$(EXE_EXTENSION)
+	$(TOOLCHAIN_BIN)/reaper -age=30m
 
 # For presubmit we want to update the protobuf generated files and verify that tests are good.
 presubmit: GOLANG_TEST_COUNT = 5
@@ -1063,4 +1063,4 @@ ifeq ($(shell whoami),root)
 endif
 endif
 
-.PHONY: docker gcloud update-deps sync-deps sleep-10 sleep-30 all build proxy-dashboard proxy-prometheus proxy-grafana clean clean-build clean-toolchain clean-archives clean-binaries clean-protos presubmit test ci-reap-clusters md-test vet
+.PHONY: docker gcloud update-deps sync-deps sleep-10 sleep-30 all build proxy-dashboard proxy-prometheus proxy-grafana clean clean-build clean-toolchain clean-archives clean-binaries clean-protos presubmit test ci-reap-namespaces md-test vet
diff --git a/cloudbuild.yaml b/cloudbuild.yaml
index 63c60b0e..6b960300 100644
--- a/cloudbuild.yaml
+++ b/cloudbuild.yaml
@@ -146,10 +146,9 @@ steps:
   - name: 'go-vol'
     path: '/go'
 
-# Disable ci-reap-clusters for now. TODO: refactor it to ci-reap-namespace
 - id: 'Test: Delete Open Match'
   name: 'gcr.io/$PROJECT_ID/open-match-build'
-  args: ['make', 'SHORT_SHA=${SHORT_SHA}', 'GCLOUD_EXTRA_FLAGS=--async', 'GCP_PROJECT_ID=${PROJECT_ID}', 'ci-reap-clusters', 'delete-chart']
+  args: ['make', 'SHORT_SHA=${SHORT_SHA}', 'GCLOUD_EXTRA_FLAGS=--async', 'GCP_PROJECT_ID=${PROJECT_ID}', 'ci-reap-namespaces', 'delete-chart']
   waitFor: ['Test: End-to-End Cluster']
 
 artifacts:
diff --git a/go.mod b/go.mod
index 347ae767..8540577b 100644
--- a/go.mod
+++ b/go.mod
@@ -17,7 +17,7 @@ module open-match.dev/open-match
 go 1.12
 
 require (
-	cloud.google.com/go v0.40.0
+	cloud.google.com/go v0.40.0 // indirect
 	contrib.go.opencensus.io/exporter/jaeger v0.1.0
 	contrib.go.opencensus.io/exporter/ocagent v0.5.0
 	contrib.go.opencensus.io/exporter/prometheus v0.1.0
diff --git a/tools/reaper/internal/internal.go b/tools/reaper/internal/internal.go
index 4e8668ed..7fadab4a 100644
--- a/tools/reaper/internal/internal.go
+++ b/tools/reaper/internal/internal.go
@@ -16,16 +16,25 @@
 package internal
 
 import (
-	"context"
 	"fmt"
 	"log"
 	"net"
 	"net/http"
+	"os"
+	"os/user"
+	"path/filepath"
 	"strings"
 	"time"
 
-	container "cloud.google.com/go/container/apiv1"
-	containerpb "google.golang.org/genproto/googleapis/container/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+
+	// Enabled gcloud auth plugin
+	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
+	"k8s.io/client-go/tools/clientcmd"
+
+	"github.com/pkg/errors"
 )
 
 // Params for deleting a cluster.
@@ -72,13 +81,13 @@ func Serve(conn net.Listener, params *Params) error {
 	})
 
 	mux.HandleFunc("/reap", func(w http.ResponseWriter, req *http.Request) {
-		resp, err := ReapCluster(params)
+		err := ReapNamespaces(params)
 		if err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 			fmt.Fprintf(w, "ERROR: %s", err.Error())
 		} else {
 			w.WriteHeader(http.StatusOK)
-			fmt.Fprintf(w, "OK: %s", resp)
+			fmt.Fprintf(w, "OK")
 		}
 	})
 
@@ -87,82 +96,61 @@ func Serve(conn net.Listener, params *Params) error {
 	return server.Serve(conn)
 }
 
-// ReapCluster scans for orphaned clusters and deletes them.
-func ReapCluster(params *Params) (string, error) {
-	log.Printf("Scanning for orphaned clusters in projects/%s/locations/%s that are older than %s with label %s.", params.ProjectID, params.Location, params.Age.String(), params.Label)
-	ctx := context.Background()
-	c, err := container.NewClusterManagerClient(ctx)
-	if err != nil {
-		return "", err
-	}
-	clusters, err := listOrphanedClusters(ctx, c, params)
+// ReapNamespaces scans for orphaned namespaces and deletes them.
+func ReapNamespaces(params *Params) error {
+	log.Printf("Scanning for orphaned namespaces that are older than %s.", params.Age.String())
+
+	u, err := user.Current()
 	if err != nil {
-		return "", err
+		log.Fatalf("cannot get current user, %s", err)
 	}
 
-	err = deleteClusters(ctx, c, clusters)
+	kubeconfig := filepath.Join(u.HomeDir, ".kube", "config")
+	kubeconfigFromEnv := os.Getenv("KUBECONFIG")
+
+	if !fileExists(kubeconfig) && fileExists(kubeconfigFromEnv) {
+		kubeconfig = kubeconfigFromEnv
+	}
+	config, err := clientcmd.BuildConfigFromFlags("", kubeconfig)
 	if err != nil {
-		return "", err
+		return errors.Wrapf(err, "building Kubernetes config from flags %s failed", kubeconfig)
 	}
 
-	if len(clusters) > 0 {
-		return fmt.Sprintf("Deleted %d GKE clusters", len(clusters)), nil
+	kubeClient, err := kubernetes.NewForConfig(config)
+	if err != nil {
+		return errors.Wrapf(err, "creating Kubernetes client from config failed\nkubeconfig= %s\nconfig= %+v", kubeconfig, config)
 	}
-	return fmt.Sprintf("There were no orphaned clusters. :)"), nil
-}
 
-func listOrphanedClusters(ctx context.Context, c *container.ClusterManagerClient, params *Params) ([]*containerpb.Cluster, error) {
-	resp, err := c.ListClusters(ctx, &containerpb.ListClustersRequest{
-		Parent: fmt.Sprintf("projects/%s/locations/%s", params.ProjectID, params.Location),
-	})
+	namespaceInterface := kubeClient.CoreV1().Namespaces()
+	namespaces, err := namespaceInterface.List(metav1.ListOptions{})
 	if err != nil {
-		return []*containerpb.Cluster{}, err
+		return errors.Wrap(err, "listing namespace from kubernetes cluster failed")
 	}
-	orphanedClusters := []*containerpb.Cluster{}
-	for _, cluster := range resp.Clusters {
-		if isOrphaned(cluster, params) {
-			log.Printf("Cluster '/%s' was orphaned.\nDetails: %+v", cluster.Name, cluster)
-			orphanedClusters = append(orphanedClusters, cluster)
-		}
-	}
-	return orphanedClusters, nil
-}
 
-func deleteClusters(ctx context.Context, c *container.ClusterManagerClient, clusters []*containerpb.Cluster) error {
-	for _, cluster := range clusters {
-		clusterName := selfLinkToQualifiedName(cluster.SelfLink)
-		log.Printf("Deleting Orphaned GKE Cluster %s", clusterName)
-		resp, err := c.DeleteCluster(ctx, &containerpb.DeleteClusterRequest{
-			Name: clusterName,
-		})
+	for _, namespace := range namespaces.Items {
+		if !isOrphaned(namespace, params) {
+			log.Printf("skipping namespace %s created at %s", namespace.ObjectMeta.Name, namespace.ObjectMeta.CreationTimestamp)
+			continue
+		}
+		
+		err = namespaceInterface.Delete(namespace.ObjectMeta.Name, &metav1.DeleteOptions{})
 		if err != nil {
-			return err
+			log.Printf("error: %s", err.Error())
+			continue
 		}
-		log.Printf("Delete GKE Cluster: %s => %v", clusterName, resp)
+
+		log.Printf("deleting namespace %s created at %s", namespace.ObjectMeta.Name, namespace.ObjectMeta.CreationTimestamp)
 	}
-	return nil
+
+	return err
 }
 
-func isOrphaned(cluster *containerpb.Cluster, params *Params) bool {
-	deadline := time.Now().Add(-1 * params.Age)
-	creationTime, err := time.Parse(time.RFC3339, cluster.CreateTime)
-	if err != nil {
-		log.Printf("cannot parse time '%s' because '%v', ignoring for orphan detection.", cluster.CreateTime, err)
-		return false
-	}
-	if creationTime.After(deadline) {
-		return false
-	}
-	if _, ok := cluster.ResourceLabels[params.Label]; !ok {
-		return false
-	}
-	return true
+func fileExists(name string) bool {
+	_, err := os.Stat(name)
+	return err == nil
 }
 
-func selfLinkToQualifiedName(selfLink string) string {
-	parts := strings.Split(selfLink, "v1/")
-	if len(parts) != 2 {
-		log.Fatalf("SelfLink: %s is not a valid selflink it should contain 'v1/'", selfLink)
-	}
-	return parts[1]
+func isOrphaned(namespace corev1.Namespace, params *Params) bool {
+	deadline := time.Now().Add(-1 * params.Age).UTC()
+	return namespace.ObjectMeta.CreationTimestamp.Time.Before(deadline) && strings.HasPrefix(namespace.ObjectMeta.Name, "open-match")
 }
diff --git a/tools/reaper/reaper.go b/tools/reaper/reaper.go
index 4ba5d884..d772d516 100644
--- a/tools/reaper/reaper.go
+++ b/tools/reaper/reaper.go
@@ -20,18 +20,16 @@ import (
 	"fmt"
 	"log"
 	"net"
-	reaperInternal "open-match.dev/open-match/tools/reaper/internal"
 	"os"
 	"strconv"
 	"time"
+
+	reaperInternal "open-match.dev/open-match/tools/reaper/internal"
 )
 
 var (
-	projectIDFlag = flag.String("project", "open-match-build", "Target Project ID to scan for leaked clusters.")
-	ageFlag       = flag.Duration("age", time.Hour*1, "Age of a cluster before it's a candidate for deletion.")
-	labelFlag     = flag.String("label", "open-match-ci", "Required label that must be on the cluster for it to be considered for reaping.")
-	locationFlag  = flag.String("location", "us-west1-a", "Location of the cluster")
-	portFlag      = flag.Int("port", 0, "HTTP Port to serve from.")
+	ageFlag  = flag.Duration("age", time.Minute*30, "Age of a namespace before it's a candidate for deletion.")
+	portFlag = flag.Int("port", 0, "HTTP Port to serve from.")
 )
 
 func main() {
@@ -57,19 +55,16 @@ func main() {
 			log.Fatal(err)
 		}
 	} else {
-		if resp, err := reaperInternal.ReapCluster(flagToParams()); err != nil {
+		if err := reaperInternal.ReapNamespaces(flagToParams()); err != nil {
 			log.Fatal(err)
 		} else {
-			log.Print(resp)
+			log.Print("OK")
 		}
 	}
 }
 
 func flagToParams() *reaperInternal.Params {
 	return &reaperInternal.Params{
-		Age:       *ageFlag,
-		Label:     *labelFlag,
-		ProjectID: *projectIDFlag,
-		Location:  *locationFlag,
+		Age: *ageFlag,
 	}
 }


Test patch:
diff --git a/tools/reaper/internal/internal_test.go b/tools/reaper/internal/internal_test.go
index c406d001..6b6dab23 100644
--- a/tools/reaper/internal/internal_test.go
+++ b/tools/reaper/internal/internal_test.go
@@ -16,7 +16,6 @@ package internal
 
 import (
 	"fmt"
-	containerpb "google.golang.org/genproto/googleapis/container/v1"
 	"io/ioutil"
 	"net"
 	"net/http"
@@ -24,7 +23,8 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/assert"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestServeAndClose(t *testing.T) {
@@ -53,10 +53,7 @@ func serve(conn net.Listener) func() {
 	go func() {
 		wg.Done()
 		err := Serve(conn, &Params{
-			Age:       time.Minute * 30,
-			Label:     "open-match-ci",
-			ProjectID: "open-match-build",
-			Location:  "us-west1-a",
+			Age: time.Minute * 30,
 		})
 		if err != nil {
 			fmt.Printf("http server did not shutdown cleanly, %s", err)
@@ -77,60 +74,56 @@ func killLater(port int) {
 
 func TestIsOrphaned(t *testing.T) {
 	testCases := []struct {
-		age      time.Duration
-		label    string
-		cluster  *containerpb.Cluster
-		expected bool
+		age       time.Duration
+		namespace corev1.Namespace
+		expected  bool
 	}{
 		{
 			time.Second,
-			"ci",
-			&containerpb.Cluster{
-				Name:           "Orphaned",
-				CreateTime:     time.Now().Add(-1 * time.Hour).Format(time.RFC3339),
-				ResourceLabels: map[string]string{"ci": "ok"},
+			corev1.Namespace{
+				TypeMeta: metav1.TypeMeta{},
+				ObjectMeta: metav1.ObjectMeta{
+					CreationTimestamp: metav1.Time{Time: time.Now().Add(-1 * time.Hour).UTC()},
+					Name:              "open-match-test-namespace",
+				},
 			},
 			true,
 		},
 		{
 			time.Second,
-			"ci",
-			&containerpb.Cluster{
-				Name:           "Not Orphaned, Created in the Future",
-				CreateTime:     time.Now().Add(1 * time.Hour).Format(time.RFC3339),
-				ResourceLabels: map[string]string{"ci": "ok"},
+			corev1.Namespace{
+				TypeMeta: metav1.TypeMeta{},
+				ObjectMeta: metav1.ObjectMeta{
+					CreationTimestamp: metav1.Time{Time: time.Now().Add(1 * time.Hour).UTC()},
+					Name:              "open-match-future-namespace",
+				},
 			},
 			false,
 		},
 		{
 			time.Second,
-			"ci",
-			&containerpb.Cluster{
-				Name:           "Not Orphaned, Created in Past but no Label",
-				CreateTime:     time.Now().Add(-1 * time.Hour).Format(time.RFC3339),
-				ResourceLabels: map[string]string{},
+			corev1.Namespace{
+				TypeMeta: metav1.TypeMeta{},
+				ObjectMeta: metav1.ObjectMeta{
+					CreationTimestamp: metav1.Time{Time: time.Now().Add(-1 * time.Hour).UTC()},
+					Name:              "kube-system",
+				},
 			},
 			false,
 		},
 	}
 	for _, tc := range testCases {
-		t.Run(fmt.Sprintf("%s deleted= %t", tc.cluster.Name, tc.expected), func(t *testing.T) {
-			actual := isOrphaned(tc.cluster, &Params{
-				Age:   tc.age,
-				Label: tc.label,
+		t.Run(fmt.Sprintf("%s deleted= %t", tc.namespace.ObjectMeta.Name, tc.expected), func(t *testing.T) {
+			actual := isOrphaned(tc.namespace, &Params{
+				Age: tc.age,
 			})
 			if actual != tc.expected {
 				if tc.expected {
-					t.Errorf("Cluster %s was NOT marked as orphaned but should have.", tc.cluster.Name)
+					t.Errorf("Namespace %s was NOT marked as orphaned but should have.", tc.namespace.ObjectMeta.Name)
 				} else {
-					t.Errorf("Cluster %s was marked as orphaned but should not have.", tc.cluster.Name)
+					t.Errorf("Namespace %s was marked as orphaned but should not have.", tc.namespace.ObjectMeta.Name)
 				}
 			}
 		})
 	}
 }
-
-func TestSelfLinkToQualifiedName(t *testing.T) {
-	assert := assert.New(t)
-	assert.Equal("projects/open-match-build/zones/us-central1-a/clusters/orphaned", selfLinkToQualifiedName("https://container.googleapis.com/v1/projects/open-match-build/zones/us-central1-a/clusters/orphaned"))
-}