Merge pull request #2640 from tejal29/collect_namespaces

Collect namespaces of deployed resources.

go.mod

@@ -27,7 +27,7 @@ require (
 	github.com/gogo/protobuf v1.1.1 // indirect
 	github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
 	github.com/golang/protobuf v1.3.2
-	github.com/google/go-cmp v0.3.0
+	github.com/google/go-cmp v0.3.1
 	github.com/google/go-containerregistry v0.0.0-20190717132004-e8c6a4993fa7
 	github.com/google/go-github v17.0.0+incompatible
 	github.com/google/go-querystring v1.0.0 // indirect
@@ -80,6 +80,7 @@ require (
 	k8s.io/apimachinery v0.0.0-20190620073744-d16981aedf33
 	k8s.io/client-go v0.0.0-20190620074045-585a16d2e773
 	k8s.io/kubectl v0.0.0-20190622051205-955b067cc6d3
+	k8s.io/utils v0.0.0-20190221042446-c2654d5206da
 	knative.dev/pkg v0.0.0-20190730155243-972acd413fb9 // indirect
 )
 

go.sum

@@ -98,6 +98,8 @@ github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ
 github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M=
 github.com/google/go-cmp v0.3.0 h1:crn/baboCvb5fXaQ0IJ1SGTsTVrWpDsCWC8EGETZijY=
 github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
+github.com/google/go-cmp v0.3.1 h1:Xye71clBPdm5HgqGwUkwhbynsUJZhDbS20FvLhQ2izg=
+github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/go-containerregistry v0.0.0-20190717132004-e8c6a4993fa7 h1:Tx9G0xLagQdDraS6GdVgRLFOXWURkSnC8RbszNCqUUc=
 github.com/google/go-containerregistry v0.0.0-20190717132004-e8c6a4993fa7/go.mod h1:yZAFP63pRshzrEYLXLGPmUt0Ay+2zdjmMN1loCnRLUk=
 github.com/google/go-github v17.0.0+incompatible h1:N0LgJ1j65A7kfXrZnUDaYCs/Sf4rEjNlfyDHW9dolSY=

pkg/skaffold/deploy/deploy.go

@@ -30,7 +30,7 @@ type Deployer interface {
 
 	// Deploy should ensure that the build results are deployed to the Kubernetes
 	// cluster.
-	Deploy(context.Context, io.Writer, []build.Artifact, []Labeller) error
+	Deploy(context.Context, io.Writer, []build.Artifact, []Labeller) *Result
 
 	// Dependencies returns a list of files that the deployer depends on.
 	// In dev mode, a redeploy will be triggered
@@ -39,3 +39,24 @@ type Deployer interface {
 	// Cleanup deletes what was deployed by calling Deploy.
 	Cleanup(context.Context, io.Writer) error
 }
+
+type Result struct {
+	err        error
+	namespaces []string
+}
+
+func NewDeployErrorResult(err error) *Result {
+	return &Result{err: err}
+}
+
+func NewDeploySuccessResult(namespaces []string) *Result {
+	return &Result{namespaces: namespaces}
+}
+
+func (d *Result) Namespaces() []string {
+	return d.namespaces
+}
+
+func (d *Result) GetError() error {
+	return d.err
+}

pkg/skaffold/deploy/helm.go

@@ -29,6 +29,11 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/mitchellh/go-homedir"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+	"gopkg.in/yaml.v2"
+
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
@@ -37,10 +42,6 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
-	homedir "github.com/mitchellh/go-homedir"
-	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
-	yaml "gopkg.in/yaml.v2"
 )
 
 type HelmDeployer struct {
@@ -70,18 +71,25 @@ func (h *HelmDeployer) Labels() map[string]string {
 	}
 }
 
-func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller) error {
+func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller) *Result {
 	var dRes []Artifact
 
 	event.DeployInProgress()
+	nsMap := map[string]struct{}{}
 
 	for _, r := range h.Releases {
 		results, err := h.deployRelease(ctx, out, r, builds)
 		if err != nil {
 			releaseName, _ := evaluateReleaseName(r.Name)
 
 			event.DeployFailed(err)
-			return errors.Wrapf(err, "deploying %s", releaseName)
+			return NewDeployErrorResult(errors.Wrapf(err, "deploying %s", releaseName))
+		}
+		// collect namespaces
+		for _, r := range results {
+			if trimmed := strings.TrimSpace(r.Namespace); trimmed != "" {
+				nsMap[trimmed] = struct{}{}
+			}
 		}
 
 		dRes = append(dRes, results...)
@@ -92,7 +100,13 @@ func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build
 	labels := merge(labellers...)
 	labelDeployResults(labels, dRes)
 
-	return nil
+	// Collect namespaces in a string
+	namespaces := make([]string, 0, len(nsMap))
+	for ns := range nsMap {
+		namespaces = append(namespaces, ns)
+	}
+
+	return NewDeploySuccessResult(namespaces)
 }
 
 func (h *HelmDeployer) Dependencies() ([]string, error) {

pkg/skaffold/deploy/helm_test.go

@@ -480,7 +480,7 @@ func TestHelmDeploy(t *testing.T) {
 			t.Override(&util.DefaultExecCommand, test.cmd)
 
 			event.InitializeState(test.runContext.Cfg.Build)
-			err := NewHelmDeployer(test.runContext).Deploy(context.Background(), ioutil.Discard, test.builds, nil)
+			err := NewHelmDeployer(test.runContext).Deploy(context.Background(), ioutil.Discard, test.builds, nil).GetError()
 
 			t.CheckError(test.shouldErr, err)
 		})

pkg/skaffold/deploy/kubectl.go

@@ -71,7 +71,7 @@ func (k *KubectlDeployer) Labels() map[string]string {
 
 // Deploy templates the provided manifests with a simple `find and replace` and
 // runs `kubectl apply` on those manifests
-func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller) error {
+func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller) *Result {
 	color.Default.Fprintln(out, "kubectl client version:", k.kubectl.Version(ctx))
 	if err := k.kubectl.CheckVersion(ctx); err != nil {
 		color.Default.Fprintln(out, err)
@@ -80,13 +80,13 @@ func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []bu
 	manifests, err := k.readManifests(ctx)
 	if err != nil {
 		event.DeployFailed(err)
-		return errors.Wrap(err, "reading manifests")
+		return NewDeployErrorResult(errors.Wrap(err, "reading manifests"))
 	}
 
 	for _, m := range k.RemoteManifests {
 		manifest, err := k.readRemoteManifest(ctx, m)
 		if err != nil {
-			return errors.Wrap(err, "get remote manifests")
+			return NewDeployErrorResult(errors.Wrap(err, "get remote manifests"))
 		}
 
 		manifests = append(manifests, manifest)
@@ -95,45 +95,51 @@ func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []bu
 	if len(k.originalImages) == 0 {
 		k.originalImages, err = manifests.GetImages()
 		if err != nil {
-			return errors.Wrap(err, "get images from manifests")
+			return NewDeployErrorResult(errors.Wrap(err, "get images from manifests"))
 		}
 	}
 
 	logrus.Debugln("manifests", manifests.String())
 
 	if len(manifests) == 0 {
-		return nil
+		return NewDeploySuccessResult(nil)
 	}
 
 	event.DeployInProgress()
 
+	namespaces, err := manifests.CollectNamespaces()
+	if err != nil {
+		event.DeployInfoEvent(errors.Wrap(err, "could not fetch deployed resource namespace. "+
+			"This might cause port-forward and deploy health-check to fail."))
+	}
+
 	manifests, err = manifests.ReplaceImages(builds, k.defaultRepo)
 	if err != nil {
 		event.DeployFailed(err)
-		return errors.Wrap(err, "replacing images in manifests")
+		return NewDeployErrorResult(errors.Wrap(err, "replacing images in manifests"))
 	}
 
 	manifests, err = manifests.SetLabels(merge(labellers...))
 	if err != nil {
 		event.DeployFailed(err)
-		return errors.Wrap(err, "setting labels in manifests")
+		return NewDeployErrorResult(errors.Wrap(err, "setting labels in manifests"))
 	}
 
 	for _, transform := range manifestTransforms {
 		manifests, err = transform(manifests, builds, k.insecureRegistries)
 		if err != nil {
 			event.DeployFailed(err)
-			return errors.Wrap(err, "unable to transform manifests")
+			return NewDeployErrorResult(errors.Wrap(err, "unable to transform manifests"))
 		}
 	}
 
 	if err := k.kubectl.Apply(ctx, out, manifests); err != nil {
 		event.DeployFailed(err)
-		return errors.Wrap(err, "kubectl error")
+		return NewDeployErrorResult(errors.Wrap(err, "kubectl error"))
 	}
 
 	event.DeployComplete()
-	return nil
+	return NewDeploySuccessResult(namespaces)
 }
 
 // Cleanup deletes what was deployed by calling Deploy.

pkg/skaffold/deploy/kubectl/namespace_test.go

@@ -0,0 +1,124 @@
+/*
+Copyright 2019 The Skaffold 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 kubectl
+
+import (
+	"testing"
+
+	"github.com/GoogleContainerTools/skaffold/testutil"
+)
+
+func TestCollectLabels(t *testing.T) {
+	tests := []struct {
+		description string
+		manifests   ManifestList
+		expected    []string
+	}{
+		{
+			description: "single manifest in the list",
+			manifests: ManifestList{[]byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: getting-started
+  namespace: test
+spec:
+  containers:
+  - image: gcr.io/k8s-skaffold/example
+    name: example
+`)},
+			expected: []string{"test"},
+		}, {
+			description: "multiple manifest in the list with different namespaces",
+			manifests: ManifestList{[]byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: foo
+  namespace: test-foo
+spec:
+  containers:
+  - image: gcr.io/k8s-skaffold/example
+    name: example`), []byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: bar
+  namespace: test-bar
+spec:
+  containers:
+  - image: gcr.io/k8s-skaffold/example
+    name: example
+`)},
+			expected: []string{"test-bar", "test-foo"},
+		}, {
+			description: "multiple manifest but same namespace",
+			manifests: ManifestList{[]byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: foo
+  namespace: test
+spec:
+  containers:
+  - image: gcr.io/k8s-skaffold/example
+    name: example`), []byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: bar
+  namespace: test
+spec:
+  containers:
+  - image: gcr.io/k8s-skaffold/example
+    name: example
+`)},
+			expected: []string{"test"},
+		}, {
+			description: "multiple manifest but no namespace",
+			manifests: ManifestList{[]byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: foo
+spec:
+  containers:
+  - image: gcr.io/k8s-skaffold/example
+    name: example`), []byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: bar
+spec:
+  containers:
+  - image: gcr.io/k8s-skaffold/example
+    name: example
+`)},
+			expected: []string{},
+		}, {
+			description: "empty manifest",
+			manifests:   ManifestList{[]byte(``)},
+			expected:    []string{},
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			actual, err := test.manifests.CollectNamespaces()
+			t.CheckErrorAndDeepEqual(false, err, test.expected, actual)
+		})
+	}
+}