Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement custom tester functionality in Skaffold #5451

Merged
merged 41 commits into from Mar 4, 2021

Conversation

PriyaModali
Copy link
Contributor

@PriyaModali PriyaModali commented Feb 25, 2021

Fixes: ##5395

Description
Add new custom tester functionality to Skaffold. This feature enable users to run custom tests in the Skaffold pipeline.

The test command:

  1. Implements Runner interface
  2. Enables ability to run custom commands between build & deploy phases

Run skaffold dev to run the command in the Skaffold pipeline.

Added unit tests and integrations tests for the newly added test command.

Related:
#5333

Note:
Right now, changing a test dependency file does not trigger a build but triggers only deployment.

Follow up work:
Changing a test dependency file does not trigger a build but will trigger test.

Example

The functionality can be tested using the example in the integration tests:
skaffold/integration/examples/custom-tests

Testing notes

Here are some sample outputs:

skaffold dev - first loop that does the output in test phase

Testing images...
Running custom test command: "./test.sh" with timeout 60 s
go custom test 
ok      github.com/GoogleContainerTools/skaffold/integration/examples/custom-tests      0.812s
Command finished successfully
Running custom test command: "echo Hello world!!"
Hello world!!
Command finished successfully
Tags used in deployment:
 - skaffold-example -> skaffold-example:0a3c7c9f048a204d72b2e4dbc017351754ad8d283505d9cd43e54023eb073ee5
Starting deploy...
 - pod/getting-started created
Waiting for deployments to stabilize...
Deployments stabilized in 42.507007ms
Press Ctrl+C to exit
Watching for changes...
[getting-started] ...
....

skaffold dev - changing a test file does not trigger a build

Testing images...
Running custom test command: "./test.sh" with timeout 60 s
go custom test 
ok      github.com/GoogleContainerTools/skaffold/integration/examples/custom-tests      0.812s
Command finished successfully
Running custom test command: "echo Hello world!!"
Hello world!!
Command finished successfully
Tags used in deployment:
 - skaffold-example -> skaffold-example:5b2a46985b6af5b3a545443d67ae6198c100080d897ffd900db702eca7eea9ba
Starting deploy...
 - pod/getting-started created
Waiting for deployments to stabilize...
Deployments stabilized in 43.06159ms
Press Ctrl+C to exit
Watching for changes...
[getting-started] ...
....
....
Tags used in deployment:
 - skaffold-example -> skaffold-example:5b2a46985b6af5b3a545443d67ae6198c100080d897ffd900db702eca7eea9ba
Starting deploy...
Waiting for deployments to stabilize...
Deployments stabilized in 54.700905ms
Watching for changes...
[getting-started] ...
....
....

skaffold dev - changing build dependency triggers build and test

Testing images...
Running custom test command: "./test.sh" with timeout 60 s
go custom test 
ok      github.com/GoogleContainerTools/skaffold/integration/examples/custom-tests      0.812s
Command finished successfully
Running custom test command: "echo Hello world!!"
Hello world!!
Command finished successfully
Tags used in deployment:
 - skaffold-example -> skaffold-example:bfe229ee3daf1af14d4db031daf84ebdcb9ddd97a0530fa8fa8d52e66d4c0780
Starting deploy...
 - pod/getting-started created
Waiting for deployments to stabilize...
Deployments stabilized in 49.206668ms
Press Ctrl+C to exit
Watching for changes...
[getting-started] ...
....
....
Generating tags...
 - skaffold-example -> skaffold-example:v1.19.0-85-g55659bd41-dirty
Found [minikube] context, using local docker daemon.
Building [skaffold-example]...
Sending build context to Docker daemon  3.584kB
....
....
Successfully built d21f428215c1
Successfully tagged skaffold-example:v1.19.0-85-g55659bd41-dirty
Testing images...
Running custom test command: "./test.sh" with timeout 60 s
go custom test 
ok      github.com/GoogleContainerTools/skaffold/integration/examples/custom-tests      0.812s
Command finished successfully
Running custom test command: "echo Hello world!!"
Hello world!!
Command finished successfully
Tags used in deployment:
 - skaffold-example -> skaffold-example:d21f428215c1ac7896825508279f6dd06830c93d5a8f189baf8627c58e5d353f
Starting deploy...
 - pod/getting-started configured
Waiting for deployments to stabilize...
Deployments stabilized in 24.410772ms
Watching for changes...
[getting-started] ...
....
....

skaffold dev - Test failure with a timeout

Testing images...
Running custom test command: "./test.sh" with timeout 60 s
go custom test 
ok      github.com/GoogleContainerTools/skaffold/integration/examples/custom-tests      0.812s
Command timed out
Pruning images...
exiting dev mode because test failed after first build: running tests: running custom test command: context deadline exceeded

skaffold dev - Test failure with a non-0 exit code

Testing images...
Running custom test command: "./test.sh" with timeout 60 s
go custom test 
ok      github.com/GoogleContainerTools/skaffold/integration/examples/custom-tests      0.812s
Pruning images...
exiting dev mode because test failed after first build: running tests: running custom test command: Command finished with non-0 exit code: exit status 20

@PriyaModali PriyaModali added kind/feature-request area/testing Issues concerning the testing phase of Skaffold priority/p1 High impact feature/bug. labels Feb 25, 2021
@google-cla google-cla bot added the cla: yes label Feb 25, 2021
@codecov
Copy link

