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

Misc improvements #911

Merged
merged 9 commits into from
Aug 23, 2018
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
38 changes: 38 additions & 0 deletions pkg/skaffold/bazel/bazel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,46 @@ package bazel

import (
"testing"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/GoogleContainerTools/skaffold/testutil"
)

func TestGetDependenciesWithWorkspace(t *testing.T) {
defer func(c util.Command) { util.DefaultExecCommand = c }(util.DefaultExecCommand)
util.DefaultExecCommand = testutil.NewFakeCmdOut(
"bazel query kind('source file', deps('target')) union buildfiles('target') --noimplicit_deps --order_output=no",
"@ignored\n//external/ignored\n\n//:dep1\n//:dep2\n",
nil,
)

tmpDir, cleanup := testutil.NewTempDir(t)
defer cleanup()
tmpDir.Write("WORKSPACE", "")

deps, err := GetDependencies(tmpDir.Root(), &v1alpha2.BazelArtifact{
BuildTarget: "target",
})

testutil.CheckErrorAndDeepEqual(t, false, err, []string{"dep1", "dep2", "WORKSPACE"}, deps)
}

func TestGetDependenciesWithoutWorkspace(t *testing.T) {
defer func(c util.Command) { util.DefaultExecCommand = c }(util.DefaultExecCommand)
util.DefaultExecCommand = testutil.NewFakeCmdOut(
"bazel query kind('source file', deps('target2')) union buildfiles('target2') --noimplicit_deps --order_output=no",
"@ignored\n//external/ignored\n\n//:dep3\n",
nil,
)

deps, err := GetDependencies(".", &v1alpha2.BazelArtifact{
BuildTarget: "target2",
})

testutil.CheckErrorAndDeepEqual(t, false, err, []string{"dep3"}, deps)
}

