-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Update integration tests to use a locally built version of kic #7857
Conversation
/ok-to-test |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: priyawadhwa The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov Report
@@ Coverage Diff @@
## master #7857 +/- ##
=======================================
Coverage 35.61% 35.61%
=======================================
Files 148 148
Lines 9303 9303
=======================================
Hits 3313 3313
Misses 5593 5593
Partials 397 397 |
kvm2 Driver |
hack/jenkins/common.sh
Outdated
touch "${TEST_OUT}" | ||
${SUDO_PREFIX}${E2E_BIN} \ | ||
-minikube-start-args="--driver=${VM_DRIVER} ${EXTRA_START_ARGS}" \ | ||
-minikube-start-args="--driver=${VM_DRIVER} --docker-base-image=kic ${EXTRA_START_ARGS}" \ |
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.
should be added to our github actions too
I recommend adding a new job in the 3 jobs groups that are running before integraiton tests
currently they are build_minikube, unit_test, lint
we should add another job called -build build-base image-
and make the follwing integraiton jobs adds
needs: [build-minikube , build-base-image-
kvm2 Driver Times for Minikube (PR 7857): [65.008399247 64.968777433 63.591683065] Averages Time Per Log
docker Driver Times for Minikube (PR 7857): [41.36577660999999 26.523642036000002 27.703785408000005] Averages Time Per Log
|
kvm2 Driver Times for Minikube (PR 7857): [68.717599767 65.56597774 66.11161530800001] Averages Time Per Log
docker Driver Times for Minikube (PR 7857): [26.10366515 26.208711791 26.890062625] Averages Time Per Log
|
kvm2 Driver Times for Minikube (PR 7857): [79.271983969 64.97733529700001 66.071959173] Averages Time Per Log
docker Driver Times for Minikube (PR 7857): [27.196796482 27.951698336 28.763457967999997] Averages Time Per Log
|
@@ -20,6 +20,8 @@ jobs: | |||
run: | | |||
make minikube-linux-amd64 | |||
make e2e-linux-amd64 | |||
make kic-base-image |
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 should only do this only if the Dockerfile been changed. if not it can cause all sort of issues. currently our base ubuntu image is not pinned and each time we get a different installed versions. not good idea for a reliable idea of testing what we ship to the users.
@priyawadhwa: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
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.
@priyawadhwa
this PR still needs to address the comment that it should build the image only if the Dockerfile was changed compared to the master, we dont want to rebuild same Dockerfile on each test (that will result in slightly different packages)
closing until i get a chance to look at this again |
Depends on #7856 and #7858 to merge first