Cluster management lib: create cluster with addon (#687)

* Create cluster with addon

* Apply suggestions from code review

Co-Authored-By: Victor Agababov <vagababov@gmail.com>
Co-Authored-By: Adriano Cunha <35786489+adrcunha@users.noreply.github.com>

testutils/clustermanager/doc.go

@@ -18,16 +18,5 @@ limitations under the License.
 Package clustermanager provides support for managing clusters for e2e tests,
 responsible for creating/deleting cluster, and cluster life cycle management if
 running in Prow
-usage example:
-func acquireCluster() {
-    clusterOps := GKEClient{}.Setup(2, "n1-standard-8", "us-east1", "a", "myproject")
-    // Cast to GKEOperation
-    GKEOps := clusterOps.(GKECluster)
-    if err = GKEOps.Acquire(); err != nil {
-        log.Fatalf("Failed acquire cluster: '%v'", err)
-    }
-    log.Printf("GKE project is: %s", GKEOps.Project)
-    log.Printf("GKE cluster is: %v", GKEOps.Cluster)
-}
 */
 package clustermanager

testutils/clustermanager/example_test.go

@@ -0,0 +1,46 @@
+/*
+Copyright 2019 The Knative 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 clustermanager
+
+import "log"
+
+// This is not a real test, it's for documenting purpose, showcasing the usage
+// of entire clustermanager package
+// Important: DO NOT add `// Output` comment inside this function as it will
+// cause `go test` execute this function. See here: https://blog.golang.org/examples
+func Example() {
+	var (
+		numNodes int64 = 2
+		nodeType       = "n1-standard-8"
+		region         = "us-east1"
+		zone           = "a"
+		project        = "myGKEproject"
+		addons         = []string{"istio"}
+	)
+	gkeClient := GKEClient{}
+	clusterOps := gkeClient.Setup(&numNodes, &nodeType, &region, &zone, &project, addons)
+	// Cast to GKEOperation
+	gkeOps := clusterOps.(*GKECluster)
+	if err := gkeOps.Initialize(); err != nil {
+		log.Fatalf("failed initializing GKE Client: '%v'", err)
+	}
+	if err := gkeOps.Acquire(); err != nil {
+		log.Fatalf("failed acquire cluster: '%v'", err)
+	}
+	log.Printf("GKE project is: %s", *gkeOps.Project)
+	log.Printf("GKE cluster is: %v", gkeOps.Cluster)
+}

testutils/clustermanager/gke.go

@@ -23,12 +23,12 @@ import (
 	"strings"
 	"time"
 
+	container "google.golang.org/api/container/v1beta1"
 	"knative.dev/pkg/testutils/clustermanager/boskos"
 	"knative.dev/pkg/testutils/common"
 
 	"golang.org/x/net/context"
 	"golang.org/x/oauth2/google"
-	container "google.golang.org/api/container/v1beta1"
 )
 
 const (
@@ -60,6 +60,7 @@ type GKERequest struct {
 	Region        string
 	Zone          string
 	BackupRegions []string
+	Addons        []string
 }
 
 // GKECluster implements ClusterOperations
@@ -114,14 +115,18 @@ func (gsc *GKESDKClient) getOperation(project, location, opName string) (*contai
 // nodeType: default to n1-standard-4 if not provided
 // region: default to regional cluster if not provided, and use default backup regions
 // zone: default is none, must be provided together with region
-func (gs *GKEClient) Setup(numNodes *int64, nodeType *string, region *string, zone *string, project *string) ClusterOperations {
+// project: no default
+// addons: cluster addons to be added to cluster
+func (gs *GKEClient) Setup(numNodes *int64, nodeType *string, region *string, zone *string, project *string, addons []string) ClusterOperations {
 	gc := &GKECluster{
 		Request: &GKERequest{
 			NumNodes:      DefaultGKENumNodes,
 			NodeType:      DefaultGKENodeType,
 			Region:        DefaultGKERegion,
 			Zone:          DefaultGKEZone,
-			BackupRegions: DefaultGKEBackupRegions},
+			BackupRegions: DefaultGKEBackupRegions,
+			Addons:        addons,
+		},
 	}
 
 	if nil != project { // use provided project and create cluster
@@ -236,7 +241,11 @@ func (gc *GKECluster) Acquire() error {
 		err = nil
 		rb := &container.CreateClusterRequest{
 			Cluster: &container.Cluster{
-				Name:             clusterName,
+				Name: clusterName,
+				// Installing addons after cluster creation takes at least 5
+				// minutes, so install addons as part of cluster creation, which
+				// doesn't seem to add much time on top of cluster creation
+				AddonsConfig:     gc.getAddonsConfig(),
 				InitialNodeCount: gc.Request.NumNodes,
 				NodeConfig: &container.NodeConfig{
 					MachineType: gc.Request.NodeType,
@@ -321,6 +330,27 @@ func (gc *GKECluster) Delete() error {
 	return nil
 }
 
+// getAddonsConfig gets AddonsConfig from Request, contains the logic of
+// converting string argument to typed AddonsConfig, for example `IstioConfig`.
+// Currently supports istio
+func (gc *GKECluster) getAddonsConfig() *container.AddonsConfig {
+	const (
+		// Define all supported addons here
+		istio = "istio"
+	)
+	ac := &container.AddonsConfig{}
+	for _, name := range gc.Request.Addons {
+		switch strings.ToLower(name) {
+		case istio:
+			ac.IstioConfig = &container.IstioConfig{Disabled: false}
+		default:
+			panic(fmt.Sprintf("addon type %q not supported. Has to be one of: %q", name, istio))
+		}
+	}
+
+	return ac
+}
+
 // wait depends on unique opName(operation ID created by cloud), and waits until
 // it's done
 func (gc *GKECluster) wait(location, opName string, wait time.Duration) error {

testutils/clustermanager/gke_test.go

@@ -31,6 +31,8 @@ import (
 	boskoscommon "k8s.io/test-infra/boskos/common"
 	boskosFake "knative.dev/pkg/testutils/clustermanager/boskos/fake"
 	"knative.dev/pkg/testutils/common"
+
+	"github.com/google/go-cmp/cmp"
 )
 
 var (
@@ -96,9 +98,10 @@ func (fgsc *FakeGKESDKClient) create(project, location string, rb *container.Cre
 		fgsc.clusters[parent] = make([]*container.Cluster, 0)
 	}
 	cluster := &container.Cluster{
-		Name:     name,
-		Location: location,
-		Status:   "RUNNING",
+		Name:         name,
+		Location:     location,
+		Status:       "RUNNING",
+		AddonsConfig: rb.Cluster.AddonsConfig,
 	}
 
 	fgsc.clusters[parent] = append(fgsc.clusters[parent], cluster)
@@ -147,15 +150,17 @@ func TestSetup(t *testing.T) {
 	nodeTypeOverride := "foonode"
 	regionOverride := "fooregion"
 	zoneOverride := "foozone"
+	fakeAddons := "fake-addon"
 	datas := []struct {
 		numNodes                        *int64
 		nodeType, region, zone, project *string
+		addons                          []string
 		regionEnv, backupRegionEnv      string
 		expClusterOperations            *GKECluster
 	}{
 		{
 			// Defaults
-			nil, nil, nil, nil, nil, "", "",
+			nil, nil, nil, nil, nil, []string{}, "", "",
 			&GKECluster{
 				Request: &GKERequest{
 					NumNodes:      1,
@@ -163,11 +168,12 @@ func TestSetup(t *testing.T) {
 					Region:        "us-central1",
 					Zone:          "",
 					BackupRegions: []string{"us-west1", "us-east1"},
+					Addons:        []string{},
 				},
 			},
 		}, {
 			// Project provided
-			nil, nil, nil, nil, &fakeProj, "", "",
+			nil, nil, nil, nil, &fakeProj, []string{}, "", "",
 			&GKECluster{
 				Request: &GKERequest{
 					NumNodes:      1,
@@ -175,13 +181,14 @@ func TestSetup(t *testing.T) {
 					Region:        "us-central1",
 					Zone:          "",
 					BackupRegions: []string{"us-west1", "us-east1"},
+					Addons:        []string{},
 				},
 				Project:     &fakeProj,
 				NeedCleanup: true,
 			},
 		}, {
 			// Override other parts
-			&numNodesOverride, &nodeTypeOverride, &regionOverride, &zoneOverride, nil, "", "",
+			&numNodesOverride, &nodeTypeOverride, &regionOverride, &zoneOverride, nil, []string{}, "", "",
 			&GKECluster{
 				Request: &GKERequest{
 					NumNodes:      2,
@@ -189,11 +196,12 @@ func TestSetup(t *testing.T) {
 					Region:        "fooregion",
 					Zone:          "foozone",
 					BackupRegions: []string{},
+					Addons:        []string{},
 				},
 			},
 		}, {
 			// Override other parts but not zone
-			&numNodesOverride, &nodeTypeOverride, &regionOverride, nil, nil, "", "",
+			&numNodesOverride, &nodeTypeOverride, &regionOverride, nil, nil, []string{}, "", "",
 			&GKECluster{
 				Request: &GKERequest{
 					NumNodes:      2,
@@ -201,11 +209,12 @@ func TestSetup(t *testing.T) {
 					Region:        "fooregion",
 					Zone:          "",
 					BackupRegions: []string{"us-west1", "us-east1"},
+					Addons:        []string{},
 				},
 			},
 		}, {
 			// Set env Region
-			nil, nil, nil, nil, nil, "customregion", "",
+			nil, nil, nil, nil, nil, []string{}, "customregion", "",
 			&GKECluster{
 				Request: &GKERequest{
 					NumNodes:      1,
@@ -213,11 +222,12 @@ func TestSetup(t *testing.T) {
 					Region:        "customregion",
 					Zone:          "",
 					BackupRegions: []string{"us-west1", "us-east1"},
+					Addons:        []string{},
 				},
 			},
 		}, {
 			// Set env backupzone
-			nil, nil, nil, nil, nil, "", "backupregion1 backupregion2",
+			nil, nil, nil, nil, nil, []string{}, "", "backupregion1 backupregion2",
 			&GKECluster{
 				Request: &GKERequest{
 					NumNodes:      1,
@@ -225,6 +235,20 @@ func TestSetup(t *testing.T) {
 					Region:        "us-central1",
 					Zone:          "",
 					BackupRegions: []string{"backupregion1", "backupregion2"},
+					Addons:        []string{},
+				},
+			},
+		}, {
+			// Set addons
+			nil, nil, nil, nil, nil, []string{fakeAddons}, "", "",
+			&GKECluster{
+				Request: &GKERequest{
+					NumNodes:      1,
+					NodeType:      "n1-standard-4",
+					Region:        "us-central1",
+					Zone:          "",
+					BackupRegions: []string{"us-west1", "us-east1"},
+					Addons:        []string{fakeAddons},
 				},
 			},
 		},
@@ -273,15 +297,15 @@ func TestSetup(t *testing.T) {
 			return oldEnvFunc(s)
 		}
 		c := GKEClient{}
-		co := c.Setup(data.numNodes, data.nodeType, data.region, data.zone, data.project)
-		errPrefix := fmt.Sprintf("testing setup with:\n\tnumNodes: %v\n\tnodeType: %v\n\tregion: %v\n\tone: %v\n\tproject: %v\n\tregionEnv: %v\n\tbackupRegionEnv: %v",
-			data.numNodes, data.nodeType, data.region, data.zone, data.project, data.regionEnv, data.backupRegionEnv)
+		co := c.Setup(data.numNodes, data.nodeType, data.region, data.zone, data.project, data.addons)
+		errMsg := fmt.Sprintf("testing setup with:\n\tnumNodes: %v\n\tnodeType: %v\n\tregion: %v\n\tzone: %v\n\tproject: %v\n\taddons: %v\n\tregionEnv: %v\n\tbackupRegionEnv: %v",
+			data.numNodes, data.nodeType, data.region, data.zone, data.project, data.addons, data.regionEnv, data.backupRegionEnv)
 		gotCo := co.(*GKECluster)
 		// mock for easier comparison
 		gotCo.operations = nil
 		gotCo.boskosOps = nil
 		if !reflect.DeepEqual(co, data.expClusterOperations) {
-			t.Fatalf("%s\nwant GKECluster:\n'%v'\ngot GKECluster:\n'%v'", errPrefix, data.expClusterOperations, co)
+			t.Fatalf("%s\nwant GKECluster:\n'%v'\ngot GKECluster:\n'%v'", errMsg, data.expClusterOperations, co)
 		}
 	}
 }
@@ -395,10 +419,16 @@ func TestInitialize(t *testing.T) {
 		}
 
 		err := fgc.Initialize()
-		if !reflect.DeepEqual(err, data.expErr) || !reflect.DeepEqual(fgc.Project, data.expProj) || !reflect.DeepEqual(fgc.Cluster, data.expCluster) {
-			t.Errorf("test initialize with:\n\tuser defined project: '%v'\n\tkubeconfig set: '%v'\n\tgcloud set: '%v'\n\trunning in prow: '%v'\n\tboskos set: '%v'\n"+
-				"want:\n\tproject - '%v'\n\tcluster - '%v'\n\terr - '%v'\ngot:\n\tproject - '%v'\n\tcluster - '%v'\n\terr - '%v'",
-				data.project, data.clusterExist, data.gcloudSet, data.isProw, data.boskosProjs, data.expProj, data.expCluster, data.expErr, fgc.Project, fgc.Cluster, err)
+		errMsg := fmt.Sprintf("test initialize with:\n\tuser defined project: '%v'\n\tkubeconfig set: '%v'\n\tgcloud set: '%v'\n\trunning in prow: '%v'\n\tboskos set: '%v'",
+			data.project, data.clusterExist, data.gcloudSet, data.isProw, data.boskosProjs)
+		if !reflect.DeepEqual(data.expErr, err) {
+			t.Errorf("%s\nerror want: '%v'\nerror got: '%v'", errMsg, err, data.expErr)
+		}
+		if dif := cmp.Diff(data.expCluster, fgc.Cluster); dif != "" {
+			t.Errorf("%s\nCluster got(+) is different from wanted(-)\n%v", errMsg, dif)
+		}
+		if dif := cmp.Diff(data.expProj, fgc.Project); dif != "" {
+			t.Errorf("%s\nProject got(+) is different from wanted(-)\n%v", errMsg, dif)
 		}
 	}
 }
@@ -480,16 +510,28 @@ func TestGKECheckEnvironment(t *testing.T) {
 		}
 
 		err := fgc.checkEnvironment()
-		var clusterGot *string
+		var gotCluster *string
 		if nil != fgc.Cluster {
-			clusterGot = &fgc.Cluster.Name
+			gotCluster = &fgc.Cluster.Name
 		}
 
-		if !reflect.DeepEqual(err, data.expErr) || !reflect.DeepEqual(fgc.Project, data.expProj) || !reflect.DeepEqual(clusterGot, data.expCluster) {
+		if !reflect.DeepEqual(err, data.expErr) || !reflect.DeepEqual(fgc.Project, data.expProj) || !reflect.DeepEqual(gotCluster, data.expCluster) {
 			t.Errorf("check environment with:\n\tkubectl output: %q\n\t\terror: '%v'\n\tgcloud output: %q\n\t\t"+
 				"error: '%v'\nwant: project - '%v', cluster - '%v', err - '%v'\ngot: project - '%v', cluster - '%v', err - '%v'",
 				data.kubectlOut, data.kubectlErr, data.gcloudOut, data.gcloudErr, data.expProj, data.expCluster, data.expErr, fgc.Project, fgc.Cluster, err)
 		}
+
+		errMsg := fmt.Sprintf("check environment with:\n\tkubectl output: %q\n\t\terror: '%v'\n\tgcloud output: %q\n\t\terror: '%v'",
+			data.kubectlOut, data.kubectlErr, data.gcloudOut, data.gcloudErr)
+		if !reflect.DeepEqual(data.expErr, err) {
+			t.Errorf("%s\nerror want: '%v'\nerror got: '%v'", errMsg, err, data.expErr)
+		}
+		if dif := cmp.Diff(data.expCluster, gotCluster); dif != "" {
+			t.Errorf("%s\nCluster got(+) is different from wanted(-)\n%v", errMsg, dif)
+		}
+		if dif := cmp.Diff(data.expProj, fgc.Project); dif != "" {
+			t.Errorf("%s\nProject got(+) is different from wanted(-)\n%v", errMsg, dif)
+		}
 	}
 }
 
@@ -497,45 +539,84 @@ func TestAcquire(t *testing.T) {
 	fakeClusterName := "kpkg-e2e-cls-1234"
 	fakeBuildID := "1234"
 	datas := []struct {
-		existCluster       *container.Cluster
-		kubeconfigSet      bool
-		nextOpStatus       []string
-		expClusterName     string
-		expClusterLocation string
-		expErr             error
+		existCluster  *container.Cluster
+		kubeconfigSet bool
+		addons        []string
+		nextOpStatus  []string
+		expCluster    *container.Cluster
+		expErr        error
+		expPanic      bool
 	}{
 		{
 			// cluster already found
 			&container.Cluster{
 				Name:     "customcluster",
 				Location: "us-central1",
-			}, true, []string{}, "customcluster", "us-central1", nil,
+			}, true, []string{}, []string{}, &container.Cluster{
+				Name:         "customcluster",
+				Location:     "us-central1",
+				Status:       "RUNNING",
+				AddonsConfig: &container.AddonsConfig{},
+			}, nil, false,
 		}, {
 			// cluster exists but not set in kubeconfig, cluster will be deleted
 			// then created
 			&container.Cluster{
 				Name:     fakeClusterName,
 				Location: "us-central1",
-			}, false, []string{}, fakeClusterName, "us-central1", nil,
+			}, false, []string{}, []string{}, &container.Cluster{
+				Name:         fakeClusterName,
+				Location:     "us-central1",
+				Status:       "RUNNING",
+				AddonsConfig: &container.AddonsConfig{},
+			}, nil, false,
 		}, {
 			// cluster exists but not set in kubeconfig, cluster deletion
 			// failed, will recreate in us-west1
 			&container.Cluster{
 				Name:     fakeClusterName,
 				Location: "us-central1",
-			}, false, []string{"BAD"}, fakeClusterName, "us-west1", nil,
+			}, false, []string{}, []string{"BAD"}, &container.Cluster{
+				Name:         fakeClusterName,
+				Location:     "us-west1",
+				Status:       "RUNNING",
+				AddonsConfig: &container.AddonsConfig{},
+			}, nil, false,
 		}, {
 			// cluster creation succeeded
-			nil, false, []string{}, fakeClusterName, "us-central1", nil,
+			nil, false, []string{}, []string{}, &container.Cluster{
+				Name:         fakeClusterName,
+				Location:     "us-central1",
+				Status:       "RUNNING",
+				AddonsConfig: &container.AddonsConfig{},
+			}, nil, false,
+		}, {
+			// cluster creation succeeded with addon
+			nil, false, []string{"istio"}, []string{}, &container.Cluster{
+				Name:     fakeClusterName,
+				Location: "us-central1",
+				Status:   "RUNNING",
+				AddonsConfig: &container.AddonsConfig{
+					IstioConfig: &container.IstioConfig{Disabled: false},
+				},
+			}, nil, false,
 		}, {
 			// cluster creation succeeded retry
-			nil, false, []string{"PENDING"}, fakeClusterName, "us-west1", nil,
+			nil, false, []string{}, []string{"PENDING"}, &container.Cluster{
+				Name:         fakeClusterName,
+				Location:     "us-west1",
+				Status:       "RUNNING",
+				AddonsConfig: &container.AddonsConfig{},
+			}, nil, false,
 		}, {
 			// cluster creation failed all retry
-			nil, false, []string{"PENDING", "PENDING", "PENDING"}, "", "", fmt.Errorf("timed out waiting"),
+			nil, false, []string{}, []string{"PENDING", "PENDING", "PENDING"}, nil, fmt.Errorf("timed out waiting"), false,
 		}, {
 			// cluster creation went bad state
-			nil, false, []string{"BAD", "BAD", "BAD"}, "", "", fmt.Errorf("unexpected operation status: %q", "BAD"),
+			nil, false, []string{}, []string{"BAD", "BAD", "BAD"}, nil, fmt.Errorf("unexpected operation status: %q", "BAD"), false,
+		}, {
+			// bad addon, should get a panic
+			nil, false, []string{"bad_addon"}, []string{}, nil, nil, true,
 		},
 	}
 
@@ -552,6 +633,11 @@ func TestAcquire(t *testing.T) {
 	}()
 
 	for _, data := range datas {
+		defer func() {
+			if r := recover(); r != nil && !data.expPanic {
+				t.Errorf("got unexpected panic: '%v'", r)
+			}
+		}()
 		common.GetOSEnv = func(key string) string {
 			switch key {
 			case "BUILD_NUMBER":
@@ -565,14 +651,21 @@ func TestAcquire(t *testing.T) {
 		opCount := 0
 		if nil != data.existCluster {
 			opCount++
+			ac := &container.AddonsConfig{}
+			for _, addon := range data.addons {
+				if addon == "istio" {
+					ac.IstioConfig = &container.IstioConfig{Disabled: false}
+				}
+			}
 			fgc.operations.create(fakeProj, data.existCluster.Location, &container.CreateClusterRequest{
 				Cluster: &container.Cluster{
-					Name: data.existCluster.Name,
+					Name:         data.existCluster.Name,
+					AddonsConfig: ac,
 				},
 				ProjectId: fakeProj,
 			})
 			if data.kubeconfigSet {
-				fgc.Cluster = data.existCluster
+				fgc.Cluster, _ = fgc.operations.get(fakeProj, data.existCluster.Location, data.existCluster.Name)
 			}
 		}
 		fgc.Project = &fakeProj
@@ -586,19 +679,19 @@ func TestAcquire(t *testing.T) {
 			Region:        DefaultGKERegion,
 			Zone:          "",
 			BackupRegions: DefaultGKEBackupRegions,
+			Addons:        data.addons,
 		}
 		// Set NeedCleanup to false for easier testing, as it launches a
 		// goroutine
 		fgc.NeedCleanup = false
 		err := fgc.Acquire()
-		var gotName, gotLocation string
-		if nil != fgc.Cluster {
-			gotName = fgc.Cluster.Name
-			gotLocation = fgc.Cluster.Location
+		errMsg := fmt.Sprintf("testing acquiring cluster, with:\n\texisting cluster: '%+v'\n\tnext operations outcomes: '%v'\n\tkubeconfig set: '%v'\n\taddons: '%v'",
+			data.existCluster, data.nextOpStatus, data.kubeconfigSet, data.addons)
+		if !reflect.DeepEqual(err, data.expErr) {
+			t.Errorf("%s\nerror want: '%v'\nerror got: '%v'", errMsg, err, data.expErr)
 		}
-		if !reflect.DeepEqual(err, data.expErr) || data.expClusterName != gotName || data.expClusterLocation != gotLocation {
-			t.Errorf("testing acquiring cluster, with:\n\texisting cluster: '%v'\n\tnext operations outcomes: '%v'\nwant: cluster name - %q, location - %q, err - '%v'\ngot: cluster name - %q, location - %q, err - '%v'",
-				data.existCluster, data.nextOpStatus, data.expClusterName, data.expClusterLocation, data.expErr, gotName, gotLocation, err)
+		if dif := cmp.Diff(data.expCluster, fgc.Cluster); dif != "" {
+			t.Errorf("%s\nCluster got(+) is different from wanted(-)\n%v", errMsg, dif)
 		}
 	}
 }
@@ -717,14 +810,21 @@ func TestDelete(t *testing.T) {
 		}
 
 		err := fgc.Delete()
-		var clusterGot *container.Cluster
+		var gotCluster *container.Cluster
 		if nil != data.cluster {
-			clusterGot, _ = fgc.operations.get(fakeProj, data.cluster.Location, data.cluster.Name)
+			gotCluster, _ = fgc.operations.get(fakeProj, data.cluster.Location, data.cluster.Name)
 		}
 		gotBoskos := fgc.boskosOps.(*boskosFake.FakeBoskosClient).GetResources()
-		if !reflect.DeepEqual(err, data.expErr) || !reflect.DeepEqual(clusterGot, data.expCluster) || !reflect.DeepEqual(gotBoskos, data.expBoskos) {
-			t.Errorf("testing deleting cluster, with:\n\tIs Prow: '%v'\n\texisting cluster: '%v'\n\tboskos state: '%v'\nwant: boskos - '%v', cluster - '%v', err - '%v'\ngot: boskos - '%v', cluster - '%v', err - '%v'",
-				data.isProw, data.cluster, data.boskosState, data.expBoskos, data.expCluster, data.expErr, nil, clusterGot, err)
+		errMsg := fmt.Sprintf("testing deleting cluster, with:\n\tIs Prow: '%v'\n\texisting cluster: '%v'\n\tboskos state: '%v'",
+			data.isProw, data.cluster, data.boskosState)
+		if !reflect.DeepEqual(err, data.expErr) {
+			t.Errorf("%s\nerror want: '%v'\nerror got: '%v'", errMsg, err, data.expErr)
+		}
+		if dif := cmp.Diff(data.expCluster, gotCluster); dif != "" {
+			t.Errorf("%s\nCluster got(+) is different from wanted(-)\n%v", errMsg, dif)
+		}
+		if dif := cmp.Diff(data.expBoskos, gotBoskos); dif != "" {
+			t.Errorf("%s\nBoskos got(+) is different from wanted(-)\n%v", errMsg, dif)
 		}
 	}
 }