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

Add debugging support for Skaffold: skaffold debug #1702

Merged
merged 66 commits into from
Mar 27, 2019
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
66 commits
Select commit Hold shift + click to select a range
b6a592f
Initial hack at `skaffold debug`:
briandealwis Jan 26, 2019
51827d6
Make kubectl transforms be configurable
briandealwis Jan 31, 2019
01354ab
Disable watching for `skaffold debug`
briandealwis Feb 7, 2019
9e553d1
Merge remote-tracking branch 'origin/master' into HEAD
briandealwis Feb 7, 2019
d548123
Add some infrastructure for guessing artifact type
briandealwis Feb 7, 2019
003a268
Add support for retrieving image configurations at build-time
briandealwis Feb 9, 2019
2909234
Workaround google/go-containerregistry#351
briandealwis Feb 14, 2019
e4d6521
Use auth when fetching image
briandealwis Feb 14, 2019
96c641b
doc improvements
briandealwis Feb 14, 2019
e28df85
separate jvm debugging; go types fixes
briandealwis Feb 14, 2019
292ed1a
rewrite using k8s api objects to avoid typing errors
briandealwis Feb 15, 2019
54225a1
Merge remote-tracking branch 'origin/master' into HEAD
briandealwis Feb 15, 2019
8bb0a8c
Avoid port conflicts
briandealwis Feb 20, 2019
ab72e71
pickup parameters from existing jdwp configuration
briandealwis Feb 22, 2019
a1b4caa
Initial support for NodeJS
briandealwis Feb 26, 2019
143a75c
merge with HEAD
briandealwis Feb 26, 2019
60161ef
Support nodemon too
briandealwis Feb 27, 2019
eca494e
Fix variable scope
briandealwis Feb 27, 2019
a524fa4
address initial review comments
briandealwis Feb 28, 2019
d4a31ee
Refactor debug code to debugging package
briandealwis Mar 1, 2019
0b6c56f
separate skaffold-specific pieces
briandealwis Mar 1, 2019
b984d32
rejig file names
briandealwis Mar 1, 2019
0863254
Move envAsMap to debug and toss unused helper functions
briandealwis Mar 4, 2019
9aaa4c5
Move TestFindArtifact to debug_test
briandealwis Mar 4, 2019
1a83a0a
fixup copyrights
briandealwis Mar 4, 2019
704729f
Add support for other k8s object types
briandealwis Mar 6, 2019
8d6c705
Add support for PodLists too
briandealwis Mar 6, 2019
6f8318d
make linter happy
briandealwis Mar 6, 2019
bead5d0
Merge with HEAD
briandealwis Mar 6, 2019
a258068
gofmt -s
briandealwis Mar 6, 2019
63d6598
Rename CheckEqual -> CheckDeepEqualWithOptions
briandealwis Mar 7, 2019
0aee98f
slight tweak; regen doc
briandealwis Mar 8, 2019
864a839
Merge remote-tracking branch 'origin/master' into HEAD
briandealwis Mar 11, 2019
6f4caa9
Merge with HEAD and fixup
briandealwis Mar 12, 2019
2060611
Get rid of ParseName workaround
briandealwis Mar 12, 2019
3fe99af
Initial hack at docs
briandealwis Mar 12, 2019
a15bc08
Merge remote-tracking branch 'origin/master' into HEAD
briandealwis Mar 12, 2019
0c1f588
move watch-related options back to `dev`
briandealwis Mar 12, 2019
883cf50
made debug go last
briandealwis Mar 14, 2019
ddff55b
Merge remote-tracking branch 'origin/master' into HEAD
briandealwis Mar 14, 2019
0defe2c
why does this keep disappearing
briandealwis Mar 14, 2019
f903b9c
make test happy
briandealwis Mar 14, 2019
21f3e1d
fix up doc
briandealwis Mar 15, 2019
48f1092
simplify debug() by delegating to dev()
briandealwis Mar 15, 2019
77edbc0
Change builder API to return Artifact
briandealwis Mar 16, 2019
8a039c2
more review nits
briandealwis Mar 20, 2019
f9c0fdc
Use testutil.CheckDeepEqual instead of CheckDeepEqualWithOptions
briandealwis Mar 20, 2019
96d2a04
Add e2e tests for ApplyDebuggingTransforms
briandealwis Mar 20, 2019
dec62cf
Merge remote-tracking branch 'origin/master' into HEAD
briandealwis Mar 20, 2019
1cf5434
Fixups
briandealwis Mar 20, 2019
b3655f0
Merge with HEAD; revert goimports on pkg/skaffold/event/proto/skaffol…
briandealwis Mar 21, 2019
ff0e805
update doc for --rpc-http-port
briandealwis Mar 21, 2019
d70cb48
Merge remote-tracking branch 'origin/master' into HEAD
briandealwis Mar 25, 2019
1dfcacb
Pick up doc fix for #1863
briandealwis Mar 25, 2019
48ef8e9
clarify todo
briandealwis Mar 26, 2019
6a92d51
formatting
briandealwis Mar 26, 2019
7fc4a67
move pkg/skaffold/debugging -> pkg/skaffold/debug
briandealwis Mar 26, 2019
2aa9e30
transform{nodejs,jvm} -> transform_{nodejs,jvm}
briandealwis Mar 26, 2019
f7cc3ae
no helm or kustomize
briandealwis Mar 26, 2019
6b8f8cb
inspectSpec doc comment
briandealwis Mar 26, 2019
17d6065
rewrite and simplify allocatePort
briandealwis Mar 26, 2019
9f9338a
extract default debug ports to constants
briandealwis Mar 26, 2019
c727bc9
Try retrieving image configuration from local docker, and then remote…
briandealwis Mar 26, 2019
80ff077
back out unnecessary build API changes
briandealwis Mar 26, 2019
3be4334
Use pkg/skaffold/docker to fetch image configuration
briandealwis Mar 26, 2019
a5e2859
goimports
briandealwis Mar 26, 2019
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
5 changes: 5 additions & 0 deletions cmd/skaffold/app/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ func NewSkaffoldCommand(out, err io.Writer) *cobra.Command {
rootCmd.AddCommand(NewCmdVersion(out))
rootCmd.AddCommand(NewCmdRun(out))
rootCmd.AddCommand(NewCmdDev(out))
rootCmd.AddCommand(NewCmdDebug(out))
rootCmd.AddCommand(NewCmdBuild(out))
rootCmd.AddCommand(NewCmdDeploy(out))
rootCmd.AddCommand(NewCmdDelete(out))
Expand Down Expand Up @@ -150,6 +151,10 @@ func AddRunDevFlags(cmd *cobra.Command) {
cmd.Flags().BoolVar(&opts.SkipTests, "skip-tests", false, "Whether to skip the tests after building")
}

func AddRunDebugFlags(cmd *cobra.Command) {
AddRunDevFlags(cmd)
}

func SetUpLogs(out io.Writer, level string) error {
logrus.SetOutput(out)
lvl, err := logrus.ParseLevel(v)
Expand Down
105 changes: 105 additions & 0 deletions cmd/skaffold/app/cmd/debug.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
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 cmd

import (
"context"
"io"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
)

// NewCmdDebug describes the CLI command to run a pipeline in debug mode.
func NewCmdDebug(out io.Writer) *cobra.Command {
cmd := &cobra.Command{
Use: "debug",
Short: "Runs a pipeline file in debug mode",
Long: "Similar to `dev`, but runs the pipeline for debugging. Artifacts that are debugged are not watched.",
Args: cobra.NoArgs,
RunE: func(cmd *cobra.Command, args []string) error {
return debug(out)
},
}
AddRunDebugFlags(cmd)
cmd.Flags().BoolVar(&opts.TailDev, "tail", true, "Stream logs from deployed objects")
briandealwis marked this conversation as resolved.
Show resolved Hide resolved
cmd.Flags().StringVar(&opts.Trigger, "trigger", "polling", "How are changes detected? (polling or manual)")
cmd.Flags().BoolVar(&opts.Cleanup, "cleanup", true, "Delete deployments after dev mode is interrupted")
cmd.Flags().StringArrayVarP(&opts.TargetImages, "watch-image", "w", nil, "Choose which artifacts to watch. Artifacts with image names that contain the expression will be watched only. Default is to watch sources for all artifacts")
briandealwis marked this conversation as resolved.
Show resolved Hide resolved
cmd.Flags().IntVarP(&opts.WatchPollInterval, "watch-poll-interval", "i", 1000, "Interval (in ms) between two checks for file changes")
cmd.Flags().BoolVar(&opts.PortForward, "port-forward", true, "Port-forward exposed container ports within pods")
cmd.Flags().StringArrayVarP(&opts.CustomLabels, "label", "l", nil, "Add custom labels to deployed objects. Set multiple times for multiple labels")
return cmd
}

func debug(out io.Writer) error {
briandealwis marked this conversation as resolved.
Show resolved Hide resolved
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
catchCtrlC(cancel)

cleanup := func() {}
if opts.Cleanup {
defer func() {
cleanup()
}()
}

output := &config.Output{
Main: out,
Logs: out,
}

// HACK: prevent redeploying changed containers during debugging
// TODO: build to ignore debuggable artifacts
if len(opts.TargetImages) == 0 {
briandealwis marked this conversation as resolved.
Show resolved Hide resolved
opts.TargetImages = []string{"none"}
}

deploy.AddManifestTransform(kubectl.ApplyDebuggingTransforms)

for {
select {
case <-ctx.Done():
return nil
default:
r, config, err := newRunner(opts)
if err != nil {
return errors.Wrap(err, "creating runner")
}

// follow the normal dev flow
err = r.Dev(ctx, output, config.Build.Artifacts)
if r.HasDeployed() {
cleanup = func() {
if err := r.Cleanup(context.Background(), out); err != nil {
logrus.Warnln("cleanup:", err)
}
}
}
if err != nil {
if errors.Cause(err) != runner.ErrorConfigurationChanged {
return err
}
}
}
}
}
75 changes: 75 additions & 0 deletions pkg/skaffold/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,24 @@ package build
import (
"context"
"io"
"strings"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/name"
config "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/remote"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

// Artifact is the result corresponding to each successful build.
type Artifact struct {
ImageName string
Tag string
Config ConfigurationRetriever
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is going to work well with builder plugins. We could maybe return a bool (local, !local) or an enum.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyhow, builders should maybe return some else that a string

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Urp! This is a really good point.

}

// Builder is an interface to the Build API of Skaffold.
Expand All @@ -41,3 +50,69 @@ type Builder interface {

DependenciesForArtifact(ctx context.Context, artifact *latest.Artifact) ([]string, error)
}

// ConfigurationRetriever is a function signature for a function that returns an OCI image configuration.
// There are two ConfigurationRetrievers available:
//
// - RegistryConfigurationRetriever for querying a registry based on the provided image reference
//
// - DockerConfigurationRetriever for querying the local docker daemon's cache
type ConfigurationRetriever func(ctx context.Context) (config.Config, error)

// RegistryConfigurationRetriever returns a function to retrieve an image configuration from a registry
func RegistryConfigurationRetriever(image string) ConfigurationRetriever {
return func(ctx context.Context) (config.Config, error) {
logrus.Debugf("Retrieving image configuration for %v", image)
briandealwis marked this conversation as resolved.
Show resolved Hide resolved
ref, err := parseReference(image)
if err != nil {
logrus.Debugf("Error parsing image %v: %v", image, err)
return config.Config{}, errors.Wrapf(err, "parsing image %q", image)
}

auth, err := authn.DefaultKeychain.Resolve(ref.Context().Registry)
if err != nil {
return config.Config{}, errors.Wrap(err, "getting default keychain auth")
}

remoteImage, err := remote.Image(ref, remote.WithAuth(auth))
if err != nil {
logrus.Debugf("Error retrieving remote image details %v: %v", image, err)
return config.Config{}, errors.Wrapf(err, "retrieving image %q", ref)
}

manifest, err := remoteImage.ConfigFile()
if err != nil {
logrus.Debugf("Error retrieving remote image manifest %v: %v", image, err)
return config.Config{}, errors.Wrapf(err, "retrieving image config for %q", ref)
}
return manifest.Config, nil
}
}

func parseReference(image string) (name.Reference, error) {
ref, err := name.ParseReference(image, name.WeakValidation)
if err == nil {
return ref, nil
}
if parts := strings.Split(image, "@"); len(parts) == 2 {
// workaround https://github.com/google/go-containerregistry/issues/351 and strip tag
if tagged, err := name.NewTag(parts[0], name.WeakValidation); err == nil {
image = tagged.Repository.Name() + "@" + parts[1]
return name.ParseReference(image, name.WeakValidation)
}
}
return nil, err
}

// DockerConfigurationRetriever returns a function to retrieve an image configuration from a local docker daemon
func DockerConfigurationRetriever(localDocker docker.LocalDaemon, image string) ConfigurationRetriever {
return func(ctx context.Context) (config.Config, error) {
manifest, err := localDocker.ConfigFile(ctx, image)
if err != nil {
logrus.Debugf("Error retrieving local image manifest for %v: %v", image, err)
return config.Config{}, errors.Wrapf(err, "retrieving image config for %q", image)
}
logrus.Debugf("Retrieved local image configuration for %v: %v", image, manifest.Config)
return manifest.Config, nil
}
}
65 changes: 65 additions & 0 deletions pkg/skaffold/build/build_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
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 build

import (
"testing"

"github.com/GoogleContainerTools/skaffold/testutil"
)

func TestParseReference(t *testing.T) {
var tests = []struct {
description string
image string
shouldErr bool
name string
identifier string
}{
{
description: "simple name",
image: "busybox",
shouldErr: false,
name: "index.docker.io/library/busybox",
identifier: "latest",
},
{
description: "with tag",
image: "busybox:1.30",
shouldErr: false,
name: "index.docker.io/library/busybox",
identifier: "1.30",
},
{
description: "with tag and digest",
image: "ubuntu:latest@sha256:868fd30a0e47b8d8ac485df174795b5e2fe8a6c8f056cc707b232d65b8a1ab68",
shouldErr: false,
name: "index.docker.io/library/ubuntu",
identifier: "sha256:868fd30a0e47b8d8ac485df174795b5e2fe8a6c8f056cc707b232d65b8a1ab68",
},
}
for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
ref, err := parseReference(test.image)
testutil.CheckError(t, test.shouldErr, err)
testutil.CheckDeepEqual(t, test.name, ref.Context().Name())
testutil.CheckDeepEqual(t, test.identifier, ref.Identifier())
})
}

}

