-
Notifications
You must be signed in to change notification settings - Fork 983
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
Added Guide for plans for Testing in Karpenter #1617
Conversation
✅ Deploy Preview for karpenter-docs-prod ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
||
Currently, users can only test Karpenter by adding to the list of integration tests run in a mock environment or installing and testing Karpenter on a real cluster. Users that want to test more are limited by the lack of automation and configurability, resulting in much less testing. | ||
|
||
To increase testing of Karpenter, we will introduce infrastructure and tools to extend the current mechanisms and enable developers to contribute with more confidence. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this referring to the CI infrastructure? If so, let's spell it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's implied that infrastructure will be CI within the use-cases of the testing detailed below. The design doc will also give more context on this, and what will be CI/CD.
__(To be implemented)__ When contributing, users will need to go through steps to automatically test their changes. | ||
|
||
* When a user cuts a PR and is ready to test, a maintainer will need to approve testing by posting `/ok-to-test` | ||
* A robot watching the PR will kick off tests configured in the testing folder that will run in a CI Host Cluster monitored by the Host Cluster. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is not clear what does it mean by "monitored by the Host Cluster". What is the "Host Cluster"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cleared up the language a bit. Let me know what you think
|
||
__(To be implemented)__ When contributing, users will need to go through steps to automatically test their changes. | ||
|
||
* When a user cuts a PR and is ready to test, a maintainer will need to approve testing by posting `/ok-to-test` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this optional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not be optional.
a maintainer will need
A maintainer will be able to do /ok-to-test
on their own PRs.
* A link to the logs and metrics will be posted in the PR, whether it’s prow as used in the [kubernetes/test-infra](https://github.com/kubernetes/test-infra) or an alternative solution. | ||
* After the tests are successful, the robot will report the results and a maintainer will merge it once approving the code. | ||
|
||
__(To be implemented)__ Users can follow instructions to replicate the infrastructure used to run the tests in the same folder as the tests. Contributors will be able to run tests on their own if they want to shorten the reviewer loop. The associated README will instruct users how to run tests as automated for a PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we repeat the tests ourselves even the contributors may have run tests using similar setup?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need to ensure that a PR can only be merged after an /ok-to-test
is successful. I included this as a way for developers to unblock themselves and iterate quicker if maintainers aren't quick enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
This document details plans for making testing within Karpenter more comprehensive, accessible, and configurable.
4. Does this change impact docs?
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.