Option to mount HostPath in each Kaniko Pod to be used as cache volume#1690
Merged
nkubala merged 6 commits intoGoogleContainerTools:masterfrom May 20, 2019
Merged
Conversation
martinhoefling
requested review from
balopat,
dgageot,
nkubala,
priyawadhwa and
r2d4
as code owners
martinhoefling
force-pushed
the
kaniko-host-path-cache
branch
from
4c933f4 to
56216fd
Compare
Codecov Report
@@ Coverage Diff @@
## master #1690 +/- ##
==========================================
- Coverage 56.74% 56.63% -0.11%
==========================================
Files 183 183
Lines 7860 7875 +15
==========================================
Hits 4460 4460
- Misses 2988 3001 +13
- Partials 412 414 +2
Continue to review full report at Codecov.
|
nkubala
suggested changes
Mar 26, 2019
Contributor
nkubala
left a comment
nkubala
left a comment
There was a problem hiding this comment.
hey @martinhoefling, thanks for the PR and sorry for the late review! looks like you need to rebase here unfortunately. you'll need to move your config changes to the latest version. also, make sure that --cache=true is being added to the kaniko args before you add the --cache-dir flag. other than that this looks good!
martinhoefling
force-pushed
the
kaniko-host-path-cache
branch
2 times, most recently
from
e271431 to
49f4d05
Compare
Contributor
Author
|
@nkubala rebased. Can you check it again? |
Contributor
|
@martinhoefling sorry for the delay. we bumped the config version since the last review so you'll need to move your changes one more time to the latest version 😨 it's annoying I know, and I'll take responsibility for that. I'll merge this once you do! |
Contributor
|
@martinhoefling If you move the changes to the latest version, make sure |
Contributor
Author
|
@corneliusweig ok, thx. How do I detect this? #2029 mentions v1beta9. Is there any other PR I should rebase on now? |
Contributor
|
So there is #2035, but honestly I'd wait until it is merged. |
martinhoefling
force-pushed
the
kaniko-host-path-cache
branch
from
59a735c to
b9ad1a1
Compare
Contributor
Author
|
@nkubala, rebased tests fixed and config migrated to v1beta10. |
Contributor
|
hey @martinhoefling, thanks for addressing the config stuff. can you check out the failing integration test? |
Contributor
|
This PR needs a |
martinhoefling
force-pushed
the
kaniko-host-path-cache
branch
from
b9ad1a1 to
82e463e
Compare
Contributor
|
@martinhoefling integration test is still failing. I don't want you to have to rebase again when we do the next config change 😬 PTAL! |
Contributor
Author
|
@nkubala, I am struggling setting up the integration testcases locally. The logs aren’t very verbose. I have to check when I find some time for this. Maybe next week at Kubecon ;-) |
Contributor
|
no worries! since it's just the kaniko test failing, you can skip trying to run all the other ones locally (and skip installing all the necessary dependencies for them). if you have kaniko configured on a cluster, you can just set up your secret in the make sure to swing by our both at kubecon and say hi! maybe someone can give you some pointers for the integration tests 😆 |
Contributor
Author
|
So when I run it locally (minikube), everything works fine except the push step. Kaniko log output (via stern) Skaffold output Skaffold version:
Btw, skaffold 0.29 produces the same output. For the push step, I need to set up a registry to push first. |
martinhoefling
force-pushed
the
kaniko-host-path-cache
branch
from
82e463e to
333737e
Compare
Contributor
Author
|
@nkubala I pushed my latest rebase. I'll try catch up at kubecon. |
Contributor
Author
|
@nkubala, got it - the latest commit should fix the issue when no cache config is given at all. That was only the case in the 'kaniko-sub-folder' testdata. I looked at the local example before, where cache is set to {}. Can you rerun the integration tests and see if this solves the issue? |
| Repo string `yaml:"repo,omitempty"` | ||
| // HostPath specifies a path on the host that is mounted to each pod as read only cache volume containing base images. | ||
| // If set, must exist on each node and prepopulated with kaniko-warmer. | ||
| HostPath string `yaml:"hostPath,omitempty"` |
Contributor
There was a problem hiding this comment.
This change should only be in the latest config version, right?
corneliusweig
suggested changes
May 20, 2019
Contributor
There was a problem hiding this comment.
Hi @martinhoefling, you need to merge in master once more, because skaffold config version v1beta10 has been frozen (See #2109). Your changeset should only touch docs/content/en/schemas/v1beta11.json. Currently, it also affects v1beta6 which must be an error.
Hopefully, this will be the last time. Enjoy kubecon!
PS: Can you also list your addition in the comments to Upgrade() in pkg/skaffold/schema/v1beta10/upgrade.go? This may lead to further merge conflicts with other PRs though..
martinhoefling
force-pushed
the
kaniko-host-path-cache
branch
from
7410a1a to
6720ca6
Compare
nkubala
approved these changes
May 20, 2019
Contributor
|
@martinhoefling thanks for seeing this one through! enjoy Kubecon, and be sure to say hi to our team at the booth :) |
Contributor
Author
|
@codecov-io @nkubala thx for your help! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The introduced
HostPathoption mounts a host path of a kubernetes node into the kaniko executor pod. This path can be prepopulated withkaniko-warmerand is used by setting--cache=trueand--cache-diron execution of kaniko.