[V3] New V3 SkaffoldRunner (#5692)

* [V3] New V3 SkaffoldRunner 1. Refactored the runner directory. 2. Create a New SkaffoldRunner for V3. Both runner.SkaffoldRunner and v3.SkaffoldRunner implements the Runner interface. 2. Build, Test, Prune and other shared methods are moved from SkaffoldRunner to several individual structs (BuildRunner, TestRunner). 3. Both SkaffoldRunner and the V3 SkaffoldRunner have the shared structs as components. 4. Unittests are updated. 5. Add placeholders (not implemented error) in v3 SkaffoldRunner for later implementation. * Rename BuildRunner TestRunner and PruneRunner to runner.Builder runner.Tester and runner.Pruner
GoogleContainerTools · Apr 22, 2021 · 21debe2 · 21debe2
1 parent ca4886f
commit 21debe2
Show file tree

Hide file tree

Showing 21 changed files with 403 additions and 148 deletions.
diff --git a/go.mod b/go.mod
@@ -17,6 +17,7 @@ require (
 	cloud.google.com/go/storage v1.10.0
 	github.com/AlecAivazis/survey/v2 v2.2.7
 	github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.13.0
+	github.com/aws/aws-sdk-go v1.36.30 // indirect
 	github.com/blang/semver v3.5.1+incompatible
 	github.com/bmatcuk/doublestar v1.2.4
 	github.com/buildpacks/imgutil v0.0.0-20201211223552-8581300fe2b2
@@ -49,6 +50,9 @@ require (
 	github.com/mitchellh/go-homedir v1.1.0
 	// github.com/moby/buildkit v0.7.1
 	github.com/moby/buildkit v0.8.0
+	github.com/nxadm/tail v1.4.8 // indirect
+	github.com/onsi/ginkgo v1.15.0 // indirect
+	github.com/onsi/gomega v1.10.5 // indirect
 	github.com/opencontainers/go-digest v1.0.0
 	github.com/opencontainers/image-spec v1.0.1
 	github.com/pkg/browser v0.0.0-20180916011732-0a3d74bf9ce4
@@ -57,27 +61,30 @@ require (
 	github.com/rjeczalik/notify v0.9.3-0.20201210012515-e2a77dcc14cf
 	github.com/russross/blackfriday/v2 v2.0.1
 	github.com/segmentio/textio v1.2.0
-	github.com/sirupsen/logrus v1.7.0
-	github.com/spf13/cobra v1.1.1
+	github.com/sirupsen/logrus v1.8.1
+	github.com/spf13/cobra v1.1.3
 	github.com/spf13/pflag v1.0.5
+	github.com/stretchr/testify v1.7.0 // indirect
 	github.com/tektoncd/pipeline v0.5.1-0.20190731183258-9d7e37e85bf8
 	github.com/xeipuuv/gojsonschema v1.2.0
 	go.opentelemetry.io/otel v0.13.0
 	go.opentelemetry.io/otel/exporters/stdout v0.13.0
 	go.opentelemetry.io/otel/sdk v0.13.0
+	golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83 // indirect
 	golang.org/x/mod v0.4.1
+	golang.org/x/net v0.0.0-20210226172049-e18ecbb05110 // indirect
 	golang.org/x/oauth2 v0.0.0-20201109201403-9fd604954f58
 	golang.org/x/sync v0.0.0-20201207232520-09787c993a3a
-	golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c
+	golang.org/x/sys v0.0.0-20210228012217-479acdf4ea46
 	golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1
-	golang.org/x/tools v0.1.0 // indirect
 	google.golang.org/api v0.35.0
 	google.golang.org/appengine v1.6.7 // indirect
 	google.golang.org/genproto v0.0.0-20201202151023-55d61f90c1ce
 	google.golang.org/grpc v1.33.2
 	google.golang.org/protobuf v1.25.0
 	gopkg.in/yaml.v2 v2.4.0
 	gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776
+	honnef.co/go/tools v0.1.3 // indirect
 	k8s.io/api v0.19.7
 	k8s.io/apimachinery v0.19.7
 	k8s.io/client-go v0.19.7

diff --git a/go.sum b/go.sum
diff --git a/pkg/skaffold/runner/build_deploy.go b/pkg/skaffold/runner/build_deploy.go
@@ -26,18 +26,35 @@ import (
 	"github.com/sirupsen/logrus"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/cache"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
 	deployutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/util"
 	eventV2 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event/v2"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/tag"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 )
 
+type Builder struct {
+	builder build.Builder
+	tagger  tag.Tagger
+	cache   cache.Cache
+	builds  []graph.Artifact
+
+	// podSelector is used to determine relevant pods for logging and portForwarding
+	podSelector *kubernetes.ImageList
+
+	hasBuilt     bool
+	devIteration int
+	runCtx       *runcontext.RunContext
+}
+
 // Build builds a list of artifacts.
-func (r *SkaffoldRunner) Build(ctx context.Context, out io.Writer, artifacts []*latest.Artifact) ([]graph.Artifact, error) {
+func (r *Builder) Build(ctx context.Context, out io.Writer, artifacts []*latest.Artifact) ([]graph.Artifact, error) {
 	eventV2.TaskInProgress(constants.Build, r.devIteration)
 
 	// Use tags directly from the Kubernetes manifests.
@@ -99,55 +116,13 @@ func (r *SkaffoldRunner) Build(ctx context.Context, out io.Writer, artifacts []*
 	return bRes, nil
 }
 
-// Test tests a list of already built artifacts.
-func (r *SkaffoldRunner) Test(ctx context.Context, out io.Writer, artifacts []graph.Artifact) error {
-	eventV2.TaskInProgress(constants.Test, r.devIteration)
-	if err := r.tester.Test(ctx, out, artifacts); err != nil {
-		eventV2.TaskFailed(constants.Test, r.devIteration, err)
-		return err
-	}
-
-	eventV2.TaskSucceeded(constants.Test, r.devIteration)
-	return nil
-}
-
-// DeployAndLog deploys a list of already built artifacts and optionally show the logs.
-func (r *SkaffoldRunner) DeployAndLog(ctx context.Context, out io.Writer, artifacts []graph.Artifact) error {
-	// Update which images are logged.
-	r.addTagsToPodSelector(artifacts)
-
-	logger := r.createLogger(out, artifacts)
-	defer logger.Stop()
-
-	// Logs should be retrieved up to just before the deploy
-	logger.SetSince(time.Now())
-	// First deploy
-	if err := r.Deploy(ctx, out, artifacts); err != nil {
-		return err
-	}
-
-	forwarderManager := r.createForwarder(out)
-	defer forwarderManager.Stop()
-
-	if err := forwarderManager.Start(ctx, r.runCtx.GetNamespaces()); err != nil {
-		logrus.Warnln("Error starting port forwarding:", err)
-	}
-
-	// Start printing the logs after deploy is finished
-	if err := logger.Start(ctx, r.runCtx.GetNamespaces()); err != nil {
-		return fmt.Errorf("starting logger: %w", err)
-	}
-
-	if r.runCtx.Tail() || r.runCtx.PortForward() {
-		color.Yellow.Fprintln(out, "Press Ctrl+C to exit")
-		<-ctx.Done()
-	}
-
-	return nil
+// HasBuilt returns true if this runner has built something.
+func (r *Builder) HasBuilt() bool {
+	return r.hasBuilt
 }
 
 // Update which images are logged.
-func (r *SkaffoldRunner) addTagsToPodSelector(artifacts []graph.Artifact) {
+func (r *Builder) addTagsToPodSelector(artifacts []graph.Artifact) {
 	for _, artifact := range artifacts {
 		r.podSelector.Add(artifact.Tag)
 	}
@@ -159,12 +134,12 @@ type tagErr struct {
 }
 
 // ApplyDefaultRepo applies the default repo to a given image tag.
-func (r *SkaffoldRunner) ApplyDefaultRepo(tag string) (string, error) {
+func (r *Builder) ApplyDefaultRepo(tag string) (string, error) {
 	return deployutil.ApplyDefaultRepo(r.runCtx.GlobalConfig(), r.runCtx.DefaultRepo(), tag)
 }
 
 // imageTags generates tags for a list of artifacts
-func (r *SkaffoldRunner) imageTags(ctx context.Context, out io.Writer, artifacts []*latest.Artifact) (tag.ImageTags, error) {
+func (r *Builder) imageTags(ctx context.Context, out io.Writer, artifacts []*latest.Artifact) (tag.ImageTags, error) {
 	start := time.Now()
 	color.Default.Fprintln(out, "Generating tags...")
 

diff --git a/pkg/skaffold/runner/changeset.go b/pkg/skaffold/runner/changeset.go
@@ -21,7 +21,7 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/sync"
 )
 
-type changeSet struct {
+type ChangeSet struct {
 	needsRebuild   []*latest.Artifact
 	rebuildTracker map[string]*latest.Artifact
 	needsResync    []*sync.Item
@@ -31,7 +31,7 @@ type changeSet struct {
 	needsReload    bool
 }
 
-func (c *changeSet) AddRebuild(a *latest.Artifact) {
+func (c *ChangeSet) AddRebuild(a *latest.Artifact) {
 	if _, ok := c.rebuildTracker[a.ImageName]; ok {
 		return
 	}
@@ -43,14 +43,14 @@ func (c *changeSet) AddRebuild(a *latest.Artifact) {
 	c.needsRebuild = append(c.needsRebuild, a)
 }
 
-func (c *changeSet) AddRetest(a *latest.Artifact) {
+func (c *ChangeSet) AddRetest(a *latest.Artifact) {
 	if c.needsRetest == nil {
 		c.needsRetest = make(map[string]bool)
 	}
 	c.needsRetest[a.ImageName] = true
 }
 
-func (c *changeSet) AddResync(s *sync.Item) {
+func (c *ChangeSet) AddResync(s *sync.Item) {
 	if _, ok := c.resyncTracker[s.Image]; ok {
 		return
 	}
@@ -62,20 +62,20 @@ func (c *changeSet) AddResync(s *sync.Item) {
 	c.needsResync = append(c.needsResync, s)
 }
 
-func (c *changeSet) resetBuild() {
+func (c *ChangeSet) resetBuild() {
 	c.rebuildTracker = make(map[string]*latest.Artifact)
 	c.needsRebuild = nil
 }
 
-func (c *changeSet) resetSync() {
+func (c *ChangeSet) resetSync() {
 	c.resyncTracker = make(map[string]*sync.Item)
 	c.needsResync = nil
 }
 
-func (c *changeSet) resetDeploy() {
+func (c *ChangeSet) resetDeploy() {
 	c.needsRedeploy = false
 }
 
-func (c *changeSet) resetTest() {
+func (c *ChangeSet) resetTest() {
 	c.needsRetest = make(map[string]bool)
 }
diff --git a/pkg/skaffold/runner/deploy.go b/pkg/skaffold/runner/deploy.go
@@ -37,6 +37,46 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 )
 
+// DeployAndLog deploys a list of already built artifacts and optionally show the logs.
+func (r *SkaffoldRunner) DeployAndLog(ctx context.Context, out io.Writer, artifacts []graph.Artifact) error {
+	eventV2.TaskInProgress(constants.Deploy, r.devIteration)
+
+	// Update which images are logged.
+	r.addTagsToPodSelector(artifacts)
+
+	logger := r.createLogger(out, artifacts)
+	defer logger.Stop()
+
+	// Logs should be retrieved up to just before the deploy
+	logger.SetSince(time.Now())
+	// First deploy
+	if err := r.Deploy(ctx, out, artifacts); err != nil {
+		eventV2.TaskFailed(constants.Deploy, r.devIteration, err)
+		return err
+	}
+
+	forwarderManager := r.createForwarder(out)
+	defer forwarderManager.Stop()
+
+	if err := forwarderManager.Start(ctx, r.runCtx.GetNamespaces()); err != nil {
+		logrus.Warnln("Error starting port forwarding:", err)
+	}
+
+	// Start printing the logs after deploy is finished
+	if err := logger.Start(ctx, r.runCtx.GetNamespaces()); err != nil {
+		eventV2.TaskFailed(constants.Deploy, r.devIteration, err)
+		return fmt.Errorf("starting logger: %w", err)
+	}
+
+	if r.runCtx.Tail() || r.runCtx.PortForward() {
+		color.Yellow.Fprintln(out, "Press Ctrl+C to exit")
+		<-ctx.Done()
+	}
+
+	eventV2.TaskSucceeded(constants.Deploy, r.devIteration)
+	return nil
+}
+
 func (r *SkaffoldRunner) Deploy(ctx context.Context, out io.Writer, artifacts []graph.Artifact) error {
 	if r.runCtx.RenderOnly() {
 		return r.Render(ctx, out, artifacts, false, r.runCtx.RenderOutput())

diff --git a/pkg/skaffold/runner/dev_test.go b/pkg/skaffold/runner/dev_test.go
@@ -298,7 +298,7 @@ func TestDevAutoTriggers(t *testing.T) {
 		expectedActions []Actions
 		autoTriggers    triggerState // the state of auto triggers
 		singleTriggers  triggerState // the state of single intent triggers at the end of dev loop
-		userIntents     []func(i *intents)
+		userIntents     []func(i *Intents)
 	}{
 		{
 			description: "build on; sync on; deploy on",
@@ -368,8 +368,8 @@ func TestDevAutoTriggers(t *testing.T) {
 			autoTriggers:    triggerState{false, false, false},
 			singleTriggers:  triggerState{false, false, false},
 			expectedActions: []Actions{},
-			userIntents: []func(i *intents){
-				func(i *intents) {
+			userIntents: []func(i *Intents){
+				func(i *Intents) {
 					i.setBuild(true)
 				},
 			},
@@ -380,8 +380,8 @@ func TestDevAutoTriggers(t *testing.T) {
 			autoTriggers:    triggerState{false, false, false},
 			singleTriggers:  triggerState{false, false, false},
 			expectedActions: []Actions{},
-			userIntents: []func(i *intents){
-				func(i *intents) {
+			userIntents: []func(i *Intents){
+				func(i *Intents) {
 					i.setBuild(true)
 					i.setSync(true)
 				},
@@ -393,11 +393,11 @@ func TestDevAutoTriggers(t *testing.T) {
 			autoTriggers:    triggerState{false, false, false},
 			singleTriggers:  triggerState{false, false, false},
 			expectedActions: []Actions{},
-			userIntents: []func(i *intents){
-				func(i *intents) {
+			userIntents: []func(i *Intents){
+				func(i *Intents) {
 					i.setBuild(true)
 				},
-				func(i *intents) {
+				func(i *Intents) {
 					i.setSync(true)
 				},
 			},