7 changes: 4 additions & 3 deletions pkg/skaffold/build/kaniko/kaniko.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,12 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, tags tag.ImageTags,
return build.InParallel(ctx, out, tags, artifacts, b.buildArtifactWithKaniko)
}

func (b *Builder) buildArtifactWithKaniko(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error) {
func (b *Builder) buildArtifactWithKaniko(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, build.ConfigurationRetriever, error) {
digest, err := b.run(ctx, out, artifact, tag)
if err != nil {
return "", errors.Wrapf(err, "kaniko build for [%s]", artifact.ImageName)
return "", nil, errors.Wrapf(err, "kaniko build for [%s]", artifact.ImageName)
}

return tag + "@" + digest, nil
image := tag + "@" + digest
return image, build.RegistryConfigurationRetriever(image), nil
}
11 changes: 6 additions & 5 deletions pkg/skaffold/build/local/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,16 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, tags tag.ImageTags,
return build.InSequence(ctx, out, tags, artifacts, b.buildArtifact)
}

func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error) {
func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, build.ConfigurationRetriever, error) {
digestOrImageID, err := b.runBuildForArtifact(ctx, out, artifact, tag)
if err != nil {
return "", errors.Wrap(err, "build artifact")
return "", nil, errors.Wrap(err, "build artifact")
}

if b.pushImages {
digest := digestOrImageID
return tag + "@" + digest, nil
image := tag + "@" + digest
return image, build.RegistryConfigurationRetriever(image), nil
}

// k8s doesn't recognize the imageID or any combination of the image name
Expand All @@ -64,10 +65,10 @@ func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, artifact *la
imageID := digestOrImageID
uniqueTag := artifact.ImageName + ":" + strings.TrimPrefix(imageID, "sha256:")
if err := b.localDocker.Tag(ctx, imageID, uniqueTag); err != nil {
return "", err
return "", nil, err
}

return uniqueTag, nil
return uniqueTag, build.DockerConfigurationRetriever(b.localDocker, uniqueTag), nil
}

