From 936e31ab6099619345383ba57bbd8c6f1f79c945 Mon Sep 17 00:00:00 2001
From: Jessica Yuen <im.jessicayuen@gmail.com>
Date: Fri, 23 Feb 2018 14:02:49 -0800
Subject: [PATCH] App spec to take a single destination

Currently the app.yaml spec takes a list of destinations. The change
exists to anticipate support for an environment supporting multiple
clusters or "destinations". The problem is, while the use case makes
sense for `apply`, and `delete`, it becomes ambiguous which cluster is
being referred to during commands such as `diff`. We've considered
specifying a cluster during a `diff`, however, ks currently doesn't have
a notion of a cluster identity. This change is to update the app.yaml to
take a single destination to more accurately represent the state of
things.

Signed-off-by: Jessica Yuen <im.jessicayuen@gmail.com>
---
 client/client.go                      |  9 ++--
 cmd/root.go                           |  5 +-
 integration/integration_suite_test.go |  8 ++-
 metadata/app/schema.go                | 40 +++++++--------
 metadata/app/schema_test.go           | 72 +++++++++++----------------
 metadata/environment.go               | 10 ++--
 metadata/environment_test.go          | 10 ++--
 pkg/kubecfg/env.go                    |  6 +--
 testdata/testapp/app.yaml             |  4 +-
 9 files changed, 67 insertions(+), 97 deletions(-)

diff --git a/client/client.go b/client/client.go
index dc5caf36..ae563045 100644
--- a/client/client.go
+++ b/client/client.go
@@ -184,8 +184,7 @@ func (c *Config) overrideCluster(envName string) error {
 		return err
 	}
 
-	// TODO support multi-cluster deployment.
-	server, err := str.NormalizeURL(env.Destinations[0].Server)
+	server, err := str.NormalizeURL(env.Destination.Server)
 	if err != nil {
 		return err
 	}
@@ -197,11 +196,11 @@ func (c *Config) overrideCluster(envName string) error {
 			c.Overrides.Context.Cluster = clusterName
 		}
 		if c.Overrides.Context.Namespace == "" {
-			log.Debugf("Overwriting --namespace flag with '%s'", env.Destinations[0].Namespace)
-			c.Overrides.Context.Namespace = env.Destinations[0].Namespace
+			log.Debugf("Overwriting --namespace flag with '%s'", env.Destination.Namespace)
+			c.Overrides.Context.Namespace = env.Destination.Namespace
 		}
 		return nil
 	}
 
-	return fmt.Errorf("Attempting to deploy to environment '%s' at '%s', but cannot locate a server at that address", envName, env.Destinations[0].Server)
+	return fmt.Errorf("Attempting to deploy to environment '%s' at '%s', but cannot locate a server at that address", envName, env.Destination.Server)
 }
diff --git a/cmd/root.go b/cmd/root.go
index 06faf1bd..97970a34 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -412,15 +412,14 @@ func importEnv(manager metadata.Manager, env string) (string, error) {
 		return "", fmt.Errorf("Environment '%s' does not exist in app.yaml", env)
 	}
 
-	// TODO pass namespace and server as params when ks supports multi-cluster deployment
 	type EnvironmentSpec struct {
 		Server    string `json:"server"`
 		Namespace string `json:"namespace"`
 	}
 
 	toMarshal := &EnvironmentSpec{
-		Server:    spec.Destinations[0].Server,
-		Namespace: spec.Destinations[0].Namespace,
+		Server:    spec.Destination.Server,
+		Namespace: spec.Destination.Namespace,
 	}
 
 	marshalled, err := json.Marshal(toMarshal)
diff --git a/integration/integration_suite_test.go b/integration/integration_suite_test.go
index d848136e..f44a534f 100644
--- a/integration/integration_suite_test.go
+++ b/integration/integration_suite_test.go
@@ -94,11 +94,9 @@ func runKsonnetWith(flags []string, host, ns string) error {
 		APIVersion: "0.0.1",
 		Environments: app.EnvironmentSpecs{
 			"default": &app.EnvironmentSpec{
-				Destinations: app.EnvironmentDestinationSpecs{
-					&app.EnvironmentDestinationSpec{
-						Namespace: ns,
-						Server:    host,
-					},
+				Destination: &app.EnvironmentDestinationSpec{
+					Namespace: ns,
+					Server:    host,
 				},
 				KubernetesVersion: "v1.7.0",
 			},
diff --git a/metadata/app/schema.go b/metadata/app/schema.go
index 7cd72781..9de648a2 100644
--- a/metadata/app/schema.go
+++ b/metadata/app/schema.go
@@ -75,33 +75,29 @@ type RegistryRefSpecs map[string]*RegistryRefSpec
 type EnvironmentSpecs map[string]*EnvironmentSpec
 
 // EnvironmentSpec contains the specification for ksonnet environments.
-//
-// KubernetesVersion: The Kubernetes version the target cluster is running on.
-// Path:              The relative path containing metadata for this environment,
-//                    rooted at the directory 'environments'.
-// Destinations:      One or more cluster addresses that this environment
-//                    points to.
-// Targets:           The relative component paths that this environment wishes
-//                    to deploy onto it's destinations.
 type EnvironmentSpec struct {
-	Name              string                      `json:"-"`
-	KubernetesVersion string                      `json:"k8sVersion"`
-	Path              string                      `json:"path"`
-	Destinations      EnvironmentDestinationSpecs `json:"destinations"`
-	Targets           []string                    `json:"targets,omitempty"`
+	// Name is the user defined name of an environment
+	Name string `json:"-"`
+	// KubernetesVersion is the kubernetes version the targetted cluster is
+	// running on.
+	KubernetesVersion string `json:"k8sVersion"`
+	// Path is the relative project path containing metadata for this
+	// environment.
+	Path string `json:"path"`
+	// Destination stores the cluster address that this environment points to.
+	Destination *EnvironmentDestinationSpec `json:"destination"`
+	// Targets contain the relative component paths that this environment
+	// wishes to deploy on it's destination.
+	Targets []string `json:"targets,omitempty"`
 }
 
-// EnvironmentDestinationSpecs contains one or more EnvironmentDestinationSpec.
-type EnvironmentDestinationSpecs []*EnvironmentDestinationSpec
-
 // EnvironmentDestinationSpec contains the specification for the cluster
-// addresses that the environment points to.
-//
-// Server:    The Kubernetes server that the cluster is running on.
-// Namespace: The namespace of the Kubernetes server that targets should
-//            be deployed to. This is "default", by default.
+// address that the environment points to.
 type EnvironmentDestinationSpec struct {
-	Server    string `json:"server"`
+	// Server is the Kubernetes server that the cluster is running on.
+	Server string `json:"server"`
+	// Namespace is the namespace of the Kubernetes server that targets should
+	// be deployed to. This is "default", if not specified.
 	Namespace string `json:"namespace"`
 }
 
diff --git a/metadata/app/schema_test.go b/metadata/app/schema_test.go
index 980dc03c..4b6cd394 100644
--- a/metadata/app/schema_test.go
+++ b/metadata/app/schema_test.go
@@ -36,11 +36,9 @@ func makeSimpleRefSpec(name, protocol, uri, version string) *RegistryRefSpec {
 func makeSimpleEnvironmentSpec(name, namespace, server, k8sVersion string) *EnvironmentSpec {
 	return &EnvironmentSpec{
 		Name: name,
-		Destinations: EnvironmentDestinationSpecs{
-			&EnvironmentDestinationSpec{
-				Namespace: namespace,
-				Server:    server,
-			},
+		Destination: &EnvironmentDestinationSpec{
+			Namespace: namespace,
+			Server:    server,
 		},
 		KubernetesVersion: k8sVersion,
 	}
@@ -175,11 +173,9 @@ func TestGetEnvironmentSpecs(t *testing.T) {
 	example1 := Spec{
 		Environments: EnvironmentSpecs{
 			"dev": &EnvironmentSpec{
-				Destinations: EnvironmentDestinationSpecs{
-					&EnvironmentDestinationSpec{
-						Namespace: "default",
-						Server:    "http://example.com",
-					},
+				Destination: &EnvironmentDestinationSpec{
+					Namespace: "default",
+					Server:    "http://example.com",
 				},
 				KubernetesVersion: "1.8.0",
 			},
@@ -207,11 +203,9 @@ func TestGetEnvironmentSpecSuccess(t *testing.T) {
 	example1 := Spec{
 		Environments: EnvironmentSpecs{
 			env: &EnvironmentSpec{
-				Destinations: EnvironmentDestinationSpecs{
-					&EnvironmentDestinationSpec{
-						Namespace: namespace,
-						Server:    server,
-					},
+				Destination: &EnvironmentDestinationSpec{
+					Namespace: namespace,
+					Server:    server,
 				},
 				KubernetesVersion: k8sVersion,
 			},
@@ -223,8 +217,8 @@ func TestGetEnvironmentSpecSuccess(t *testing.T) {
 		t.Errorf("Expected environments to contain '%s'", env)
 	}
 
-	if len(r1.Destinations) != 1 || r1.Destinations[0].Namespace != namespace ||
-		r1.Destinations[0].Server != server || r1.KubernetesVersion != k8sVersion {
+	if r1.Destination.Namespace != namespace ||
+		r1.Destination.Server != server || r1.KubernetesVersion != k8sVersion {
 		t.Errorf("Environment did not add correct values:\n%s", r1)
 	}
 }
@@ -233,11 +227,9 @@ func TestGetEnvironmentSpecFailure(t *testing.T) {
 	example1 := Spec{
 		Environments: EnvironmentSpecs{
 			"dev": &EnvironmentSpec{
-				Destinations: EnvironmentDestinationSpecs{
-					&EnvironmentDestinationSpec{
-						Namespace: "default",
-						Server:    "http://example.com",
-					},
+				Destination: &EnvironmentDestinationSpec{
+					Namespace: "default",
+					Server:    "http://example.com",
 				},
 				KubernetesVersion: "1.8.0",
 			},
@@ -268,8 +260,8 @@ func TestAddEnvironmentSpecSuccess(t *testing.T) {
 	}
 
 	r1, ok1 := example1.GetEnvironmentSpec(env)
-	if !ok1 || len(r1.Destinations) != 1 || r1.Destinations[0].Namespace != namespace ||
-		r1.Destinations[0].Server != server || r1.KubernetesVersion != k8sVersion {
+	if !ok1 || r1.Destination.Namespace != namespace ||
+		r1.Destination.Server != server || r1.KubernetesVersion != k8sVersion {
 		t.Errorf("Environment did not add correct values:\n%s", r1)
 	}
 }
@@ -286,11 +278,9 @@ func TestAddEnvironmentSpecFailure(t *testing.T) {
 	example1 := Spec{
 		Environments: EnvironmentSpecs{
 			envName1: &EnvironmentSpec{
-				Destinations: EnvironmentDestinationSpecs{
-					&EnvironmentDestinationSpec{
-						Namespace: namespace,
-						Server:    server,
-					},
+				Destination: &EnvironmentDestinationSpec{
+					Namespace: namespace,
+					Server:    server,
 				},
 				KubernetesVersion: k8sVersion,
 			},
@@ -312,11 +302,9 @@ func TestDeleteEnvironmentSpec(t *testing.T) {
 	example1 := Spec{
 		Environments: EnvironmentSpecs{
 			"dev": &EnvironmentSpec{
-				Destinations: EnvironmentDestinationSpecs{
-					&EnvironmentDestinationSpec{
-						Namespace: "default",
-						Server:    "http://example.com",
-					},
+				Destination: &EnvironmentDestinationSpec{
+					Namespace: "default",
+					Server:    "http://example.com",
 				},
 				KubernetesVersion: "1.8.0",
 			},
@@ -337,11 +325,9 @@ func TestUpdateEnvironmentSpec(t *testing.T) {
 	example1 := Spec{
 		Environments: EnvironmentSpecs{
 			"dev": &EnvironmentSpec{
-				Destinations: EnvironmentDestinationSpecs{
-					&EnvironmentDestinationSpec{
-						Namespace: "default",
-						Server:    "http://example.com",
-					},
+				Destination: &EnvironmentDestinationSpec{
+					Namespace: "default",
+					Server:    "http://example.com",
 				},
 				KubernetesVersion: "1.8.0",
 			},
@@ -350,11 +336,9 @@ func TestUpdateEnvironmentSpec(t *testing.T) {
 
 	example2 := EnvironmentSpec{
 		Name: "foo",
-		Destinations: EnvironmentDestinationSpecs{
-			&EnvironmentDestinationSpec{
-				Namespace: "foo",
-				Server:    "http://example.com",
-			},
+		Destination: &EnvironmentDestinationSpec{
+			Namespace: "foo",
+			Server:    "http://example.com",
 		},
 		KubernetesVersion: "1.8.0",
 	}
diff --git a/metadata/environment.go b/metadata/environment.go
index d23d7b71..14f86701 100644
--- a/metadata/environment.go
+++ b/metadata/environment.go
@@ -115,11 +115,9 @@ func (m *manager) CreateEnvironment(name, server, namespace, k8sSpecFlag string)
 	err = appSpec.AddEnvironmentSpec(&app.EnvironmentSpec{
 		Name: name,
 		Path: name,
-		Destinations: app.EnvironmentDestinationSpecs{
-			&app.EnvironmentDestinationSpec{
-				Server:    server,
-				Namespace: namespace,
-			},
+		Destination: &app.EnvironmentDestinationSpec{
+			Server:    server,
+			Namespace: namespace,
 		},
 		KubernetesVersion: libManager.K8sVersion,
 	})
@@ -232,7 +230,7 @@ func (m *manager) SetEnvironment(name, desiredName string) error {
 
 	err = appSpec.UpdateEnvironmentSpec(name, &app.EnvironmentSpec{
 		Name:              desiredName,
-		Destinations:      current.Destinations,
+		Destination:       current.Destination,
 		KubernetesVersion: current.KubernetesVersion,
 		Targets:           current.Targets,
 		Path:              desiredName,
diff --git a/metadata/environment_test.go b/metadata/environment_test.go
index 017ef0ef..572c3439 100644
--- a/metadata/environment_test.go
+++ b/metadata/environment_test.go
@@ -81,11 +81,9 @@ func mockEnvironmentsWith(t *testing.T, appName string, envNames []string) *mana
 		appSpec.AddEnvironmentSpec(&app.EnvironmentSpec{
 			Name: env,
 			Path: env,
-			Destinations: app.EnvironmentDestinationSpecs{
-				&app.EnvironmentDestinationSpec{
-					Server:    mockAPIServer,
-					Namespace: mockNamespace,
-				},
+			Destination: &app.EnvironmentDestinationSpec{
+				Server:    mockAPIServer,
+				Namespace: mockNamespace,
 			},
 		})
 		m.WriteAppSpec(appSpec)
@@ -165,7 +163,7 @@ func TestGetEnvironments(t *testing.T) {
 		t.Fatalf("Expected env name to be '%s', got '%s'", mockEnvName, name)
 	}
 
-	server := envs[mockEnvName].Destinations[0].Server
+	server := envs[mockEnvName].Destination.Server
 	if server != mockAPIServer {
 		t.Fatalf("Expected env server to be %s, got %s", mockAPIServer, server)
 	}
diff --git a/pkg/kubecfg/env.go b/pkg/kubecfg/env.go
index c990fda8..5338c3df 100644
--- a/pkg/kubecfg/env.go
+++ b/pkg/kubecfg/env.go
@@ -83,7 +83,7 @@ func (c *EnvListCmd) Run(out io.Writer) error {
 		return err
 	}
 
-	envs := make([]app.EnvironmentSpec, len(envMap))
+	var envs []app.EnvironmentSpec
 	for _, e := range envMap {
 		envs = append(envs, *e)
 	}
@@ -101,9 +101,7 @@ func (c *EnvListCmd) Run(out io.Writer) error {
 	}
 
 	for _, env := range envs {
-		for _, dest := range env.Destinations {
-			rows = append(rows, []string{env.Name, env.KubernetesVersion, dest.Namespace, dest.Server})
-		}
+		rows = append(rows, []string{env.Name, env.KubernetesVersion, env.Destination.Namespace, env.Destination.Server})
 	}
 
 	formattedEnvsList, err := str.PadRows(rows)
diff --git a/testdata/testapp/app.yaml b/testdata/testapp/app.yaml
index 0f8e9878..9dc2c5f3 100644
--- a/testdata/testapp/app.yaml
+++ b/testdata/testapp/app.yaml
@@ -10,8 +10,8 @@ registries:
     uri: github.com/ksonnet/parts/tree/test-reg/incubator
 environments:
   default:
-    destinations:
-    - namespace: foo
+    destination:
+      namespace: foo
       server: foo
     k8sVersion: "v1.8.1"
     path: default
-- 
GitLab