-
Notifications
You must be signed in to change notification settings - Fork 213
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
Wl/webhook test #182
Wl/webhook test #182
Conversation
Codecov Report
@@ Coverage Diff @@
## master #182 +/- ##
=======================================
Coverage 82.84% 82.84%
=======================================
Files 10 10
Lines 647 647
=======================================
Hits 536 536
Misses 87 87
Partials 24 24 Continue to review full report at Codecov.
|
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.
Looks great! We just have to add these tests to CircleCI to get them to run every time someone submits a pull request.
To do this, you'll want to edit ./.circleci/config.yml
:
- add an
install_kind
section toreferences
, with the commands necessary to install KIND - add the lines
* install_kind
andrun: ./test/webhook_test.sh
to thetest
job
I think that should do it!
…in webhook_test.sh
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.
Looks great! Just have some unused code to clean out, otherwise I think this is ready to go
test/webhook_test.sh
Outdated
echo "Webhook started!" | ||
} | ||
|
||
function check_dashboard_is_ready() { |
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 you can get rid of this now
test/webhook_test.sh
Outdated
|
||
# Install the webhook and dashboard | ||
kubectl apply -f ./deploy/webhook.yaml &> /dev/null | ||
kubectl apply -f ./deploy/dashboard.yaml &> /dev/null |
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.
You can get rid of this line
test/webhook_test.sh
Outdated
|
||
# wait for the webhook and dashboard to come online | ||
check_webhook_is_ready | ||
check_dashboard_is_ready |
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.
Ditto
@@ -130,7 +130,8 @@ jobs: | |||
steps: | |||
- checkout | |||
- *install_k8s | |||
- *test_kube_dashboard |
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.
You can get rid of test_kube_dashboard
above now
timeout_epoch=$(date -d "+2 minutes" +%s) | ||
echo "Waiting for dashboard to be ready" | ||
while ! kubectl get pods -n polaris | grep -E "dashboard.*1/1.*Running"; do | ||
echo -n "." |
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 you need a check_timeout "${timeout_epoch}"
above this
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.
One comment on the dashboard timeout, but once that's done I think we're ready to merge!
Be sure to use "Squash and merge" when you merge this - lots of commits in there :)
No description provided.