From 160cfa11fce8cc70537f5beb294932079b8f7bee Mon Sep 17 00:00:00 2001 From: David Gageot Date: Tue, 29 Jan 2019 02:09:49 +0100 Subject: [PATCH] Use kubectl to read the manifests (#1451) * Use kubectl to read the manifests + Adds support for json manifests + Should help when kustomize is built into kubectl Fixes #921 Signed-off-by: David Gageot --- pkg/skaffold/deploy/kubectl.go | 56 +------- pkg/skaffold/deploy/kubectl/cli.go | 40 +++++- pkg/skaffold/deploy/kubectl/manifests.go | 9 +- pkg/skaffold/deploy/kubectl/manifests_test.go | 61 ++++++++ pkg/skaffold/deploy/kubectl_test.go | 130 +++++++++--------- 5 files changed, 171 insertions(+), 125 deletions(-) create mode 100644 pkg/skaffold/deploy/kubectl/manifests_test.go diff --git a/pkg/skaffold/deploy/kubectl.go b/pkg/skaffold/deploy/kubectl.go index 83452a1f2f5..8c435d7674e 100644 --- a/pkg/skaffold/deploy/kubectl.go +++ b/pkg/skaffold/deploy/kubectl.go @@ -21,8 +21,6 @@ import ( "bytes" "context" "io" - "io/ioutil" - "strings" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/color" @@ -150,58 +148,14 @@ func parseManifestsForDeploys(namespace string, manifests kubectl.ManifestList) // readManifests reads the manifests to deploy/delete. func (k *KubectlDeployer) readManifests(ctx context.Context) (kubectl.ManifestList, error) { - files, err := k.manifestFiles(k.Manifests) + manifests, err := k.Dependencies() if err != nil { - return nil, errors.Wrap(err, "expanding user manifest list") + return nil, errors.Wrap(err, "listing manifests") } - var manifests kubectl.ManifestList - for _, manifest := range files { - buf, err := ioutil.ReadFile(manifest) - if err != nil { - return nil, errors.Wrap(err, "reading manifest") - } - - manifests.Append(buf) - } - - for _, manifest := range k.Manifests { - if util.IsURL(manifest) { - buf, err := util.Download(manifest) - if err != nil { - return nil, errors.Wrap(err, "downloading manifest") - } - manifests.Append(buf) - } - } - - for _, m := range k.RemoteManifests { - manifest, err := k.readRemoteManifest(ctx, m) - if err != nil { - return nil, errors.Wrap(err, "get remote manifests") - } - - manifests = append(manifests, manifest) - } - - logrus.Debugln("manifests", manifests.String()) - - return manifests, nil -} - -func (k *KubectlDeployer) readRemoteManifest(ctx context.Context, name string) ([]byte, error) { - var args []string - if parts := strings.Split(name, ":"); len(parts) > 1 { - args = append(args, "--namespace", parts[0]) - name = parts[1] - } - args = append(args, name, "-o", "yaml") - - var manifest bytes.Buffer - err := k.kubectl.Run(ctx, nil, &manifest, "get", nil, args...) - if err != nil { - return nil, errors.Wrap(err, "getting manifest") + if len(manifests) == 0 { + return kubectl.ManifestList{}, nil } - return manifest.Bytes(), nil + return k.kubectl.ReadManifests(ctx, manifests) } diff --git a/pkg/skaffold/deploy/kubectl/cli.go b/pkg/skaffold/deploy/kubectl/cli.go index 4980e12dcfb..8059feb58df 100644 --- a/pkg/skaffold/deploy/kubectl/cli.go +++ b/pkg/skaffold/deploy/kubectl/cli.go @@ -67,8 +67,41 @@ func (c *CLI) Apply(ctx context.Context, out io.Writer, manifests ManifestList) return updated, nil } +// ReadManifests reads a list of manifests in yaml format. +func (c *CLI) ReadManifests(ctx context.Context, manifests []string) (ManifestList, error) { + var list []string + for _, manifest := range manifests { + list = append(list, "-f", manifest) + } + + args := c.args("create", []string{"--dry-run", "-oyaml"}, list...) + + cmd := exec.CommandContext(ctx, "kubectl", args...) + buf, err := util.RunCmdOut(cmd) + if err != nil { + return nil, errors.Wrap(err, "kubectl create") + } + + var manifestList ManifestList + manifestList.Append(buf) + logrus.Debugln("manifests", manifestList.String()) + + return manifestList, nil +} + // Run shells out kubectl CLI. func (c *CLI) Run(ctx context.Context, in io.Reader, out io.Writer, command string, commandFlags []string, arg ...string) error { + args := c.args(command, commandFlags, arg...) + + cmd := exec.CommandContext(ctx, "kubectl", args...) + cmd.Stdin = in + cmd.Stdout = out + cmd.Stderr = out + + return util.RunCmd(cmd) +} + +func (c *CLI) args(command string, commandFlags []string, arg ...string) []string { args := []string{"--context", c.KubeContext} if c.Namespace != "" { args = append(args, "--namespace", c.Namespace) @@ -78,10 +111,5 @@ func (c *CLI) Run(ctx context.Context, in io.Reader, out io.Writer, command stri args = append(args, commandFlags...) args = append(args, arg...) - cmd := exec.CommandContext(ctx, "kubectl", args...) - cmd.Stdin = in - cmd.Stdout = out - cmd.Stderr = out - - return util.RunCmd(cmd) + return args } diff --git a/pkg/skaffold/deploy/kubectl/manifests.go b/pkg/skaffold/deploy/kubectl/manifests.go index 83bebb45099..3de0fe00c88 100644 --- a/pkg/skaffold/deploy/kubectl/manifests.go +++ b/pkg/skaffold/deploy/kubectl/manifests.go @@ -19,6 +19,7 @@ package kubectl import ( "bytes" "io" + "regexp" "strings" ) @@ -38,7 +39,13 @@ func (l *ManifestList) String() string { // Append appends the yaml manifests defined in the given buffer. func (l *ManifestList) Append(buf []byte) { - parts := bytes.Split(buf, []byte("\n---")) + // `kubectl create --dry-run -oyaml` outputs manifests without --- separator + // But we can rely on `apiVersion:` being here as a "separator". + buf = regexp. + MustCompile("\n(|---\n)apiVersion: "). + ReplaceAll(buf, []byte("\n---\napiVersion: ")) + + parts := bytes.Split(buf, []byte("\n---\n")) for _, part := range parts { *l = append(*l, part) } diff --git a/pkg/skaffold/deploy/kubectl/manifests_test.go b/pkg/skaffold/deploy/kubectl/manifests_test.go new file mode 100644 index 00000000000..07fd8035bd2 --- /dev/null +++ b/pkg/skaffold/deploy/kubectl/manifests_test.go @@ -0,0 +1,61 @@ +/* +Copyright 2018 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" +) + +const pod1 = `apiVersion: v1 +kind: Pod +metadata: + name: leeroy-web +spec: + containers: + - name: leeroy-web + image: leeroy-web` + +const pod2 = `apiVersion: v1 +kind: Pod +metadata: + name: leeroy-app +spec: + containers: + - name: leeroy-app + image: leeroy-app` + +func TestAppend(t *testing.T) { + var manifests ManifestList + + manifests.Append([]byte(pod1 + "\n---\n" + pod2)) + + testutil.CheckDeepEqual(t, 2, len(manifests)) + testutil.CheckDeepEqual(t, pod1, string(manifests[0])) + testutil.CheckDeepEqual(t, pod2, string(manifests[1])) +} + +func TestAppendWithoutSeperator(t *testing.T) { + var manifests ManifestList + + manifests.Append([]byte(pod1 + "\n" + pod2)) + + testutil.CheckDeepEqual(t, 2, len(manifests)) + testutil.CheckDeepEqual(t, pod1, string(manifests[0])) + testutil.CheckDeepEqual(t, pod2, string(manifests[1])) +} diff --git a/pkg/skaffold/deploy/kubectl_test.go b/pkg/skaffold/deploy/kubectl_test.go index 2fb6c2bfb66..41932c09069 100644 --- a/pkg/skaffold/deploy/kubectl_test.go +++ b/pkg/skaffold/deploy/kubectl_test.go @@ -43,7 +43,7 @@ spec: - name: leeroy-web image: leeroy-web` -const deploymentAppYaml = `apiVersion: v1 +const deploymentAppYAML = `apiVersion: v1 kind: Pod metadata: name: leeroy-app @@ -53,6 +53,12 @@ spec: image: leeroy-app` func TestKubectlDeploy(t *testing.T) { + tmpDir, cleanup := testutil.NewTempDir(t) + defer cleanup() + + tmpDir.Write("deployment.yaml", deploymentWebYAML) + tmpDir.Write("empty.ignored", "") + var tests = []struct { description string cfg *latest.KubectlDeploy @@ -61,30 +67,23 @@ func TestKubectlDeploy(t *testing.T) { shouldErr bool }{ { - description: "parameter mismatch", - shouldErr: true, - cfg: &latest.KubectlDeploy{ - Manifests: []string{"deployment.yaml"}, - }, - builds: []build.Artifact{ - { - ImageName: "leeroy-web", - Tag: "leeroy-web:v1", - }, - }, + description: "no manifest", + cfg: &latest.KubectlDeploy{}, + command: testutil.NewFakeCmd(t).WithRunOut("kubectl version --client -ojson", kubectlVersion), }, { description: "missing manifest file", - shouldErr: true, cfg: &latest.KubectlDeploy{ - Manifests: []string{"deployment.yaml"}, + Manifests: []string{"missing.yaml"}, }, - builds: []build.Artifact{ - { - ImageName: "leeroy-web", - Tag: "leeroy-web:123", - }, + command: testutil.NewFakeCmd(t).WithRunOut("kubectl version --client -ojson", kubectlVersion), + }, + { + description: "ignore non-manifest", + cfg: &latest.KubectlDeploy{ + Manifests: []string{"*.ignored"}, }, + command: testutil.NewFakeCmd(t).WithRunOut("kubectl version --client -ojson", kubectlVersion), }, { description: "deploy success", @@ -93,33 +92,30 @@ func TestKubectlDeploy(t *testing.T) { }, command: testutil.NewFakeCmd(t). WithRunOut("kubectl version --client -ojson", kubectlVersion). + WithRunOut("kubectl --context kubecontext --namespace testNamespace create --dry-run -oyaml -f "+tmpDir.Path("deployment.yaml"), deploymentWebYAML). WithRun("kubectl --context kubecontext --namespace testNamespace apply --force -f -"), - builds: []build.Artifact{ - { - ImageName: "leeroy-web", - Tag: "leeroy-web:123", - }, - }, + builds: []build.Artifact{{ + ImageName: "leeroy-web", + Tag: "leeroy-web:123", + }}, }, { description: "deploy command error", - shouldErr: true, cfg: &latest.KubectlDeploy{ Manifests: []string{"deployment.yaml"}, }, command: testutil.NewFakeCmd(t). WithRunOut("kubectl version --client -ojson", kubectlVersion). + WithRunOut("kubectl --context kubecontext --namespace testNamespace create --dry-run -oyaml -f "+tmpDir.Path("deployment.yaml"), deploymentWebYAML). WithRunErr("kubectl --context kubecontext --namespace testNamespace apply --force -f -", fmt.Errorf("")), - builds: []build.Artifact{ - { - ImageName: "leeroy-web", - Tag: "leeroy-web:123", - }, - }, + builds: []build.Artifact{{ + ImageName: "leeroy-web", + Tag: "leeroy-web:123", + }}, + shouldErr: true, }, { description: "additional flags", - shouldErr: true, cfg: &latest.KubectlDeploy{ Manifests: []string{"deployment.yaml"}, Flags: latest.KubectlFlags{ @@ -130,27 +126,20 @@ func TestKubectlDeploy(t *testing.T) { }, command: testutil.NewFakeCmd(t). WithRunOut("kubectl version --client -ojson", kubectlVersion). + WithRunOut("kubectl --context kubecontext --namespace testNamespace -v=0 create --dry-run -oyaml -f "+tmpDir.Path("deployment.yaml"), deploymentWebYAML). WithRunErr("kubectl --context kubecontext --namespace testNamespace -v=0 apply --overwrite=true --force -f -", fmt.Errorf("")), - builds: []build.Artifact{ - { - ImageName: "leeroy-web", - Tag: "leeroy-web:123", - }, - }, + builds: []build.Artifact{{ + ImageName: "leeroy-web", + Tag: "leeroy-web:123", + }}, + shouldErr: true, }, } - tmpDir, cleanup := testutil.NewTempDir(t) - defer cleanup() - - tmpDir.Write("deployment.yaml", deploymentWebYAML) - for _, test := range tests { t.Run(test.description, func(t *testing.T) { - if test.command != nil { - defer func(c util.Command) { util.DefaultExecCommand = c }(util.DefaultExecCommand) - util.DefaultExecCommand = test.command - } + defer func(c util.Command) { util.DefaultExecCommand = c }(util.DefaultExecCommand) + util.DefaultExecCommand = test.command k := NewKubectlDeployer(tmpDir.Root(), test.cfg, testKubeContext, testNamespace, "") err := k.Deploy(context.Background(), ioutil.Discard, test.builds, nil) @@ -161,6 +150,11 @@ func TestKubectlDeploy(t *testing.T) { } func TestKubectlCleanup(t *testing.T) { + tmpDir, cleanup := testutil.NewTempDir(t) + defer cleanup() + + tmpDir.Write("deployment.yaml", deploymentWebYAML) + var tests = []struct { description string cfg *latest.KubectlDeploy @@ -172,14 +166,18 @@ func TestKubectlCleanup(t *testing.T) { cfg: &latest.KubectlDeploy{ Manifests: []string{"deployment.yaml"}, }, - command: testutil.NewFakeCmd(t).WithRun("kubectl --context kubecontext --namespace testNamespace delete --ignore-not-found=true -f -"), + command: testutil.NewFakeCmd(t). + WithRunOut("kubectl --context kubecontext --namespace testNamespace create --dry-run -oyaml -f "+tmpDir.Path("deployment.yaml"), deploymentWebYAML). + WithRun("kubectl --context kubecontext --namespace testNamespace delete --ignore-not-found=true -f -"), }, { description: "cleanup error", cfg: &latest.KubectlDeploy{ Manifests: []string{"deployment.yaml"}, }, - command: testutil.NewFakeCmd(t).WithRunErr("kubectl --context kubecontext --namespace testNamespace delete --ignore-not-found=true -f -", errors.New("BUG")), + command: testutil.NewFakeCmd(t). + WithRunOut("kubectl --context kubecontext --namespace testNamespace create --dry-run -oyaml -f "+tmpDir.Path("deployment.yaml"), deploymentWebYAML). + WithRunErr("kubectl --context kubecontext --namespace testNamespace delete --ignore-not-found=true -f -", errors.New("BUG")), shouldErr: true, }, { @@ -192,21 +190,16 @@ func TestKubectlCleanup(t *testing.T) { Delete: []string{"--grace-period=1"}, }, }, - command: testutil.NewFakeCmd(t).WithRun("kubectl --context kubecontext --namespace testNamespace -v=0 delete --grace-period=1 --ignore-not-found=true -f -"), + command: testutil.NewFakeCmd(t). + WithRunOut("kubectl --context kubecontext --namespace testNamespace -v=0 create --dry-run -oyaml -f "+tmpDir.Path("deployment.yaml"), deploymentWebYAML). + WithRun("kubectl --context kubecontext --namespace testNamespace -v=0 delete --grace-period=1 --ignore-not-found=true -f -"), }, } - tmpDir, cleanup := testutil.NewTempDir(t) - defer cleanup() - - tmpDir.Write("deployment.yaml", deploymentWebYAML) - for _, test := range tests { t.Run(test.description, func(t *testing.T) { - if test.command != nil { - defer func(c util.Command) { util.DefaultExecCommand = c }(util.DefaultExecCommand) - util.DefaultExecCommand = test.command - } + defer func(c util.Command) { util.DefaultExecCommand = c }(util.DefaultExecCommand) + util.DefaultExecCommand = test.command k := NewKubectlDeployer(tmpDir.Root(), test.cfg, testKubeContext, testNamespace, "") err := k.Cleanup(context.Background(), ioutil.Discard) @@ -217,9 +210,15 @@ func TestKubectlCleanup(t *testing.T) { } func TestKubectlRedeploy(t *testing.T) { + tmpDir, cleanup := testutil.NewTempDir(t) + defer cleanup() + tmpDir.Write("deployment-web.yaml", deploymentWebYAML) + tmpDir.Write("deployment-app.yaml", deploymentAppYAML) + defer func(c util.Command) { util.DefaultExecCommand = c }(util.DefaultExecCommand) util.DefaultExecCommand = testutil.NewFakeCmd(t). WithRunOut("kubectl version --client -ojson", kubectlVersion). + WithRunOut("kubectl --context kubecontext --namespace testNamespace create --dry-run -oyaml -f "+tmpDir.Path("deployment-app.yaml")+" -f "+tmpDir.Path("deployment-web.yaml"), deploymentAppYAML+"\n"+deploymentWebYAML). WithRunInput("kubectl --context kubecontext --namespace testNamespace apply --force -f -", `apiVersion: v1 kind: Pod metadata: @@ -237,6 +236,7 @@ spec: containers: - image: leeroy-web:v1 name: leeroy-web`). + WithRunOut("kubectl --context kubecontext --namespace testNamespace create --dry-run -oyaml -f "+tmpDir.Path("deployment-app.yaml")+" -f "+tmpDir.Path("deployment-web.yaml"), deploymentAppYAML+"\n"+deploymentWebYAML). WithRunInput("kubectl --context kubecontext --namespace testNamespace apply --force -f -", `apiVersion: v1 kind: Pod metadata: @@ -244,15 +244,11 @@ metadata: spec: containers: - image: leeroy-app:v2 - name: leeroy-app`) - - tmpDir, cleanup := testutil.NewTempDir(t) - defer cleanup() - tmpDir.Write("deployment-web.yaml", deploymentWebYAML) - tmpDir.Write("deployment-app.yaml", deploymentAppYaml) + name: leeroy-app`). + WithRunOut("kubectl --context kubecontext --namespace testNamespace create --dry-run -oyaml -f "+tmpDir.Path("deployment-app.yaml")+" -f "+tmpDir.Path("deployment-web.yaml"), deploymentAppYAML+"\n"+deploymentWebYAML) cfg := &latest.KubectlDeploy{ - Manifests: []string{"deployment-web.yaml", "deployment-app.yaml"}, + Manifests: []string{"*.yaml"}, } deployer := NewKubectlDeployer(tmpDir.Root(), cfg, testKubeContext, testNamespace, "")