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.

Sign up for GitHub

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

[v2] Major refactoring for v2 runner/runcontext #6077

Closed
wants to merge 1 commit into from
Closed

[v2] Major refactoring for v2 runner/runcontext #6077

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 1 addition & 2 deletions cmd/skaffold/app/cmd/apply.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
)

// NewCmdApply describes the CLI command to apply manifests to a cluster.
Expand All @@ -53,7 +52,7 @@ func doApply(ctx context.Context, out io.Writer, args []string) error {
if err := validateManifests(args); err != nil {
return err
}
return withRunner(ctx, out, func(r runner.Runner, configs []*latestV1.SkaffoldConfig) error {
return withRunner(ctx, out, func(r runner.Runner) error {
return r.Apply(ctx, out)
})
}
Expand Down
21 changes: 2 additions & 19 deletions cmd/skaffold/app/cmd/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ import (
"github.com/spf13/cobra"

"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/flags"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
)

var (
Expand Down Expand Up @@ -67,22 +65,19 @@ func doBuild(ctx context.Context, out io.Writer) error {
buildOut = ioutil.Discard
}

return withRunner(ctx, out, func(r runner.Runner, configs []*latestV1.SkaffoldConfig) error {
bRes, err := r.Build(ctx, buildOut, targetArtifacts(opts, configs))

return withRunner(ctx, out, func(r runner.Runner) error {
bRes, err := r.Build(ctx, buildOut, opts)
if quietFlag || buildOutputFlag != "" {
cmdOut := flags.BuildOutput{Builds: bRes}
var buildOutput bytes.Buffer
if err := buildFormatFlag.Template().Execute(&buildOutput, cmdOut); err != nil {
return fmt.Errorf("executing template: %w", err)
}

if quietFlag {
if _, err := out.Write(buildOutput.Bytes()); err != nil {
return fmt.Errorf("writing build output: %w", err)
}
}

if buildOutputFlag != "" {
if err := ioutil.WriteFile(buildOutputFlag, buildOutput.Bytes(), 0644); err != nil {
return fmt.Errorf("writing build output to file: %w", err)
Expand All @@ -93,15 +88,3 @@ func doBuild(ctx context.Context, out io.Writer) error {
return err
})
}

func targetArtifacts(opts config.SkaffoldOptions, configs []*latestV1.SkaffoldConfig) []*latestV1.Artifact {
var targetArtifacts []*latestV1.Artifact
for _, cfg := range configs {
for _, artifact := range cfg.Build.Artifacts {
if opts.IsTargetImage(artifact) {
targetArtifacts = append(targetArtifacts, artifact)
}
}
}
return targetArtifacts
}
52 changes: 35 additions & 17 deletions cmd/skaffold/app/cmd/build_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,17 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext/v1"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
"github.com/GoogleContainerTools/skaffold/testutil"
)

type mockRunner struct {
runner.Runner
}

func (r *mockRunner) Build(ctx context.Context, out io.Writer, artifacts []*latestV1.Artifact) ([]graph.Artifact, error) {
func (r *mockRunner) Build(ctx context.Context, out io.Writer, opts config.SkaffoldOptions) ([]graph.Artifact, error) {
out.Write([]byte("Build Completed"))
return []graph.Artifact{{
ImageName: "gcr.io/skaffold/example",
Expand All @@ -50,27 +51,34 @@ func (r *mockRunner) Stop() error {
}

func TestTagFlag(t *testing.T) {
mockCreateRunner := func(io.Writer, config.SkaffoldOptions) (runner.Runner, []*latestV1.SkaffoldConfig, *runcontext.RunContext, error) {
return &mockRunner{}, []*latestV1.SkaffoldConfig{{}}, nil, nil
mockCreateRunner := func(config.SkaffoldOptions, []util.VersionedConfig) (runner.Runner, *runcontext.RunContext, error) {
return &mockRunner{}, nil, nil
}
mockParseAllConfigs := func(io.Writer, config.SkaffoldOptions,
func(opts config.SkaffoldOptions) ([]util.VersionedConfig, error)) ([]util.VersionedConfig, error) {
return []util.VersionedConfig{&latestV1.SkaffoldConfig{}}, nil
}

testutil.Run(t, "override tag with argument", func(t *testutil.T) {
t.Override(&quietFlag, true)
t.Override(&opts.CustomTag, "tag")
t.Override(&createRunner, mockCreateRunner)

var output bytes.Buffer

err := doBuild(context.Background(), &output)
t.Override(&parseAllConfigs, mockParseAllConfigs)
output := bytes.NewBufferString("")
err := doBuild(context.Background(), output)

t.CheckNoError(err)
t.CheckDeepEqual(string([]byte(`{"builds":[{"imageName":"gcr.io/skaffold/example","tag":"test"}]}`)), output.String())
})
}

func TestQuietFlag(t *testing.T) {
mockCreateRunner := func(io.Writer, config.SkaffoldOptions) (runner.Runner, []*latestV1.SkaffoldConfig, *runcontext.RunContext, error) {
return &mockRunner{}, []*latestV1.SkaffoldConfig{{}}, nil, nil
mockCreateRunner := func(config.SkaffoldOptions, []util.VersionedConfig) (runner.Runner, *runcontext.RunContext, error) {
return &mockRunner{}, nil, nil
}
mockParseAllConfigs := func(io.Writer, config.SkaffoldOptions,
func(opts config.SkaffoldOptions) ([]util.VersionedConfig, error)) ([]util.VersionedConfig, error) {
return []util.VersionedConfig{&latestV1.SkaffoldConfig{}}, nil
}

tests := []struct {
Expand Down Expand Up @@ -101,6 +109,7 @@ func TestQuietFlag(t *testing.T) {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&quietFlag, true)
t.Override(&createRunner, mockCreateRunner)
t.Override(&parseAllConfigs, mockParseAllConfigs)
if test.template != "" {
t.Override(&buildFormatFlag, flags.NewTemplateFlag(test.template, flags.BuildOutput{}))
}
Expand All @@ -115,8 +124,12 @@ func TestQuietFlag(t *testing.T) {
}

func TestFileOutputFlag(t *testing.T) {
mockCreateRunner := func(io.Writer, config.SkaffoldOptions) (runner.Runner, []*latestV1.SkaffoldConfig, *runcontext.RunContext, error) {
return &mockRunner{}, []*latestV1.SkaffoldConfig{{}}, nil, nil
mockCreateRunner := func(config.SkaffoldOptions, []util.VersionedConfig) (runner.Runner, *runcontext.RunContext, error) {
return &mockRunner{}, nil, nil
}
mockParseAllConfigs := func(io.Writer, config.SkaffoldOptions,
func(opts config.SkaffoldOptions) ([]util.VersionedConfig, error)) ([]util.VersionedConfig, error) {
return []util.VersionedConfig{&latestV1.SkaffoldConfig{}}, nil
}

tests := []struct {
Expand Down Expand Up @@ -155,6 +168,7 @@ func TestFileOutputFlag(t *testing.T) {
t.Override(&quietFlag, test.quietFlag)
t.Override(&buildOutputFlag, test.filename)
t.Override(&createRunner, mockCreateRunner)
t.Override(&parseAllConfigs, mockParseAllConfigs)
if test.template != "" {
t.Override(&buildFormatFlag, flags.NewTemplateFlag(test.template, flags.BuildOutput{}))
}
Expand All @@ -178,16 +192,19 @@ func TestFileOutputFlag(t *testing.T) {
}

func TestRunBuild(t *testing.T) {
errRunner := func(io.Writer, config.SkaffoldOptions) (runner.Runner, []*latestV1.SkaffoldConfig, *runcontext.RunContext, error) {
return nil, nil, nil, errors.New("some error")
errRunner := func(config.SkaffoldOptions, []util.VersionedConfig) (runner.Runner, *runcontext.RunContext, error) {
return nil, nil, errors.New("some error")
}
mockCreateRunner := func(config.SkaffoldOptions, []util.VersionedConfig) (runner.Runner, *runcontext.RunContext, error) {
return &mockRunner{}, nil, nil
}
mockCreateRunner := func(io.Writer, config.SkaffoldOptions) (runner.Runner, []*latestV1.SkaffoldConfig, *runcontext.RunContext, error) {
return &mockRunner{}, []*latestV1.SkaffoldConfig{{}}, nil, nil
mockParseAllConfig := func(io.Writer, config.SkaffoldOptions, func(opts config.SkaffoldOptions) ([]util.VersionedConfig, error)) ([]util.VersionedConfig, error) {
return []util.VersionedConfig{&latestV1.SkaffoldConfig{}}, nil
}

tests := []struct {
description string
mock func(io.Writer, config.SkaffoldOptions) (runner.Runner, []*latestV1.SkaffoldConfig, *runcontext.RunContext, error)
mock func(config.SkaffoldOptions, []util.VersionedConfig) (runner.Runner, *runcontext.RunContext, error)
shouldErr bool
}{
{
Expand All @@ -204,6 +221,7 @@ func TestRunBuild(t *testing.T) {
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&createRunner, test.mock)
t.Override(&parseAllConfigs, mockParseAllConfig)

err := doBuild(context.Background(), ioutil.Discard)

Expand Down
21 changes: 0 additions & 21 deletions cmd/skaffold/app/cmd/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package cmd

import (
"context"
"fmt"
"io"
"os"
Expand All @@ -30,12 +29,10 @@ import (

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation/prompt"
kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/output"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/server"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/survey"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/update"
Expand Down Expand Up @@ -266,24 +263,6 @@ func setUpLogs(stdErr io.Writer, level string, timestamp bool) error {
return nil
}

// alwaysSucceedWhenCancelled returns nil if the context was cancelled.
// If the error is due to cancellation, return it as it gets swallowed
// in skaffold main.
// For all other errors, pass through known errors.
// TODO: Return nil if error is `context.Cancelled` and remove check in main.
func alwaysSucceedWhenCancelled(ctx context.Context, runCtx *runcontext.RunContext, err error) error {
if err == nil {
return err
}
// if the context was cancelled act as if all is well
if ctx.Err() == context.Canceled {
return nil
} else if err == context.Canceled {
return err
}
return sErrors.ShowAIError(runCtx, err)
}

func isHouseKeepingMessagesAllowed(cmd *cobra.Command) bool {
if cmd.Annotations == nil {
return false
Expand Down
8 changes: 4 additions & 4 deletions cmd/skaffold/app/cmd/debug_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import (

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext/v1"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
"github.com/GoogleContainerTools/skaffold/testutil"
)

Expand All @@ -49,8 +49,8 @@ func TestNewCmdDebug(t *testing.T) {
func TestDebugIndependentFromDev(t *testing.T) {
mockRunner := &mockDevRunner{}
testutil.Run(t, "DevDebug", func(t *testutil.T) {
t.Override(&createRunner, func(io.Writer, config.SkaffoldOptions) (runner.Runner, []*latestV1.SkaffoldConfig, *runcontext.RunContext, error) {
return mockRunner, []*latestV1.SkaffoldConfig{{}}, nil, nil
t.Override(&createRunner, func(config.SkaffoldOptions, []util.VersionedConfig) (runner.Runner, *runcontext.RunContext, error) {
return mockRunner, nil, nil
})
t.Override(&opts, config.SkaffoldOptions{})
t.Override(&doDev, func(context.Context, io.Writer) error {
Expand Down
3 changes: 1 addition & 2 deletions cmd/skaffold/app/cmd/delete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/spf13/cobra"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
)

// NewCmdDelete describes the CLI command to delete deployed resources.
Expand All @@ -35,7 +34,7 @@ func NewCmdDelete() *cobra.Command {
}

func doDelete(ctx context.Context, out io.Writer) error {
return withRunner(ctx, out, func(r runner.Runner, _ []*latestV1.SkaffoldConfig) error {
return withRunner(ctx, out, func(r runner.Runner) error {
return r.Cleanup(ctx, out)
})
}
9 changes: 2 additions & 7 deletions cmd/skaffold/app/cmd/deploy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/tips"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
)

var (
Expand All @@ -51,15 +50,11 @@ func NewCmdDeploy() *cobra.Command {
}

func doDeploy(ctx context.Context, out io.Writer) error {
return withRunner(ctx, out, func(r runner.Runner, configs []*latestV1.SkaffoldConfig) error {
return withRunner(ctx, out, func(r runner.Runner) error {
if opts.SkipRender {
return r.DeployAndLog(ctx, out, []graph.Artifact{})
}
var artifacts []*latestV1.Artifact
for _, cfg := range configs {
artifacts = append(artifacts, cfg.Build.Artifacts...)
}
buildArtifacts, err := getBuildArtifactsAndSetTags(artifacts, r.ApplyDefaultRepo)
buildArtifacts, err := getBuildArtifactsAndSetTags(r.GetArtifacts(), r.ApplyDefaultRepo)
if err != nil {
tips.PrintUseRunVsDeploy(out)
return err
Expand Down
9 changes: 2 additions & 7 deletions cmd/skaffold/app/cmd/dev.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/spf13/cobra"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
)

// for testing
Expand Down Expand Up @@ -60,12 +59,8 @@ func runDev(ctx context.Context, out io.Writer) error {
case <-ctx.Done():
return nil
default:
err := withRunner(ctx, out, func(r runner.Runner, configs []*latestV1.SkaffoldConfig) error {
var artifacts []*latestV1.Artifact
for _, cfg := range configs {
artifacts = append(artifacts, cfg.Build.Artifacts...)
}
err := r.Dev(ctx, out, artifacts)
err := withRunner(ctx, out, func(r runner.Runner) error {
err := r.Dev(ctx, out, r.GetArtifacts())

if r.HasDeployed() {
cleanup = func() {
Expand Down
Loading