Option to mount HostPath in each Kaniko Pod to be used as cache volume #1690
Conversation
This file contains 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
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
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
|
@nkubala rebased. Can you check it again? |
|
@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! |
|
@martinhoefling If you move the changes to the latest version, make sure |
|
@corneliusweig ok, thx. How do I detect this? #2029 mentions v1beta9. Is there any other PR I should rebase on now? |
|
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
|
@nkubala, rebased tests fixed and config migrated to v1beta10. |
|
hey @martinhoefling, thanks for addressing the config stuff. can you check out the failing integration test? |
|
This PR needs a |
martinhoefling
force-pushed
the
kaniko-host-path-cache
branch
from
b9ad1a1 to
82e463e
Compare
|
@martinhoefling integration test is still failing. I don't want you to have to rebase again when we do the next config change 😬 PTAL! |
|
@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 ;-) |
|
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 😆 |
|
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
|
@nkubala I pushed my latest rebase. I'll try catch up at kubecon. |
|
@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? |
| @@ -229,6 +229,9 @@ type KanikoCache struct { | |||
| // Repo is a remote repository to store cached layers. If none is specified, one will be | |||
| // inferred from the image name. See [Kaniko Caching](https://github.com/GoogleContainerTools/kaniko#caching). | |||
| 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"` | |||
There was a problem hiding this comment.
This change should only be in the latest config version, right?
corneliusweig
suggested changes
May 20, 2019
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
|
@martinhoefling thanks for seeing this one through! enjoy Kubecon, and be sure to say hi to our team at the booth :) |
|
@codecov-io @nkubala thx for your help! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.