Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collect namespaces of deployed resources. #2640

Merged
merged 7 commits into from
Aug 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -79,6 +79,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
)

Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
23 changes: 22 additions & 1 deletion pkg/skaffold/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
28 changes: 21 additions & 7 deletions pkg/skaffold/deploy/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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...)
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/deploy/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
26 changes: 16 additions & 10 deletions pkg/skaffold/deploy/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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."))
tejal29 marked this conversation as resolved.
Show resolved Hide resolved
}

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.
Expand Down
124 changes: 124 additions & 0 deletions pkg/skaffold/deploy/kubectl/namespace_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
Loading