diff --git a/pkg/skaffold/runner/runner.go b/pkg/skaffold/runner/runner.go index 5814567a157..542dee9acf1 100644 --- a/pkg/skaffold/runner/runner.go +++ b/pkg/skaffold/runner/runner.go @@ -190,7 +190,7 @@ func (r *SkaffoldRunner) Run(ctx context.Context, out io.Writer, artifacts []*la return errors.Wrap(err, "build step") } - if err = r.Test(out, bRes); err != nil { + if err = r.Test(ctx, out, bRes); err != nil { return errors.Wrap(err, "test step") } @@ -257,7 +257,7 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*la } r.updateBuiltImages(imageList, bRes) - if err := r.Test(out, bRes); err != nil { + if err := r.Test(ctx, out, bRes); err != nil { logrus.Warnln("Skipping Deploy due to failed tests:", err) return nil } @@ -267,7 +267,7 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*la return nil } case changed.needsRedeploy: - if err := r.Test(out, r.builds); err != nil { + if err := r.Test(ctx, out, r.builds); err != nil { logrus.Warnln("Skipping Deploy due to failed tests:", err) return nil } @@ -301,7 +301,7 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*la // Watch test configuration if err := watcher.Register( - func() ([]string, error) { return r.TestDependencies(), nil }, + func() ([]string, error) { return r.TestDependencies() }, func(watch.Events) { changed.needsRedeploy = true }, ); err != nil { return nil, errors.Wrap(err, "watching test files") @@ -330,7 +330,7 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*la } r.updateBuiltImages(imageList, bRes) - if err := r.Test(out, bRes); err != nil { + if err := r.Test(ctx, out, bRes); err != nil { return nil, errors.Wrap(err, "exiting dev mode because the first test run failed") } diff --git a/pkg/skaffold/runner/runner_test.go b/pkg/skaffold/runner/runner_test.go index 2260d97e414..e0bcf6430f6 100644 --- a/pkg/skaffold/runner/runner_test.go +++ b/pkg/skaffold/runner/runner_test.go @@ -70,7 +70,7 @@ type TestTester struct { errors []error } -func (t *TestTester) Test(out io.Writer, builds []build.Artifact) error { +func (t *TestTester) Test(ctx context.Context, out io.Writer, builds []build.Artifact) error { if len(t.errors) > 0 { err := t.errors[0] t.errors = t.errors[1:] @@ -79,8 +79,8 @@ func (t *TestTester) Test(out io.Writer, builds []build.Artifact) error { return nil } -func (t *TestTester) TestDependencies() []string { - return []string{} +func (t *TestTester) TestDependencies() ([]string, error) { + return nil, nil } type TestDeployer struct { diff --git a/pkg/skaffold/runner/timings.go b/pkg/skaffold/runner/timings.go index 0bf13b0b000..a4b880dee8c 100644 --- a/pkg/skaffold/runner/timings.go +++ b/pkg/skaffold/runner/timings.go @@ -21,6 +21,8 @@ import ( "io" "time" + "k8s.io/apimachinery/pkg/labels" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/color" @@ -46,26 +48,34 @@ type withTimings struct { deploy.Deployer } +func (w withTimings) Labels() map[string]string { + return labels.Merge(w.Builder.Labels(), w.Deployer.Labels()) +} + func (w withTimings) Build(ctx context.Context, out io.Writer, tagger tag.Tagger, artifacts []*latest.Artifact) ([]build.Artifact, error) { start := time.Now() color.Default.Fprintln(out, "Starting build...") bRes, err := w.Builder.Build(ctx, out, tagger, artifacts) - if err == nil { - color.Default.Fprintln(out, "Build complete in", time.Since(start)) + if err != nil { + return nil, err } - return bRes, err + + color.Default.Fprintln(out, "Build complete in", time.Since(start)) + return bRes, nil } -func (w withTimings) Test(out io.Writer, builds []build.Artifact) error { +func (w withTimings) Test(ctx context.Context, out io.Writer, builds []build.Artifact) error { start := time.Now() color.Default.Fprintln(out, "Starting test...") - err := w.Tester.Test(out, builds) - if err == nil { - color.Default.Fprintln(out, "Test complete in", time.Since(start)) + err := w.Tester.Test(ctx, out, builds) + if err != nil { + return err } - return err + + color.Default.Fprintln(out, "Test complete in", time.Since(start)) + return nil } func (w withTimings) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact) ([]deploy.Artifact, error) { @@ -73,21 +83,12 @@ func (w withTimings) Deploy(ctx context.Context, out io.Writer, builds []build.A color.Default.Fprintln(out, "Starting deploy...") dRes, err := w.Deployer.Deploy(ctx, out, builds) - if err == nil { - color.Default.Fprintln(out, "Deploy complete in", time.Since(start)) + if err != nil { + return nil, err } - return dRes, err -} -func (w withTimings) Labels() map[string]string { - labels := map[string]string{} - for k, v := range w.Builder.Labels() { - labels[k] = v - } - for k, v := range w.Deployer.Labels() { - labels[k] = v - } - return labels + color.Default.Fprintln(out, "Deploy complete in", time.Since(start)) + return dRes, nil } func (w withTimings) Cleanup(ctx context.Context, out io.Writer) error { @@ -95,8 +96,10 @@ func (w withTimings) Cleanup(ctx context.Context, out io.Writer) error { color.Default.Fprintln(out, "Cleaning up...") err := w.Deployer.Cleanup(ctx, out) - if err == nil { - color.Default.Fprintln(out, "Cleanup complete in", time.Since(start)) + if err != nil { + return err } - return err + + color.Default.Fprintln(out, "Cleanup complete in", time.Since(start)) + return nil } diff --git a/pkg/skaffold/test/structure/structure.go b/pkg/skaffold/test/structure/structure.go index fb1e49aa401..63f226459c3 100644 --- a/pkg/skaffold/test/structure/structure.go +++ b/pkg/skaffold/test/structure/structure.go @@ -17,23 +17,30 @@ limitations under the License. package structure import ( + "context" + "io" "os/exec" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" - + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) // Test is the entrypoint for running structure tests -func (tr *Runner) Test(image string) error { - logrus.Infof("running structure tests for files %v", tr.testFiles) - args := []string{"test", "--image", image} +func (tr *Runner) Test(ctx context.Context, out io.Writer, image string) error { + logrus.Infof("Running structure tests for files %v", tr.testFiles) + + args := []string{"test", "-v", "warn", "--image", image} for _, f := range tr.testFiles { args = append(args, "--config", f) } - args = append(args, tr.testFiles...) - cmd := exec.Command("container-structure-test", args...) - _, err := util.RunCmdOut(cmd) - return err + cmd := exec.CommandContext(ctx, "container-structure-test", args...) + cmd.Stdout = out + cmd.Stderr = out + + if err := cmd.Run(); err != nil { + return errors.Wrap(err, "running container-structure-test") + } + + return nil } diff --git a/pkg/skaffold/test/structure/types.go b/pkg/skaffold/test/structure/types.go index dddf0618b5a..11727061213 100644 --- a/pkg/skaffold/test/structure/types.go +++ b/pkg/skaffold/test/structure/types.go @@ -20,8 +20,9 @@ type Runner struct { testFiles []string } -func NewStructureTestRunner(files []string) (*Runner, error) { +// NewRunner creates a new structure.Runner. +func NewRunner(files []string) *Runner { return &Runner{ testFiles: files, - }, nil + } } diff --git a/pkg/skaffold/test/test.go b/pkg/skaffold/test/test.go index e75861411e0..a46b7bf499a 100644 --- a/pkg/skaffold/test/test.go +++ b/pkg/skaffold/test/test.go @@ -17,6 +17,7 @@ limitations under the License. package test import ( + "context" "io" "os" @@ -32,73 +33,72 @@ import ( // and returns a Tester instance with all the necessary test runners // to run all specified tests. func NewTester(testCases *[]latest.TestCase) (Tester, error) { - testers := []*ArtifactTester{} - deps := []string{} // TODO(nkubala): copied this from runner.getDeployer(), this should be moved somewhere else cwd, err := os.Getwd() if err != nil { return nil, errors.Wrap(err, "finding current directory") } - for _, testCase := range *testCases { - testRunner := &ArtifactTester{ - ImageName: testCase.ImageName, - } - if testCase.StructureTests != nil { - stFiles, err := util.ExpandPathsGlob(cwd, testCase.StructureTests) - if err != nil { - return FullTester{}, errors.Wrap(err, "expanding test file paths") - } - stRunner, err := structure.NewStructureTestRunner(stFiles) - if err != nil { - return FullTester{}, errors.Wrap(err, "retrieving structure test runner") - } - testRunner.TestRunners = append(testRunner.TestRunners, stRunner) - - deps = append(deps, stFiles...) - } - testers = append(testers, testRunner) - } + return FullTester{ - ArtifactTesters: testers, - Dependencies: deps, + testCases: testCases, + workingDir: cwd, }, nil } // TestDependencies returns the watch dependencies to the runner. -func (t FullTester) TestDependencies() []string { - return t.Dependencies +func (t FullTester) TestDependencies() ([]string, error) { + var deps []string + + for _, test := range *t.testCases { + if test.StructureTests == nil { + continue + } + + files, err := util.ExpandPathsGlob(t.workingDir, test.StructureTests) + if err != nil { + return nil, errors.Wrap(err, "expanding test file paths") + } + + deps = append(deps, files...) + } + + return deps, nil } // Test is the top level testing execution call. It serves as the // entrypoint to all individual tests. -func (t FullTester) Test(out io.Writer, bRes []build.Artifact) error { - t.resolveArtifactImageTags(bRes) - for _, aTester := range t.ArtifactTesters { - if err := aTester.RunTests(); err != nil { - return err +func (t FullTester) Test(ctx context.Context, out io.Writer, bRes []build.Artifact) error { + for _, test := range *t.testCases { + if err := t.runStructureTests(ctx, out, bRes, test); err != nil { + return errors.Wrap(err, "running structure tests") } } + return nil } -// RunTests serves as the entrypoint to each group of -// artifact-specific tests. -func (a *ArtifactTester) RunTests() error { - for _, t := range a.TestRunners { - if err := t.Test(a.ImageName); err != nil { - return err - } +func (t FullTester) runStructureTests(ctx context.Context, out io.Writer, bRes []build.Artifact, testCase latest.TestCase) error { + if len(testCase.StructureTests) == 0 { + return nil } - return nil + + files, err := util.ExpandPathsGlob(t.workingDir, testCase.StructureTests) + if err != nil { + return errors.Wrap(err, "expanding test file paths") + } + + runner := structure.NewRunner(files) + fqn := resolveArtifactImageTag(testCase.ImageName, bRes) + + return runner.Test(ctx, out, fqn) } -// replace original test artifact images with tagged build artifact images -func (t *FullTester) resolveArtifactImageTags(bRes []build.Artifact) { - for _, aTest := range t.ArtifactTesters { - for _, res := range bRes { - if aTest.ImageName == res.ImageName { - aTest.ImageName = res.Tag - } +func resolveArtifactImageTag(imageName string, bRes []build.Artifact) string { + for _, res := range bRes { + if imageName == res.ImageName { + return res.Tag } } + + return imageName } diff --git a/pkg/skaffold/test/types.go b/pkg/skaffold/test/types.go index 524cdf0d1e2..e23d46a6324 100644 --- a/pkg/skaffold/test/types.go +++ b/pkg/skaffold/test/types.go @@ -17,9 +17,11 @@ limitations under the License. package test import ( + "context" "io" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" ) // Tester is the top level test executor in Skaffold. @@ -27,9 +29,9 @@ import ( // each of which contains one or more TestRunners which implements // a single test run. type Tester interface { - Test(io.Writer, []build.Artifact) error + Test(context.Context, io.Writer, []build.Artifact) error - TestDependencies() []string + TestDependencies() ([]string, error) } // FullTester serves as a holder for the individual artifact-specific @@ -38,22 +40,15 @@ type Tester interface { // the FullTester actually handles the work. // FullTester should always be the ONLY implementation of the Tester interface; -// newly added testing implementations should implement the TestRunner interface. +// newly added testing implementations should implement the Runner interface. type FullTester struct { - ArtifactTesters []*ArtifactTester - Dependencies []string + testCases *[]latest.TestCase + workingDir string } -// ArtifactTester is an artifact-specific test holder, which contains -// tests runners to run all specified tests on an individual artifact. -type ArtifactTester struct { - ImageName string - TestRunners []Runner -} - -// TestRunner is the lowest-level test executor in Skaffold, responsible for +// Runner is the lowest-level test executor in Skaffold, responsible for // running a single test on a single artifact image and returning its result. // Any new test type should implement this interface. type Runner interface { - Test(image string) error + Test(ctx context.Context, out io.Writer, image string) error }