codecov bot commented Feb 25, 2021

Codecov Report

Merging #5451 (27238e5) into master (14d4128) will decrease coverage by 0.08%.
The diff coverage is 61.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5451      +/-   ##
==========================================
- Coverage   71.49%   71.41%   -0.09%     
==========================================
  Files         397      398       +1     
  Lines       14577    14653      +76     
==========================================
+ Hits        10422    10464      +42     
- Misses       3387     3410      +23     
- Partials      768      779      +11     
Impacted Files Coverage Δ
pkg/skaffold/schema/latest/config.go 58.82% <ø> (ø)
pkg/skaffold/test/test_factory.go 72.09% <33.33%> (-6.86%) ⬇️
pkg/skaffold/test/custom/custom.go 58.33% <58.33%> (ø)
pkg/skaffold/schema/validation/validation.go 90.41% <100.00%> (+0.46%) ⬆️
pkg/skaffold/util/tar.go 50.66% <0.00%> (-5.34%) ⬇️
pkg/skaffold/docker/image.go 78.13% <0.00%> (-1.40%) ⬇️
pkg/skaffold/docker/parse.go 83.24% <0.00%> (+1.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14d4128...27238e5. Read the comment docs.

@PriyaModali PriyaModali marked this pull request as ready for review February 25, 2021 17:05
@PriyaModali PriyaModali requested a review from a team as a code owner February 25, 2021 17:05
@PriyaModali
Copy link
Contributor Author

@briandealwis & @tejal29, thank you for the reviews. I have addressed all the feedback. Welcome more. Otherwise, I'd like to get this into a good state for approval so that I can move on to #5431.

@tejal29
Copy link
Member

tejal29 commented Mar 3, 2021

@briandealwis & @tejal29, thank you for the reviews. I have addressed all the feedback. Welcome more. Otherwise, I'd like to get this into a good state for approval so that I can move on to #5431.

Thanks @PriyaModali. Can you mention little bit in the description what are the output changes. It will be great to include example running skaffold dev on the custom-tests example you have.

I would probably do the following testing and add Testing notes e.g. 0, 1

  1. skaffold dev, - first loop what does the output in test phase look like
  2. skaffold dev - changing a test file does not trigger a build but only test
  3. skaffold dev - changing build dependency triggers build and test.

@PriyaModali
Copy link
Contributor Author

PriyaModali commented Mar 3, 2021

@briandealwis & @tejal29, thank you for the reviews. I have addressed all the feedback. Welcome more. Otherwise, I'd like to get this into a good state for approval so that I can move on to #5431.

Thanks @PriyaModali. Can you mention little bit in the description what are the output changes. It will be great to include example running skaffold dev on the custom-tests example you have.

I would probably do the following testing and add Testing notes e.g. 0, 1

  1. skaffold dev, - first loop what does the output in test phase look like
  2. skaffold dev - changing a test file does not trigger a build but only test
  3. skaffold dev - changing build dependency triggers build and test.

Added Testing notes and future work to the description.

pkg/skaffold/test/custom/custom_test.go Outdated Show resolved Hide resolved
pkg/skaffold/test/custom/custom_test.go Outdated Show resolved Hide resolved
pkg/skaffold/test/custom/custom_test.go Outdated Show resolved Hide resolved
pkg/skaffold/test/custom/custom.go Show resolved Hide resolved
pkg/skaffold/schema/validation/validation_test.go Outdated Show resolved Hide resolved
pkg/skaffold/test/custom/custom.go Outdated Show resolved Hide resolved
pkg/skaffold/test/custom/custom_test.go Outdated Show resolved Hide resolved
pkg/skaffold/test/custom/custom_test.go Show resolved Hide resolved
pkg/skaffold/test/custom/custom_test.go Outdated Show resolved Hide resolved
pkg/skaffold/test/custom/custom_test.go Show resolved Hide resolved
Copy link
Member

@tejal29 tejal29 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something went wrong with github and not all comment went through last time.

@PriyaModali PriyaModali added kokoro:run runs the kokoro jobs on a PR kokoro:force-run forces a kokoro re-run on a PR labels Mar 3, 2021
@briandealwis briandealwis added kokoro:force-run forces a kokoro re-run on a PR and removed kokoro:run runs the kokoro jobs on a PR kokoro:force-run forces a kokoro re-run on a PR labels Mar 3, 2021
@kokoro-team kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Mar 3, 2021
Copy link
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is almost done. The integration test needs another instance to verify the test-dependencies-from-a-command, particularly for Windows.

pkg/skaffold/test/custom/custom.go Show resolved Hide resolved
integration/examples/custom-tests/skaffold.yaml Outdated Show resolved Hide resolved
Copy link
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small changes and good to go.

Note that the ./test.sh will fail on Windows, but we're not running our integration tests on Windows. I suspect we'll have many other fixes requires for integration tests on Windows.

@PriyaModali PriyaModali added the kokoro:force-run forces a kokoro re-run on a PR label Mar 4, 2021
@kokoro-team kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Mar 4, 2021
@PriyaModali PriyaModali merged commit 2e55bec into GoogleContainerTools:master Mar 4, 2021
@nkubala nkubala mentioned this pull request Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Issues concerning the testing phase of Skaffold cla: yes kind/feature-request priority/p1 High impact feature/bug. size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants