Skip to content

Commit

Permalink
Pass output directly to the Runner
Browse files Browse the repository at this point in the history 
Signed-off-by: David Gageot <david@gageot.net>
  • Loading branch information
@dgageot
dgageot committed May 12, 2018
1 parent c20f522 commit d590165
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 46 deletions.
3 changes: 1 addition & 2 deletions cmd/skaffold/app/cmd/cmd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ func NewRunner(out io.Writer, filename string) (*runner.SkaffoldRunner, error) {
return nil, errors.Wrap(err, "reading configuration")
}

opts.Output = out
r, err := runner.NewForConfig(opts, config)
r, err := runner.NewForConfig(opts, config, out)
if err != nil {
return nil, errors.Wrap(err, "getting skaffold config")
}
Expand Down
3 changes: 0 additions & 3 deletions pkg/skaffold/config/options.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,11 @@ limitations under the License.

package config

import "io"

// SkaffoldOptions are options that are set by command line arguments not included
// in the config file itself
type SkaffoldOptions struct {
Cleanup bool
Notification bool
Profiles []string
CustomTag string
Output io.Writer
}
33 changes: 18 additions & 15 deletions pkg/skaffold/runner/runner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package runner
import (
"context"
"fmt"
"io"
"os"
"os/signal"
"syscall"
Expand Down Expand Up @@ -50,12 +51,13 @@ type SkaffoldRunner struct {
kubeclient clientgo.Interface
builds []build.Build
depMap *build.DependencyMap
out io.Writer
}

var kubernetesClient = kubernetes.GetClientset

// NewForConfig returns a new SkaffoldRunner for a SkaffoldConfig
func NewForConfig(opts *config.SkaffoldOptions, cfg *config.SkaffoldConfig) (*SkaffoldRunner, error) {
func NewForConfig(opts *config.SkaffoldOptions, cfg *config.SkaffoldConfig, out io.Writer) (*SkaffoldRunner, error) {
kubeContext, err := kubernetes.CurrentContext()
if err != nil {
return nil, errors.Wrap(err, "getting current cluster context")
Expand Down Expand Up @@ -90,6 +92,7 @@ func NewForConfig(opts *config.SkaffoldOptions, cfg *config.SkaffoldConfig) (*Sk
opts: opts,
kubeclient: client,
WatcherFactory: watch.NewWatcher,
out: out,
}, nil
}

Expand Down Expand Up @@ -146,7 +149,7 @@ func (r *SkaffoldRunner) Build(ctx context.Context) error {
bRes, err := r.build(ctx, r.config.Build.Artifacts)

for _, res := range bRes.Builds {
fmt.Fprintf(r.opts.Output, "%s -> %s\n", res.ImageName, res.Tag)
fmt.Fprintf(r.out, "%s -> %s\n", res.ImageName, res.Tag)
}

return err
Expand Down Expand Up @@ -194,7 +197,7 @@ func (r *SkaffoldRunner) watchBuildDeploy(ctx context.Context) error {

podSelector := kubernetes.NewImageList()
colorPicker := kubernetes.NewColorPicker(artifacts)
logger := kubernetes.NewLogAggregator(r.opts.Output, podSelector, colorPicker)
logger := kubernetes.NewLogAggregator(r.out, podSelector, colorPicker)

onBuildSuccess := func(bRes *build.BuildResult) {
// Update which images are logged with which color
Expand All @@ -214,7 +217,7 @@ func (r *SkaffoldRunner) watchBuildDeploy(ctx context.Context) error {
logrus.Warnf("run: %s", err)
}

fmt.Fprint(r.opts.Output, "Watching for changes...\n")
fmt.Fprint(r.out, "Watching for changes...\n")
logger.Unmute()
}

Expand All @@ -226,7 +229,7 @@ func (r *SkaffoldRunner) watchBuildDeploy(ctx context.Context) error {
if err != nil {
logrus.Warnf("deploy: %s", err)
}
fmt.Fprint(r.opts.Output, "Watching for changes...\n")
fmt.Fprint(r.out, "Watching for changes...\n")
logger.Unmute()
}

Expand Down Expand Up @@ -274,31 +277,31 @@ func (r *SkaffoldRunner) buildAndDeploy(ctx context.Context, artifacts []*v1alph

func (r *SkaffoldRunner) build(ctx context.Context, artifacts []*v1alpha2.Artifact) (*build.BuildResult, error) {
start := time.Now()
fmt.Fprintln(r.opts.Output, "Starting build...")
fmt.Fprintln(r.out, "Starting build...")

bRes, err := r.Builder.Build(ctx, r.opts.Output, r.Tagger, artifacts)
bRes, err := r.Builder.Build(ctx, r.out, r.Tagger, artifacts)
if err != nil {
return nil, errors.Wrap(err, "build step")
}

fmt.Fprintln(r.opts.Output, "Build complete in", time.Since(start))
fmt.Fprintln(r.out, "Build complete in", time.Since(start))

return bRes, nil
}

func (r *SkaffoldRunner) deploy(ctx context.Context, bRes *build.BuildResult) (*deploy.Result, error) {
start := time.Now()
fmt.Fprintln(r.opts.Output, "Starting deploy...")
fmt.Fprintln(r.out, "Starting deploy...")

dRes, err := r.Deployer.Deploy(ctx, r.opts.Output, bRes)
dRes, err := r.Deployer.Deploy(ctx, r.out, bRes)
if err != nil {
return nil, errors.Wrap(err, "deploy step")
}
if r.opts.Notification {
fmt.Fprint(r.opts.Output, constants.TerminalBell)
fmt.Fprint(r.out, constants.TerminalBell)
}

fmt.Fprintln(r.opts.Output, "Deploy complete in", time.Since(start))
fmt.Fprintln(r.out, "Deploy complete in", time.Since(start))

return dRes, nil
}
Expand All @@ -325,15 +328,15 @@ func cleanUpOnCtrlC(ctx context.Context, runDevMode func(context.Context) error,

func (r *SkaffoldRunner) cleanup(ctx context.Context) {
start := time.Now()
fmt.Fprintln(r.opts.Output, "Cleaning up...")
fmt.Fprintln(r.out, "Cleaning up...")

err := r.Deployer.Cleanup(ctx, r.opts.Output)
err := r.Deployer.Cleanup(ctx, r.out)
if err != nil {
logrus.Warnf("cleanup: %s", err)
return
}

fmt.Fprintln(r.opts.Output, "Cleanup complete in", time.Since(start))
fmt.Fprintln(r.out, "Cleanup complete in", time.Since(start))
}

func mergeWithPreviousBuilds(builds, previous []build.Build) []build.Build {
Expand Down
45 changes: 19 additions & 26 deletions pkg/skaffold/runner/runner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ limitations under the License.
package runner

import (
"bytes"
"context"
"fmt"
"io"
"io/ioutil"
"testing"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
Expand Down Expand Up @@ -215,7 +215,7 @@ func TestNewForConfig(t *testing.T) {
}
for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
cfg, err := NewForConfig(&config.SkaffoldOptions{}, test.config)
cfg, err := NewForConfig(&config.SkaffoldOptions{}, test.config, ioutil.Discard)
testutil.CheckError(t, test.shouldErr, err)
if cfg != nil {
testutil.CheckErrorAndTypeEquality(t, test.shouldErr, err, test.expected, cfg.Builder)
Expand All @@ -239,13 +239,12 @@ func TestRun(t *testing.T) {
res: &build.BuildResult{},
},
kubeclient: client,
opts: &config.SkaffoldOptions{
Output: &bytes.Buffer{},
},
Tagger: &tag.ChecksumTagger{},
opts: &config.SkaffoldOptions{},
Tagger: &tag.ChecksumTagger{},
Deployer: &TestDeployer{
res: &deploy.Result{},
},
out: ioutil.Discard,
},
},
{
Expand All @@ -256,10 +255,9 @@ func TestRun(t *testing.T) {
Builder: &TestBuilder{
err: fmt.Errorf(""),
},
opts: &config.SkaffoldOptions{
Output: &bytes.Buffer{},
},
opts: &config.SkaffoldOptions{},
Tagger: &tag.ChecksumTagger{},
out: ioutil.Discard,
},
shouldErr: true,
},
Expand All @@ -278,14 +276,13 @@ func TestRun(t *testing.T) {
},
},
},
opts: &config.SkaffoldOptions{
Output: &bytes.Buffer{},
},
opts: &config.SkaffoldOptions{},
kubeclient: client,
Tagger: &tag.ChecksumTagger{},
Builder: &TestBuilder{
res: &build.BuildResult{},
},
out: ioutil.Discard,
},
shouldErr: true,
},
Expand Down Expand Up @@ -324,10 +321,9 @@ func TestDev(t *testing.T) {
},
Deployer: &TestDeployer{},
WatcherFactory: NewWatcherFactory(nil, []string{}),
opts: &config.SkaffoldOptions{
Output: &bytes.Buffer{},
},
Tagger: &tag.ChecksumTagger{},
opts: &config.SkaffoldOptions{},
Tagger: &tag.ChecksumTagger{},
out: ioutil.Discard,
},
},
{
Expand All @@ -342,9 +338,8 @@ func TestDev(t *testing.T) {
Deployer: &TestDeployer{},
Tagger: &TestTagger{},
WatcherFactory: NewWatcherFactory(nil, []string{}),
opts: &config.SkaffoldOptions{
Output: &bytes.Buffer{},
},
opts: &config.SkaffoldOptions{},
out: ioutil.Discard,
},
},
{
Expand All @@ -357,10 +352,9 @@ func TestDev(t *testing.T) {
},
Deployer: &TestDeployer{},
WatcherFactory: NewWatcherFactory(fmt.Errorf("")),
opts: &config.SkaffoldOptions{
Output: &bytes.Buffer{},
},
Tagger: &tag.ChecksumTagger{},
opts: &config.SkaffoldOptions{},
Tagger: &tag.ChecksumTagger{},
out: ioutil.Discard,
},
shouldErr: true,
},
Expand All @@ -381,12 +375,11 @@ func TestBuildAndDeployAllArtifacts(t *testing.T) {
deployer := &TestDeployAll{}

runner := &SkaffoldRunner{
opts: &config.SkaffoldOptions{
Output: &bytes.Buffer{},
},
opts: &config.SkaffoldOptions{},
kubeclient: kubeclient,
Builder: builder,
Deployer: deployer,
out: ioutil.Discard,
}

ctx := context.Background()
Expand Down

0 comments on commit d590165

Please sign in to comment.