volumes: document alternates to the gitRepo volume type #8429
Conversation
k8s-ci-robot
added
sig/storage
k8s-ci-robot
added
the
cncf-cla: yes
|
Deploy preview for kubernetes-io-master-staging ready! Built with commit 242144a https://deploy-preview-8429--kubernetes-io-master-staging.netlify.com |
| rather than extending the Kubernetes API for every such use case. | ||
|
|
||
| Here is an example for gitRepo volume: | ||
| While Kubernetes has a built-in `gitRepo` volume type, its functionality is |
There was a problem hiding this comment.
Maybe here, the doc should say gitRepo volume type is deprecated. And then point to a new doc, maybe under tasks?
There was a problem hiding this comment.
What about adding this example to the init containers doc? https://kubernetes.io/docs/concepts/workloads/pods/init-containers/#what-can-init-containers-be-used-for
There was a problem hiding this comment.
I think it might be better as a new task doc. I think users will want to know "how do I access git in my container", and won't think to go to the init containers page.
k8s-ci-robot
added
size/L
|
@msau42 added a task document. |
| git-clone.sh: | | ||
| #!/bin/sh -e | ||
|
|
||
| # Init Containers will re-run on Pod restart. Remove the directory's contents |
There was a problem hiding this comment.
Can you document in a comment what each argument is?
There was a problem hiding this comment.
Is storing a script a recommended use of a configmap?
There was a problem hiding this comment.
Is storing a script a recommended use of a configmap?
I couldn't find an example in https://kubernetes.io/docs/tasks/configure-pod-container/configure-pod-configmap/
However, I think it's nice to be able to inline the example like this. Realistically, a user would probably build a "git-puller" image or something like that.
|
Command arguments now documented. |
|
@ericchiang Thanks for adding this, great job on the formatting. 👍 @msau42 If this looks good to you, please |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: zacharysarah 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 |
k8s-ci-robot
added
the
approved
zacharysarah
added
Docs Review: Open Issues
and removed
approved
|
/assign @tallclair |
|
/hold |
k8s-ci-robot
added
the
do-not-merge/hold
|
@tallclair It would be great to get your review on this in light of CVE 2018-11235. |
| metadata: | ||
| name: git-clone | ||
| data: | ||
| git-clone.sh: | |
There was a problem hiding this comment.
I haven't seen the pattern of passing a script as a configmap before.... I'm not sure that's a pattern we should be encouraging? I think this can all just be inlined into the deployment.
|
|
||
| REPO=$1 | ||
| REF=$2 | ||
| DIR=$3 |
There was a problem hiding this comment.
nit: I think it's simpler to just use environment variables instead.
| rm -rf $( find $DIR -mindepth 1 ) | ||
| fi | ||
|
|
||
| git clone $REPO $DIR |
There was a problem hiding this comment.
nit: prefer git clone -- $REPO $DIR (less error-prone)
| The [emptyDir](/docs/concepts/storage/volumes/#emptydir) volume type can be used | ||
| to share data between multiple containers in a Pod. | ||
|
|
||
| First, define a script for cloning a repo to run in the Init Container: |
There was a problem hiding this comment.
Should we also mention https://github.com/kubernetes/git-sync as another approach, when continuous syncing is desired?
|
@ericchiang 👋 Please feel free to reopen this PR when you're ready to incorporate review feedback. |
Users are better served by not using the built-in gitRepo volume
type. Update the documentation to suggesting using an init
container instead.
NOTE: This doesn't need to wait for 1.11 and can be merged
to master.
Background discussion can be found in:
kubernetes/kubernetes#60999
kubernetes/kubernetes#63445
cc @kubernetes/sig-storage-pr-reviews