func (b *Builder) runBuildForArtifact(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error) {
Expand Down
11 changes: 10 additions & 1 deletion pkg/skaffold/build/local/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/warnings"
"github.com/GoogleContainerTools/skaffold/testutil"
"github.com/docker/docker/api/types"
"github.com/google/go-cmp/cmp"
)

type testAuthHelper struct{}
Expand Down Expand Up @@ -194,7 +195,15 @@ func TestLocalRun(t *testing.T) {

res, err := l.Build(context.Background(), ioutil.Discard, test.tags, test.artifacts)

testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, res)
testutil.CheckError(t, test.shouldErr, err)

// this feels like a total hack
filter := func(p cmp.Path) bool {
return p.Last().String() == ".Config"
}
ignoreConfigField := cmp.Options{cmp.FilterPath(filter, cmp.Ignore())}

testutil.CheckEqual(t, ignoreConfigField, test.expected, res)
testutil.CheckDeepEqual(t, test.expectedWarnings, fakeWarner.Warnings)
})
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/skaffold/build/parallel.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (

const bufferedLinesPerArtifact = 10000

type artifactBuilder func(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error)
type artifactBuilder func(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, ConfigurationRetriever, error)

// InParallel builds a list of artifacts in parallel but prints the logs in sequential order.
func InParallel(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact, buildArtifact artifactBuilder) ([]Artifact, error) {
Expand All @@ -43,6 +43,7 @@ func InParallel(ctx context.Context, out io.Writer, tags tag.ImageTags, artifact

n := len(artifacts)
finalTags := make([]string, n)
retrievers := make([]ConfigurationRetriever, n)
errs := make([]error, n)
outputs := make([]chan []byte, n)

Expand Down Expand Up @@ -70,7 +71,7 @@ func InParallel(ctx context.Context, out io.Writer, tags tag.ImageTags, artifact
if !present {
errs[i] = fmt.Errorf("unable to find tag for image %s", artifacts[i].ImageName)
} else {
finalTags[i], errs[i] = buildArtifact(ctx, cw, artifacts[i], tag)
finalTags[i], retrievers[i], errs[i] = buildArtifact(ctx, cw, artifacts[i], tag)
}

cw.Close()
Expand Down Expand Up @@ -101,6 +102,7 @@ func InParallel(ctx context.Context, out io.Writer, tags tag.ImageTags, artifact
built = append(built, Artifact{
ImageName: artifact.ImageName,
Tag: finalTags[i],
Config: retrievers[i],
})
}

Expand Down
Loading