func TestQuery(t *testing.T) {
query := query("//:skaffold_example.tar")

Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/build/local/bazel.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (
)

func (b *Builder) buildBazel(ctx context.Context, out io.Writer, workspace string, a *v1alpha2.BazelArtifact) (string, error) {
cmd := exec.Command("bazel", "build", a.BuildTarget)
cmd := exec.CommandContext(ctx, "bazel", "build", a.BuildTarget)
cmd.Dir = workspace
cmd.Stdout = out
cmd.Stderr = out
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/build/local/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (b *Builder) buildDocker(ctx context.Context, out io.Writer, workspace stri
args := []string{"build", workspace, "--file", dockerfilePath, "-t", initialTag}
args = append(args, docker.GetBuildArgs(a)...)

cmd := exec.Command("docker", args...)
cmd := exec.CommandContext(ctx, "docker", args...)
if b.cfg.UseBuildkit {
cmd.Env = append(os.Environ(), "DOCKER_BUILDKIT=1")
}
Expand Down
36 changes: 18 additions & 18 deletions pkg/skaffold/deploy/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (h *HelmDeployer) Labels() map[string]string {
func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact) ([]Artifact, error) {
deployResults := []Artifact{}
for _, r := range h.Releases {
results, err := h.deployRelease(out, r, builds)
results, err := h.deployRelease(ctx, out, r, builds)
if err != nil {
releaseName, _ := evaluateReleaseName(r.Name)
return deployResults, errors.Wrapf(err, "deploying %s", releaseName)
Expand All @@ -92,32 +92,32 @@ func (h *HelmDeployer) Dependencies() ([]string, error) {
// Cleanup deletes what was deployed by calling Deploy.
func (h *HelmDeployer) Cleanup(ctx context.Context, out io.Writer) error {
for _, r := range h.Releases {
if err := h.deleteRelease(out, r); err != nil {
if err := h.deleteRelease(ctx, out, r); err != nil {
releaseName, _ := evaluateReleaseName(r.Name)
return errors.Wrapf(err, "deploying %s", releaseName)
}
}
return nil
}

func (h *HelmDeployer) helm(out io.Writer, arg ...string) error {
func (h *HelmDeployer) helm(ctx context.Context, out io.Writer, arg ...string) error {
args := append([]string{"--kube-context", h.kubeContext}, arg...)

cmd := exec.Command("helm", args...)
cmd := exec.CommandContext(ctx, "helm", args...)
cmd.Stdout = out
cmd.Stderr = out

return util.RunCmd(cmd)
}

func (h *HelmDeployer) deployRelease(out io.Writer, r v1alpha2.HelmRelease, builds []build.Artifact) ([]Artifact, error) {
func (h *HelmDeployer) deployRelease(ctx context.Context, out io.Writer, r v1alpha2.HelmRelease, builds []build.Artifact) ([]Artifact, error) {
isInstalled := true

releaseName, err := evaluateReleaseName(r.Name)
if err != nil {
return nil, errors.Wrap(err, "cannot parse the release name template")
}
if err := h.helm(out, "get", releaseName); err != nil {
if err := h.helm(ctx, out, "get", releaseName); err != nil {
color.Red.Fprintf(out, "Helm release %s not installed. Installing...\n", releaseName)
isInstalled = false
}
Expand All @@ -140,7 +140,7 @@ func (h *HelmDeployer) deployRelease(out io.Writer, r v1alpha2.HelmRelease, buil

// First build dependencies.
logrus.Infof("Building helm dependencies...")
if err := h.helm(out, "dep", "build", r.ChartPath); err != nil {
if err := h.helm(ctx, out, "dep", "build", r.ChartPath); err != nil {
return nil, errors.Wrap(err, "building helm dependencies")
}

Expand All @@ -164,7 +164,7 @@ func (h *HelmDeployer) deployRelease(out io.Writer, r v1alpha2.HelmRelease, buil
}
args = append(args, r.ChartPath)
} else {
chartPath, err := h.packageChart(r)
chartPath, err := h.packageChart(ctx, r)
if err != nil {
return nil, errors.WithMessage(err, "cannot package chart")
}
Expand Down Expand Up @@ -240,8 +240,8 @@ func (h *HelmDeployer) deployRelease(out io.Writer, r v1alpha2.HelmRelease, buil
}
args = append(args, setOpts...)

helmErr := h.helm(out, args...)
return h.getDeployResults(ns, releaseName), helmErr
helmErr := h.helm(ctx, out, args...)
return h.getDeployResults(ctx, ns, releaseName), helmErr
}

// imageName if the given string includes a fully qualified docker image name then lets trim just the tag part out
Expand All @@ -260,7 +260,7 @@ func extractTag(imageName string) string {

// packageChart packages the chart and returns path to the chart archive file.
// If this function returns an error, it will always be wrapped.
func (h *HelmDeployer) packageChart(r v1alpha2.HelmRelease) (string, error) {
func (h *HelmDeployer) packageChart(ctx context.Context, r v1alpha2.HelmRelease) (string, error) {
tmp := os.TempDir()
packageArgs := []string{"package", r.ChartPath, "--destination", tmp}
if r.Packaged.Version != "" {
Expand All @@ -279,7 +279,7 @@ func (h *HelmDeployer) packageChart(r v1alpha2.HelmRelease) (string, error) {
}

buf := &bytes.Buffer{}
err := h.helm(buf, packageArgs...)
err := h.helm(ctx, buf, packageArgs...)
output := strings.TrimSpace(buf.String())
if err != nil {
return "", errors.Wrapf(err, "package chart into a .tgz archive (%s)", output)
Expand All @@ -293,9 +293,9 @@ func (h *HelmDeployer) packageChart(r v1alpha2.HelmRelease) (string, error) {
return filepath.Join(tmp, fpath), nil
}

func (h *HelmDeployer) getReleaseInfo(release string) (*bufio.Reader, error) {
func (h *HelmDeployer) getReleaseInfo(ctx context.Context, release string) (*bufio.Reader, error) {
var releaseInfo bytes.Buffer
if err := h.helm(&releaseInfo, "get", release); err != nil {
if err := h.helm(ctx, &releaseInfo, "get", release); err != nil {
return nil, fmt.Errorf("error retrieving helm deployment info: %s", releaseInfo.String())
}
return bufio.NewReader(&releaseInfo), nil
Expand All @@ -304,22 +304,22 @@ func (h *HelmDeployer) getReleaseInfo(release string) (*bufio.Reader, error) {
// Retrieve info about all releases using helm get
// Skaffold labels will be applied to each deployed k8s object
// Since helm isn't always consistent with retrieving results, don't return errors here
func (h *HelmDeployer) getDeployResults(namespace string, release string) []Artifact {
b, err := h.getReleaseInfo(release)
func (h *HelmDeployer) getDeployResults(ctx context.Context, namespace string, release string) []Artifact {
b, err := h.getReleaseInfo(ctx, release)
if err != nil {
logrus.Warnf(err.Error())
return nil
}
return parseReleaseInfo(namespace, b)
}

func (h *HelmDeployer) deleteRelease(out io.Writer, r v1alpha2.HelmRelease) error {
func (h *HelmDeployer) deleteRelease(ctx context.Context, out io.Writer, r v1alpha2.HelmRelease) error {
releaseName, err := evaluateReleaseName(r.Name)
if err != nil {
return errors.Wrap(err, "cannot parse the release name template")
}

if err := h.helm(out, "delete", releaseName, "--purge"); err != nil {
if err := h.helm(ctx, out, "delete", releaseName, "--purge"); err != nil {
logrus.Debugf("deleting release %s: %v\n", releaseName, err)
}

Expand Down
16 changes: 8 additions & 8 deletions pkg/skaffold/deploy/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,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) ([]Artifact, error) {
manifests, err := k.readManifests()
manifests, err := k.readManifests(ctx)
if err != nil {
return nil, errors.Wrap(err, "reading manifests")
}
Expand All @@ -78,7 +78,7 @@ func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []bu
return nil, errors.Wrap(err, "replacing images in manifests")
}

updated, err := k.kubectl.Apply(out, manifests)
updated, err := k.kubectl.Apply(ctx, out, manifests)
if err != nil {
return nil, errors.Wrap(err, "apply")
}
Expand All @@ -88,12 +88,12 @@ func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []bu

// Cleanup deletes what was deployed by calling Deploy.
func (k *KubectlDeployer) Cleanup(ctx context.Context, out io.Writer) error {
manifests, err := k.readManifests()
manifests, err := k.readManifests(ctx)
if err != nil {
return errors.Wrap(err, "reading manifests")
}

if err := k.kubectl.Detete(out, manifests); err != nil {
if err := k.kubectl.Detete(ctx, out, manifests); err != nil {
return errors.Wrap(err, "delete")
}

Expand Down Expand Up @@ -135,7 +135,7 @@ func parseManifestsForDeploys(manifests kubectl.ManifestList) ([]Artifact, error
}

// readManifests reads the manifests to deploy/delete.
func (k *KubectlDeployer) readManifests() (kubectl.ManifestList, error) {
func (k *KubectlDeployer) readManifests(ctx context.Context) (kubectl.ManifestList, error) {
files, err := k.manifestFiles(k.Manifests)
if err != nil {
return nil, errors.Wrap(err, "expanding user manifest list")
Expand All @@ -152,7 +152,7 @@ func (k *KubectlDeployer) readManifests() (kubectl.ManifestList, error) {
}

for _, m := range k.RemoteManifests {
manifest, err := k.readRemoteManifest(m)
manifest, err := k.readRemoteManifest(ctx, m)
if err != nil {
return nil, errors.Wrap(err, "get remote manifests")
}
Expand All @@ -165,7 +165,7 @@ func (k *KubectlDeployer) readManifests() (kubectl.ManifestList, error) {
return manifests, nil
}

func (k *KubectlDeployer) readRemoteManifest(name string) ([]byte, error) {
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])
Expand All @@ -174,7 +174,7 @@ func (k *KubectlDeployer) readRemoteManifest(name string) ([]byte, error) {
args = append(args, name, "-o", "yaml")

var manifest bytes.Buffer
err := k.kubectl.Run(nil, &manifest, "get", nil, args...)
err := k.kubectl.Run(ctx, nil, &manifest, "get", nil, args...)
if err != nil {
return nil, errors.Wrap(err, "getting manifest")
}
Expand Down
13 changes: 7 additions & 6 deletions pkg/skaffold/deploy/kubectl/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package kubectl

import (
"context"
"io"
"os/exec"

Expand All @@ -36,16 +37,16 @@ type CLI struct {
}

// Detete runs `kubectl delete` on a list of manifests.
func (c *CLI) Detete(out io.Writer, manifests ManifestList) error {
if err := c.Run(manifests.Reader(), out, "delete", c.Flags.Delete, "--ignore-not-found=true", "-f", "-"); err != nil {
func (c *CLI) Detete(ctx context.Context, out io.Writer, manifests ManifestList) error {
if err := c.Run(ctx, manifests.Reader(), out, "delete", c.Flags.Delete, "--ignore-not-found=true", "-f", "-"); err != nil {
return errors.Wrap(err, "kubectl delete")
}

return nil
}

// Apply runs `kubectl apply` on a list of manifests.
func (c *CLI) Apply(out io.Writer, manifests ManifestList) (ManifestList, error) {
func (c *CLI) Apply(ctx context.Context, out io.Writer, manifests ManifestList) (ManifestList, error) {
// Only redeploy modified or new manifests
// TODO(dgageot): should we delete a manifest that was deployed and is not anymore?
updated := c.previousApply.Diff(manifests)
Expand All @@ -55,15 +56,15 @@ func (c *CLI) Apply(out io.Writer, manifests ManifestList) (ManifestList, error)
return nil, nil
}

if err := c.Run(manifests.Reader(), out, "apply", c.Flags.Apply, "-f", "-"); err != nil {
if err := c.Run(ctx, manifests.Reader(), out, "apply", c.Flags.Apply, "-f", "-"); err != nil {
return nil, errors.Wrap(err, "kubectl apply")
}

return updated, nil
}

// Run shells out kubectl CLI.
func (c *CLI) Run(in io.Reader, out io.Writer, command string, commandFlags []string, arg ...string) error {
func (c *CLI) Run(ctx context.Context, in io.Reader, out io.Writer, command string, commandFlags []string, arg ...string) error {
args := []string{"--context", c.KubeContext}
if c.Namespace != "" {
args = append(args, "--namespace", c.Namespace)
Expand All @@ -73,7 +74,7 @@ func (c *CLI) Run(in io.Reader, out io.Writer, command string, commandFlags []st
args = append(args, commandFlags...)
args = append(args, arg...)

cmd := exec.Command("kubectl", args...)
cmd := exec.CommandContext(ctx, "kubectl", args...)
cmd.Stdin = in
cmd.Stdout = out
cmd.Stderr = out
Expand Down
12 changes: 6 additions & 6 deletions pkg/skaffold/deploy/kustomize.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (k *KustomizeDeployer) Labels() map[string]string {
}

func (k *KustomizeDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact) ([]Artifact, error) {
manifests, err := k.readManifests()
manifests, err := k.readManifests(ctx)
if err != nil {
return nil, errors.Wrap(err, "reading manifests")
}
Expand All @@ -67,7 +67,7 @@ func (k *KustomizeDeployer) Deploy(ctx context.Context, out io.Writer, builds []
return nil, errors.Wrap(err, "replacing images in manifests")
}

updated, err := k.kubectl.Apply(out, manifests)
updated, err := k.kubectl.Apply(ctx, out, manifests)
if err != nil {
return nil, errors.Wrap(err, "apply")
}
Expand All @@ -76,12 +76,12 @@ func (k *KustomizeDeployer) Deploy(ctx context.Context, out io.Writer, builds []
}

func (k *KustomizeDeployer) Cleanup(ctx context.Context, out io.Writer) error {
manifests, err := k.readManifests()
manifests, err := k.readManifests(ctx)
if err != nil {
return errors.Wrap(err, "reading manifests")
}

if err := k.kubectl.Detete(out, manifests); err != nil {
if err := k.kubectl.Detete(ctx, out, manifests); err != nil {
return errors.Wrap(err, "delete")
}

Expand All @@ -93,8 +93,8 @@ func (k *KustomizeDeployer) Dependencies() ([]string, error) {
return []string{k.KustomizePath}, nil
}

func (k *KustomizeDeployer) readManifests() (kubectl.ManifestList, error) {
cmd := exec.Command("kustomize", "build", k.KustomizePath)
func (k *KustomizeDeployer) readManifests(ctx context.Context) (kubectl.ManifestList, error) {
cmd := exec.CommandContext(ctx, "kustomize", "build", k.KustomizePath)
out, err := util.RunCmdOut(cmd)
if err != nil {
return nil, errors.Wrap(err, "kustomize build")
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/kubernetes/colorpicker.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ type colorPicker struct {
imageColors map[string]color.Color
}

// NewColorPicker creates a new ColorPicker. For each artfact, a color will be selected
// NewColorPicker creates a new ColorPicker. For each artifact, a color will be selected
// sequentially from `colorCodes`. If all colors are used, the first color will be used
// again. The formatter for the associated color will then be returned by `Pick` each
// time it is called for the artifact and can be used to write to out in that color.
Expand Down
Loading