Skip to content

Commit

Permalink
[v2] Decouple the runner creation from v1 schema but using schema int…
Browse files Browse the repository at this point in the history
…erface util.VersionedConfig.
  • Loading branch information
yuwenma committed Jun 23, 2021
1 parent d658049 commit 6dc89d8
Show file tree
Hide file tree
Showing 23 changed files with 214 additions and 163 deletions.
4 changes: 2 additions & 2 deletions cmd/skaffold/app/cmd/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ 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"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
)

// NewCmdApply describes the CLI command to apply manifests to a cluster.
Expand All @@ -53,7 +53,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, configs []util.VersionedConfig) error {
return r.Apply(ctx, out)
})
}
Expand Down
7 changes: 4 additions & 3 deletions cmd/skaffold/app/cmd/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
)

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

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

if quietFlag || buildOutputFlag != "" {
Expand All @@ -94,10 +95,10 @@ func doBuild(ctx context.Context, out io.Writer) error {
})
}

func targetArtifacts(opts config.SkaffoldOptions, configs []*latestV1.SkaffoldConfig) []*latestV1.Artifact {
func targetArtifacts(opts config.SkaffoldOptions, configs []util.VersionedConfig) []*latestV1.Artifact {
var targetArtifacts []*latestV1.Artifact
for _, cfg := range configs {
for _, artifact := range cfg.Build.Artifacts {
for _, artifact := range cfg.(*latestV1.SkaffoldConfig).Build.Artifacts {
if opts.IsTargetImage(artifact) {
targetArtifacts = append(targetArtifacts, artifact)
}
Expand Down
21 changes: 11 additions & 10 deletions cmd/skaffold/app/cmd/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"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"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
"github.com/GoogleContainerTools/skaffold/testutil"
)

Expand All @@ -50,8 +51,8 @@ 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(io.Writer, config.SkaffoldOptions) (runner.Runner, []util.VersionedConfig, *runcontext.RunContext, error) {
return &mockRunner{}, []util.VersionedConfig{&latestV1.SkaffoldConfig{}}, nil, nil
}

testutil.Run(t, "override tag with argument", func(t *testutil.T) {
Expand All @@ -69,8 +70,8 @@ func TestTagFlag(t *testing.T) {
}

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(io.Writer, config.SkaffoldOptions) (runner.Runner, []util.VersionedConfig, *runcontext.RunContext, error) {
return &mockRunner{}, []util.VersionedConfig{&latestV1.SkaffoldConfig{}}, nil, nil
}

tests := []struct {
Expand Down Expand Up @@ -115,8 +116,8 @@ 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(io.Writer, config.SkaffoldOptions) (runner.Runner, []util.VersionedConfig, *runcontext.RunContext, error) {
return &mockRunner{}, []util.VersionedConfig{&latestV1.SkaffoldConfig{}}, nil, nil
}

tests := []struct {
Expand Down Expand Up @@ -178,16 +179,16 @@ func TestFileOutputFlag(t *testing.T) {
}

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

tests := []struct {
description string
mock func(io.Writer, config.SkaffoldOptions) (runner.Runner, []*latestV1.SkaffoldConfig, *runcontext.RunContext, error)
mock func(io.Writer, config.SkaffoldOptions) (runner.Runner, []util.VersionedConfig, *runcontext.RunContext, error)
shouldErr bool
}{
{
Expand Down
5 changes: 3 additions & 2 deletions cmd/skaffold/app/cmd/debug_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"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"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
"github.com/GoogleContainerTools/skaffold/testutil"
)

Expand All @@ -49,8 +50,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(io.Writer, config.SkaffoldOptions) (runner.Runner, []util.VersionedConfig, *runcontext.RunContext, error) {
return mockRunner, []util.VersionedConfig{&latestV1.SkaffoldConfig{}}, nil, nil
})
t.Override(&opts, config.SkaffoldOptions{})
t.Override(&doDev, func(context.Context, io.Writer) error {
Expand Down
4 changes: 2 additions & 2 deletions cmd/skaffold/app/cmd/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"github.com/spf13/cobra"

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

// NewCmdDelete describes the CLI command to delete deployed resources.
Expand All @@ -35,7 +35,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, _ []util.VersionedConfig) error {
return r.Cleanup(ctx, out)
})
}
5 changes: 3 additions & 2 deletions cmd/skaffold/app/cmd/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
)

var (
Expand All @@ -51,13 +52,13 @@ 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, configs []util.VersionedConfig) 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...)
artifacts = append(artifacts, cfg.(*latestV1.SkaffoldConfig).Build.Artifacts...)
}
buildArtifacts, err := getBuildArtifactsAndSetTags(artifacts, r.ApplyDefaultRepo)
if err != nil {
Expand Down
7 changes: 5 additions & 2 deletions cmd/skaffold/app/cmd/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

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

// for testing
Expand Down Expand Up @@ -60,10 +61,12 @@ 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 {
// Note: The latestV1.SkaffoldConfig is used for both latestV1 schema and latestV2 schema because
// the latestV1 and latestV2 use the same Build struct. Ideally they should be separated.
err := withRunner(ctx, out, func(r runner.Runner, configs []util.VersionedConfig) error {
var artifacts []*latestV1.Artifact
for _, cfg := range configs {
artifacts = append(artifacts, cfg.Build.Artifacts...)
artifacts = append(artifacts, cfg.(*latestV1.SkaffoldConfig).Build.Artifacts...)
}
err := r.Dev(ctx, out, artifacts)

Expand Down
9 changes: 5 additions & 4 deletions cmd/skaffold/app/cmd/dev_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"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"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
"github.com/GoogleContainerTools/skaffold/testutil"
)

Expand Down Expand Up @@ -96,8 +97,8 @@ func TestDoDev(t *testing.T) {
hasDeployed: test.hasDeployed,
errDev: context.Canceled,
}
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(io.Writer, config.SkaffoldOptions) (runner.Runner, []util.VersionedConfig, *runcontext.RunContext, error) {
return mockRunner, []util.VersionedConfig{&latestV1.SkaffoldConfig{}}, nil, nil
})
t.Override(&opts, config.SkaffoldOptions{
Cleanup: true,
Expand Down Expand Up @@ -146,8 +147,8 @@ func TestDevConfigChange(t *testing.T) {
testutil.Run(t, "test config change", func(t *testutil.T) {
mockRunner := &mockConfigChangeRunner{}

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(io.Writer, config.SkaffoldOptions) (runner.Runner, []util.VersionedConfig, *runcontext.RunContext, error) {
return mockRunner, []util.VersionedConfig{&latestV1.SkaffoldConfig{}}, nil, nil
})
t.Override(&opts, config.SkaffoldOptions{
Cleanup: true,
Expand Down
8 changes: 5 additions & 3 deletions cmd/skaffold/app/cmd/diagnose.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/parser"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
schemaUtil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/version"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/yaml"
Expand Down Expand Up @@ -66,7 +67,7 @@ func doDiagnose(ctx context.Context, out io.Writer) error {
}
// remove the dependency config references since they have already been imported and will be marshalled together.
for i := range configs {
configs[i].Dependencies = nil
configs[i].(*latestV1.SkaffoldConfig).Dependencies = nil
}
buf, err := yaml.MarshalWithSeparator(configs)
if err != nil {
Expand All @@ -77,12 +78,13 @@ func doDiagnose(ctx context.Context, out io.Writer) error {
return nil
}

func printArtifactDiagnostics(ctx context.Context, out io.Writer, configs []*latestV1.SkaffoldConfig) error {
func printArtifactDiagnostics(ctx context.Context, out io.Writer, configs []schemaUtil.VersionedConfig) error {
runCtx, err := getRunContext(opts, configs)
if err != nil {
return fmt.Errorf("getting run context: %w", err)
}
for _, config := range configs {
for _, c := range configs {
config := c.(*latestV1.SkaffoldConfig)
fmt.Fprintln(out, "Skaffold version:", version.Get().GitCommit)
fmt.Fprintln(out, "Configuration version:", config.APIVersion)
fmt.Fprintln(out, "Number of artifacts:", len(config.Build.Artifacts))
Expand Down
11 changes: 6 additions & 5 deletions cmd/skaffold/app/cmd/diagnose_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
"github.com/GoogleContainerTools/skaffold/testutil"
)

Expand Down Expand Up @@ -57,20 +58,20 @@ metadata:

for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&getRunContext, func(config.SkaffoldOptions, []*latestV1.SkaffoldConfig) (*runcontext.RunContext, error) {
t.Override(&getRunContext, func(config.SkaffoldOptions, []util.VersionedConfig) (*runcontext.RunContext, error) {
return nil, fmt.Errorf("cannot get the runtime context")
})
t.Override(&yamlOnly, test.yamlOnly)
t.Override(&getCfgs, func(opts config.SkaffoldOptions) ([]*latestV1.SkaffoldConfig, error) {
return []*latestV1.SkaffoldConfig{
{
t.Override(&getCfgs, func(opts config.SkaffoldOptions) ([]util.VersionedConfig, error) {
return []util.VersionedConfig{
&latestV1.SkaffoldConfig{
APIVersion: "testVersion",
Kind: "Config",
Metadata: latestV1.Metadata{
Name: "config1",
},
},
{
&latestV1.SkaffoldConfig{
APIVersion: "testVersion",
Kind: "Config",
Metadata: latestV1.Metadata{
Expand Down
7 changes: 4 additions & 3 deletions cmd/skaffold/app/cmd/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/manifest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
)

// for tests
Expand Down Expand Up @@ -58,7 +59,7 @@ func NewCmdFilter() *cobra.Command {
// runFilter loads the Kubernetes manifests from stdin and applies the debug transformations.
// Unlike `skaffold debug`, this filtering affects all images and not just the built artifacts.
func runFilter(ctx context.Context, out io.Writer, debuggingFilters bool, buildArtifacts []graph.Artifact) error {
return withRunner(ctx, out, func(r runner.Runner, configs []*latestV1.SkaffoldConfig) error {
return withRunner(ctx, out, func(r runner.Runner, configs []util.VersionedConfig) error {
manifestList, err := manifest.Load(os.Stdin)
if err != nil {
return fmt.Errorf("loading manifests: %w", err)
Expand Down Expand Up @@ -87,7 +88,7 @@ func runFilter(ctx context.Context, out io.Writer, debuggingFilters bool, buildA
})
}

func getInsecureRegistries(opts config.SkaffoldOptions, configs []*latestV1.SkaffoldConfig) (map[string]bool, error) {
func getInsecureRegistries(opts config.SkaffoldOptions, configs []util.VersionedConfig) (map[string]bool, error) {
cfgRegistries, err := config.GetInsecureRegistries(opts.GlobalConfig)
if err != nil {
return nil, err
Expand All @@ -96,7 +97,7 @@ func getInsecureRegistries(opts config.SkaffoldOptions, configs []*latestV1.Skaf

regList = append(regList, opts.InsecureRegistries...)
for _, cfg := range configs {
regList = append(regList, cfg.Build.InsecureRegistries...)
regList = append(regList, cfg.(*latestV1.SkaffoldConfig).Build.InsecureRegistries...)
}
regList = append(regList, cfgRegistries...)
insecureRegistries := make(map[string]bool, len(regList))
Expand Down
4 changes: 2 additions & 2 deletions cmd/skaffold/app/cmd/generate_pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (

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

var (
Expand All @@ -44,7 +44,7 @@ func NewCmdGeneratePipeline() *cobra.Command {
}

func doGeneratePipeline(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, configs []util.VersionedConfig) error {
if err := r.GeneratePipeline(ctx, out, configs, configFiles, "pipeline.yaml"); err != nil {
return fmt.Errorf("generating : %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/skaffold/app/cmd/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/flags"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
)

var (
Expand Down Expand Up @@ -60,7 +60,7 @@ func doRender(ctx context.Context, out io.Writer) error {
buildOut = out
}

return withRunner(ctx, out, func(r runner.Runner, configs []*latestV1.SkaffoldConfig) error {
return withRunner(ctx, out, func(r runner.Runner, configs []util.VersionedConfig) error {
var bRes []graph.Artifact

if renderFromBuildOutputFile.String() != "" {
Expand Down
4 changes: 2 additions & 2 deletions cmd/skaffold/app/cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (

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

// NewCmdRun describes the CLI command to run a pipeline.
Expand All @@ -41,7 +41,7 @@ func NewCmdRun() *cobra.Command {
}

func doRun(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, configs []util.VersionedConfig) error {
bRes, err := r.Build(ctx, out, targetArtifacts(opts, configs))
if err != nil {
return fmt.Errorf("failed to build: %w", err)
Expand Down
Loading

0 comments on commit 6dc89d8

Please sign in to comment.