From 182719506b893e09b2f1ef5ec19e8b9fd4348ef0 Mon Sep 17 00:00:00 2001 From: Nick Kubala Date: Mon, 20 May 2019 12:01:35 -0700 Subject: [PATCH] Refactor the runner a bit --- pkg/skaffold/runner/build.go | 57 +++++++++++++++ pkg/skaffold/runner/deploy.go | 47 +++++++++++++ pkg/skaffold/runner/logger.go | 46 ++++++++++++ pkg/skaffold/runner/run.go | 36 ++++++++++ pkg/skaffold/runner/run_test.go | 85 ++++++++++++++++++++++ pkg/skaffold/runner/runner.go | 109 ++++------------------------- pkg/skaffold/runner/runner_test.go | 59 ---------------- 7 files changed, 283 insertions(+), 156 deletions(-) create mode 100644 pkg/skaffold/runner/build.go create mode 100644 pkg/skaffold/runner/deploy.go create mode 100644 pkg/skaffold/runner/logger.go create mode 100644 pkg/skaffold/runner/run.go create mode 100644 pkg/skaffold/runner/run_test.go diff --git a/pkg/skaffold/runner/build.go b/pkg/skaffold/runner/build.go new file mode 100644 index 00000000000..50a877cd2ab --- /dev/null +++ b/pkg/skaffold/runner/build.go @@ -0,0 +1,57 @@ +/* +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 runner + +import ( + "context" + "io" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" +) + +// BuildAndTest builds artifacts and runs tests on built artifacts +func (r *SkaffoldRunner) BuildAndTest(ctx context.Context, out io.Writer, artifacts []*latest.Artifact) ([]build.Artifact, error) { + tags, err := r.imageTags(ctx, out, artifacts) + if err != nil { + return nil, errors.Wrap(err, "generating tag") + } + r.hasBuilt = true + + artifactsToBuild, res, err := r.cache.RetrieveCachedArtifacts(ctx, out, artifacts) + if err != nil { + return nil, errors.Wrap(err, "retrieving cached artifacts") + } + + bRes, err := r.Build(ctx, out, tags, artifactsToBuild) + if err != nil { + return nil, errors.Wrap(err, "build failed") + } + r.cache.RetagLocalImages(ctx, out, artifactsToBuild, bRes) + bRes = append(bRes, res...) + if err := r.cache.CacheArtifacts(ctx, artifacts, bRes); err != nil { + logrus.Warnf("error caching artifacts: %v", err) + } + if !r.runCtx.Opts.SkipTests { + if err = r.Test(ctx, out, bRes); err != nil { + return nil, errors.Wrap(err, "test failed") + } + } + return bRes, err +} diff --git a/pkg/skaffold/runner/deploy.go b/pkg/skaffold/runner/deploy.go new file mode 100644 index 00000000000..00ac452f4ef --- /dev/null +++ b/pkg/skaffold/runner/deploy.go @@ -0,0 +1,47 @@ +/* +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 runner + +import ( + "context" + "io" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" +) + +// Deploy deploys build artifacts. +func (r *SkaffoldRunner) Deploy(ctx context.Context, out io.Writer, artifacts []build.Artifact) error { + if err := r.deploy(ctx, out, artifacts); err != nil { + return err + } + if r.runCtx.Opts.Tail { + images := make([]string, len(artifacts)) + for i, a := range artifacts { + images[i] = a.ImageName + } + logger := r.newLoggerForImages(out, images) + return r.TailLogs(ctx, out, logger) + } + return nil +} + +// Deploy deploys the given artifacts and tail logs if tail present +func (r *SkaffoldRunner) deploy(ctx context.Context, out io.Writer, artifacts []build.Artifact) error { + err := r.Deployer.Deploy(ctx, out, artifacts, r.labellers) + r.hasDeployed = true + return err +} diff --git a/pkg/skaffold/runner/logger.go b/pkg/skaffold/runner/logger.go new file mode 100644 index 00000000000..a9312dc378a --- /dev/null +++ b/pkg/skaffold/runner/logger.go @@ -0,0 +1,46 @@ +/* +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 runner + +import ( + "context" + "io" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" + "github.com/pkg/errors" +) + +func (r *SkaffoldRunner) newLogger(out io.Writer, artifacts []*latest.Artifact) *kubernetes.LogAggregator { + var imageNames []string + for _, artifact := range artifacts { + imageNames = append(imageNames, artifact.ImageName) + } + return r.newLoggerForImages(out, imageNames) +} + +func (r *SkaffoldRunner) newLoggerForImages(out io.Writer, images []string) *kubernetes.LogAggregator { + return kubernetes.NewLogAggregator(out, images, r.imageList, r.runCtx.Namespaces) +} + +func (r *SkaffoldRunner) TailLogs(ctx context.Context, out io.Writer, logger *kubernetes.LogAggregator) error { + if err := logger.Start(ctx); err != nil { + return errors.Wrap(err, "starting logger") + } + <-ctx.Done() + return nil +} diff --git a/pkg/skaffold/runner/run.go b/pkg/skaffold/runner/run.go new file mode 100644 index 00000000000..56e21b6ac2b --- /dev/null +++ b/pkg/skaffold/runner/run.go @@ -0,0 +1,36 @@ +/* +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 runner + +import ( + "context" + "io" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" +) + +// Run builds artifacts, runs tests on built artifacts, and then deploys them. +func (r *SkaffoldRunner) Run(ctx context.Context, out io.Writer, artifacts []*latest.Artifact) error { + if err := r.buildTestDeploy(ctx, out, artifacts); err != nil { + return err + } + if r.runCtx.Opts.Tail { + logger := r.newLogger(out, artifacts) + return r.TailLogs(ctx, out, logger) + } + return nil +} diff --git a/pkg/skaffold/runner/run_test.go b/pkg/skaffold/runner/run_test.go new file mode 100644 index 00000000000..a50ecf07bce --- /dev/null +++ b/pkg/skaffold/runner/run_test.go @@ -0,0 +1,85 @@ +/* +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 runner + +import ( + "context" + "io/ioutil" + "testing" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" + "github.com/GoogleContainerTools/skaffold/testutil" + "github.com/pkg/errors" + "k8s.io/client-go/tools/clientcmd/api" +) + +func TestRun(t *testing.T) { + restore := testutil.SetupFakeKubernetesContext(t, api.Config{CurrentContext: "cluster1"}) + defer restore() + + var tests = []struct { + description string + testBench *TestBench + shouldErr bool + expectedActions []Actions + }{ + { + description: "run no error", + testBench: &TestBench{}, + expectedActions: []Actions{{ + Built: []string{"img:1"}, + Tested: []string{"img:1"}, + Deployed: []string{"img:1"}, + }}, + }, + { + description: "run build error", + testBench: &TestBench{buildErrors: []error{errors.New("")}}, + shouldErr: true, + expectedActions: []Actions{{}}, + }, + { + description: "run test error", + testBench: &TestBench{testErrors: []error{errors.New("")}}, + shouldErr: true, + expectedActions: []Actions{{ + Built: []string{"img:1"}, + }}, + }, + { + description: "run deploy error", + testBench: &TestBench{deployErrors: []error{errors.New("")}}, + shouldErr: true, + expectedActions: []Actions{{ + Built: []string{"img:1"}, + Tested: []string{"img:1"}, + }}, + }, + } + + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + runner := createRunner(t, test.testBench) + + err := runner.Run(context.Background(), ioutil.Discard, []*latest.Artifact{{ + ImageName: "img", + }}) + + testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expectedActions, test.testBench.Actions()) + }) + } +} diff --git a/pkg/skaffold/runner/runner.go b/pkg/skaffold/runner/runner.go index c96f7bb4226..d3e9ec6a6c0 100644 --- a/pkg/skaffold/runner/runner.go +++ b/pkg/skaffold/runner/runner.go @@ -189,18 +189,6 @@ func getTagger(t latest.TagPolicy, customTag string) (tag.Tagger, error) { } } -func (r *SkaffoldRunner) newLogger(out io.Writer, artifacts []*latest.Artifact) *kubernetes.LogAggregator { - var imageNames []string - for _, artifact := range artifacts { - imageNames = append(imageNames, artifact.ImageName) - } - return r.newLoggerForImages(out, imageNames) -} - -func (r *SkaffoldRunner) newLoggerForImages(out io.Writer, images []string) *kubernetes.LogAggregator { - return kubernetes.NewLogAggregator(out, images, r.imageList, r.runCtx.Namespaces) -} - // HasDeployed returns true if this runner has deployed something. func (r *SkaffoldRunner) HasDeployed() bool { return r.hasDeployed @@ -211,63 +199,6 @@ func (r *SkaffoldRunner) HasBuilt() bool { return r.hasBuilt } -func (r *SkaffoldRunner) buildTestDeploy(ctx context.Context, out io.Writer, artifacts []*latest.Artifact) error { - bRes, err := r.BuildAndTest(ctx, out, artifacts) - if err != nil { - return err - } - - // Update which images are logged. - for _, build := range bRes { - r.imageList.Add(build.Tag) - } - - // Make sure all artifacts are redeployed. Not only those that were just built. - r.builds = build.MergeWithPreviousBuilds(bRes, r.builds) - - if err := r.deploy(ctx, out, r.builds); err != nil { - return errors.Wrap(err, "deploy failed") - } - - return nil -} - -// Run builds artifacts, runs tests on built artifacts, and then deploys them. -func (r *SkaffoldRunner) Run(ctx context.Context, out io.Writer, artifacts []*latest.Artifact) error { - if err := r.buildTestDeploy(ctx, out, artifacts); err != nil { - return err - } - if r.runCtx.Opts.Tail { - logger := r.newLogger(out, artifacts) - return r.TailLogs(ctx, out, logger) - } - return nil -} - -// Deploy deploys build artifacts. -func (r *SkaffoldRunner) Deploy(ctx context.Context, out io.Writer, artifacts []build.Artifact) error { - if err := r.deploy(ctx, out, artifacts); err != nil { - return err - } - if r.runCtx.Opts.Tail { - images := make([]string, len(artifacts)) - for i, a := range artifacts { - images[i] = a.ImageName - } - logger := r.newLoggerForImages(out, images) - return r.TailLogs(ctx, out, logger) - } - return nil -} - -func (r *SkaffoldRunner) TailLogs(ctx context.Context, out io.Writer, logger *kubernetes.LogAggregator) error { - if err := logger.Start(ctx); err != nil { - return errors.Wrap(err, "starting logger") - } - <-ctx.Done() - return nil -} - type tagErr struct { tag string err error @@ -317,39 +248,23 @@ func (r *SkaffoldRunner) imageTags(ctx context.Context, out io.Writer, artifacts return imageTags, nil } -// BuildAndTest builds artifacts and runs tests on built artifacts -func (r *SkaffoldRunner) BuildAndTest(ctx context.Context, out io.Writer, artifacts []*latest.Artifact) ([]build.Artifact, error) { - tags, err := r.imageTags(ctx, out, artifacts) +func (r *SkaffoldRunner) buildTestDeploy(ctx context.Context, out io.Writer, artifacts []*latest.Artifact) error { + bRes, err := r.BuildAndTest(ctx, out, artifacts) if err != nil { - return nil, errors.Wrap(err, "generating tag") + return err } - r.hasBuilt = true - artifactsToBuild, res, err := r.cache.RetrieveCachedArtifacts(ctx, out, artifacts) - if err != nil { - return nil, errors.Wrap(err, "retrieving cached artifacts") + // Update which images are logged. + for _, build := range bRes { + r.imageList.Add(build.Tag) } - bRes, err := r.Build(ctx, out, tags, artifactsToBuild) - if err != nil { - return nil, errors.Wrap(err, "build failed") - } - r.cache.RetagLocalImages(ctx, out, artifactsToBuild, bRes) - bRes = append(bRes, res...) - if err := r.cache.CacheArtifacts(ctx, artifacts, bRes); err != nil { - logrus.Warnf("error caching artifacts: %v", err) - } - if !r.runCtx.Opts.SkipTests { - if err = r.Test(ctx, out, bRes); err != nil { - return nil, errors.Wrap(err, "test failed") - } + // Make sure all artifacts are redeployed. Not only those that were just built. + r.builds = build.MergeWithPreviousBuilds(bRes, r.builds) + + if err := r.deploy(ctx, out, r.builds); err != nil { + return errors.Wrap(err, "deploy failed") } - return bRes, err -} -// Deploy deploys the given artifacts and tail logs if tail present -func (r *SkaffoldRunner) deploy(ctx context.Context, out io.Writer, artifacts []build.Artifact) error { - err := r.Deployer.Deploy(ctx, out, artifacts, r.labellers) - r.hasDeployed = true - return err + return nil } diff --git a/pkg/skaffold/runner/runner_test.go b/pkg/skaffold/runner/runner_test.go index 1a123ff540f..ae2474a2aed 100644 --- a/pkg/skaffold/runner/runner_test.go +++ b/pkg/skaffold/runner/runner_test.go @@ -18,10 +18,8 @@ package runner import ( "context" - "errors" "fmt" "io" - "io/ioutil" "testing" "k8s.io/client-go/tools/clientcmd/api" @@ -301,60 +299,3 @@ func TestNewForConfig(t *testing.T) { }) } } - -func TestRun(t *testing.T) { - restore := testutil.SetupFakeKubernetesContext(t, api.Config{CurrentContext: "cluster1"}) - defer restore() - - var tests = []struct { - description string - testBench *TestBench - shouldErr bool - expectedActions []Actions - }{ - { - description: "run no error", - testBench: &TestBench{}, - expectedActions: []Actions{{ - Built: []string{"img:1"}, - Tested: []string{"img:1"}, - Deployed: []string{"img:1"}, - }}, - }, - { - description: "run build error", - testBench: &TestBench{buildErrors: []error{errors.New("")}}, - shouldErr: true, - expectedActions: []Actions{{}}, - }, - { - description: "run test error", - testBench: &TestBench{testErrors: []error{errors.New("")}}, - shouldErr: true, - expectedActions: []Actions{{ - Built: []string{"img:1"}, - }}, - }, - { - description: "run deploy error", - testBench: &TestBench{deployErrors: []error{errors.New("")}}, - shouldErr: true, - expectedActions: []Actions{{ - Built: []string{"img:1"}, - Tested: []string{"img:1"}, - }}, - }, - } - - for _, test := range tests { - t.Run(test.description, func(t *testing.T) { - runner := createRunner(t, test.testBench) - - err := runner.Run(context.Background(), ioutil.Discard, []*latest.Artifact{{ - ImageName: "img", - }}) - - testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expectedActions, test.testBench.Actions()) - }) - } -}