Merge pull request #2258 from tanner-bruce/fix-remote-manifests

tejal29 · web-flow · commit d81b9cc8b52b · 2019-08-09T14:35:37.000-07:00
Fix remoteManifests
diff --git a/pkg/skaffold/deploy/kubectl.go b/pkg/skaffold/deploy/kubectl.go
@@ -17,8 +17,10 @@ limitations under the License.
 package deploy
 
 import (
+	"bytes"
 	"context"
 	"io"
+	"strings"
 
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -38,6 +40,7 @@ import (
 type KubectlDeployer struct {
 	*latest.KubectlDeploy
 
+	originalImages     []build.Artifact
 	workingDir         string
 	kubectl            deploy.CLI
 	defaultRepo        string
@@ -80,6 +83,24 @@ func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []bu
 		return 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")
+		}
+
+		manifests = append(manifests, manifest)
+	}
+
+	if len(k.originalImages) == 0 {
+		k.originalImages, err = manifests.GetImages()
+		if err != nil {
+			return errors.Wrap(err, "get images from manifests")
+		}
+	}
+
+	logrus.Debugln("manifests", manifests.String())
+
 	if len(manifests) == 0 {
 		return nil
 	}
@@ -122,6 +143,22 @@ func (k *KubectlDeployer) Cleanup(ctx context.Context, out io.Writer) error {
 		return errors.Wrap(err, "reading manifests")
 	}
 
+	// pull remote manifests
+	var rm deploy.ManifestList
+	for _, m := range k.RemoteManifests {
+		manifest, err := k.readRemoteManifest(ctx, m)
+		if err != nil {
+			return errors.Wrap(err, "get remote manifests")
+		}
+		rm = append(rm, manifest)
+	}
+	upd, err := rm.ReplaceImages(k.originalImages, k.defaultRepo)
+	if err != nil {
+		return errors.Wrap(err, "replacing with originals")
+	}
+	if err := k.kubectl.Apply(ctx, out, upd); err != nil {
+		return errors.Wrap(err, "apply original")
+	}
 	if err := k.kubectl.Delete(ctx, out, manifests); err != nil {
 		return errors.Wrap(err, "delete")
 	}
@@ -182,3 +219,23 @@ func (k *KubectlDeployer) readManifests(ctx context.Context) (deploy.ManifestLis
 
 	return k.kubectl.ReadManifests(ctx, manifests)
 }
+
+// readRemoteManifests will try to read manifests from the given kubernetes
+// context in the specified namespace and for the specified type
+func (k *KubectlDeployer) readRemoteManifest(ctx context.Context, name string) ([]byte, error) {
+	var args []string
+	ns := ""
+	if parts := strings.Split(name, ":"); len(parts) > 1 {
+		ns = parts[0]
+		name = parts[1]
+	}
+	args = append(args, name, "-o", "yaml")
+
+	var manifest bytes.Buffer
+	err := k.kubectl.RunInNamespace(ctx, nil, &manifest, "get", ns, args...)
+	if err != nil {
+		return nil, errors.Wrap(err, "getting manifest")
+	}
+
+	return manifest.Bytes(), nil
+}
diff --git a/pkg/skaffold/deploy/kubectl/images.go b/pkg/skaffold/deploy/kubectl/images.go
@@ -17,13 +17,13 @@ limitations under the License.
 package kubectl
 
 import (
-	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
-
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/warnings"
+
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 // ReplaceImages replaces image names in a list of manifests.
@@ -41,6 +41,38 @@ func (l *ManifestList) ReplaceImages(builds []build.Artifact, defaultRepo string
 	return updated, nil
 }
 
+// GetImages gathers a map of base image names to the image with its tag
+func (l *ManifestList) GetImages() ([]build.Artifact, error) {
+	s := &imageSaver{}
+	_, err := l.Visit(s)
+	return s.Images, err
+}
+
+type imageSaver struct {
+	Images []build.Artifact
+}
+
+func (is *imageSaver) Matches(key string) bool {
+	return key == "image"
+}
+
+func (is *imageSaver) NewValue(old interface{}) (bool, interface{}) {
+	image, ok := old.(string)
+	if !ok {
+		return false, nil
+	}
+	parsed, err := docker.ParseReference(image)
+	if err != nil {
+		return false, err
+	}
+
+	is.Images = append(is.Images, build.Artifact{
+		Tag:       image,
+		ImageName: parsed.BaseName,
+	})
+	return false, nil
+}
+
 type imageReplacer struct {
 	defaultRepo     string
 	tagsByImageName map[string]string
@@ -82,6 +114,8 @@ func (r *imageReplacer) NewValue(old interface{}) (bool, interface{}) {
 	return found, tag
 }
 
+// parseAndReplace takes an image from a manifest and if that image matches
+// a built image it will update the tag
 func (r *imageReplacer) parseAndReplace(image string) (bool, interface{}) {
 	parsed, err := docker.ParseReference(image)
 	if err != nil {
diff --git a/pkg/skaffold/deploy/kubectl/visitor.go b/pkg/skaffold/deploy/kubectl/visitor.go
@@ -18,7 +18,7 @@ package kubectl
 
 import (
 	"github.com/pkg/errors"
-	yaml "gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v2"
 )
 
 // Replacer is used to replace portions of yaml manifests that match a given key.
diff --git a/pkg/skaffold/deploy/kubectl_test.go b/pkg/skaffold/deploy/kubectl_test.go
@@ -270,6 +270,61 @@ func TestKubectlCleanup(t *testing.T) {
 	}
 }
 
+func TestKubectlDeployerRemoteCleanup(t *testing.T) {
+	tests := []struct {
+		description string
+		cfg         *latest.KubectlDeploy
+		command     util.Command
+	}{
+		{
+			description: "cleanup success",
+			cfg: &latest.KubectlDeploy{
+				RemoteManifests: []string{"pod/leeroy-web"},
+			},
+			command: testutil.NewFakeCmd(t).
+				WithRun("kubectl --context kubecontext --namespace testNamespace get pod/leeroy-web -o yaml").
+				WithRun("kubectl --context kubecontext --namespace testNamespace delete --ignore-not-found=true -f -").
+				WithRunInput("kubectl --context kubecontext --namespace testNamespace apply -f -", deploymentWebYAML),
+		},
+		{
+			description: "cleanup error",
+			cfg: &latest.KubectlDeploy{
+				RemoteManifests: []string{"anotherNamespace:pod/leeroy-web"},
+			},
+			command: testutil.NewFakeCmd(t).
+				WithRun("kubectl --context kubecontext --namespace anotherNamespace get pod/leeroy-web -o yaml").
+				WithRun("kubectl --context kubecontext --namespace testNamespace delete --ignore-not-found=true -f -").
+				WithRunInput("kubectl --context kubecontext --namespace anotherNamespace apply -f -", deploymentWebYAML),
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, "cleanup remote", func(t *testutil.T) {
+			t.Override(&util.DefaultExecCommand, test.command)
+			t.NewTempDir().
+				Write("deployment.yaml", deploymentWebYAML).
+				Chdir()
+
+			k := NewKubectlDeployer(&runcontext.RunContext{
+				WorkingDir: ".",
+				Cfg: latest.Pipeline{
+					Deploy: latest.DeployConfig{
+						DeployType: latest.DeployType{
+							KubectlDeploy: test.cfg,
+						},
+					},
+				},
+				KubeContext: testKubeContext,
+				Opts: config.SkaffoldOptions{
+					Namespace: testNamespace,
+				},
+			})
+			err := k.Cleanup(context.Background(), ioutil.Discard)
+
+			t.CheckError(false, err)
+		})
+	}
+}
+
 func TestKubectlRedeploy(t *testing.T) {
 	testutil.Run(t, "", func(t *testutil.T) {
 		tmpDir := t.NewTempDir().
diff --git a/pkg/skaffold/kubectl/cli.go b/pkg/skaffold/kubectl/cli.go
@@ -44,7 +44,13 @@ func NewFromRunContext(runCtx *runcontext.RunContext) *CLI {
 
 // Command creates the underlying exec.CommandContext. This allows low-level control of the executed command.
 func (c *CLI) Command(ctx context.Context, command string, arg ...string) *exec.Cmd {
-	args := c.args(command, arg...)
+	args := c.args(command, "", arg...)
+	return exec.CommandContext(ctx, "kubectl", args...)
+}
+
+// Command creates the underlying exec.CommandContext with namespace. This allows low-level control of the executed command.
+func (c *CLI) CommandWithNamespaceArg(ctx context.Context, command string, namespace string, arg ...string) *exec.Cmd {
+	args := c.args(command, namespace, arg...)
 	return exec.CommandContext(ctx, "kubectl", args...)
 }
 
@@ -57,6 +63,15 @@ func (c *CLI) Run(ctx context.Context, in io.Reader, out io.Writer, command stri
 	return util.RunCmd(cmd)
 }
 
+// Run shells out kubectl CLI with given namespace
+func (c *CLI) RunInNamespace(ctx context.Context, in io.Reader, out io.Writer, command string, namespace string, arg ...string) error {
+	cmd := c.CommandWithNamespaceArg(ctx, command, namespace, arg...)
+	cmd.Stdin = in
+	cmd.Stdout = out
+	cmd.Stderr = out
+	return util.RunCmd(cmd)
+}
+
 // Run shells out kubectl CLI.
 func (c *CLI) RunOut(ctx context.Context, command string, arg ...string) ([]byte, error) {
 	cmd := c.Command(ctx, command, arg...)
@@ -65,12 +80,20 @@ func (c *CLI) RunOut(ctx context.Context, command string, arg ...string) ([]byte
 
 // args builds an argument list for calling kubectl and consistently
 // adds the `--context` and `--namespace` flags.
-func (c *CLI) args(command string, arg ...string) []string {
+func (c *CLI) args(command string, namespace string, arg ...string) []string {
 	args := []string{"--context", c.KubeContext}
-	if c.Namespace != "" {
-		args = append(args, "--namespace", c.Namespace)
+	namespace = c.resolveNamespace(namespace)
+	if namespace != "" {
+		args = append(args, "--namespace", namespace)
 	}
 	args = append(args, command)
 	args = append(args, arg...)
 	return args
 }
+
+func (c *CLI) resolveNamespace(ns string) string {
+	if ns != "" {
+		return ns
+	}
+	return c.Namespace
+}

Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,7 @@ package kubectl`
`18`	`18`
`19`	`19`	`import (`
`20`	`20`	`"github.com/pkg/errors"`
`21`		`- yaml "gopkg.in/yaml.v2"`
	`21`	`+ "gopkg.in/yaml.v2"`
`22`	`22`	`)`
`23`	`23`
`24`	`24`	`// Replacer is used to replace portions of yaml manifests that match a given key.`