example: pull from upstream repos directly, use fixed revisions #72
Conversation
k8s-ci-robot
added
size/L
|
/cc @msau42 kubernetes-csi/external-snapshotter#53 must be merged first, then the Example.md needs to be updated an the HTML files re-generated. But some early feedback would be welcome before I invest more time into this, and I wanted to have this PR open as explanation why that other PR is useful. |
k8s-ci-robot
added
the
do-not-merge/hold
|
FWIW, I tested this on a 1.13 pre-release of Kubernetes (current master, basically). Would it make sense to mention at the top of the Example.md that the instructions work for both 1.12 and 1.13? |
pohly
force-pushed
the
upstream-yaml
branch
2 times, most recently
from
b59af7d to
69d1220
Compare
k8s-ci-robot
added
size/XXL
|
/hold cancel PR was merged, all upstream files expected by this PR are now available. /assign @msau42 |
k8s-ci-robot
removed
the
do-not-merge/hold
pohly
changed the title
There was a problem hiding this comment.
@pohly Thanks for this, I just noticed the URLs were outdated and saw your PR. I'm not completely sure about using the versioned manifest, although your update-example.sh script is slick! IIUC that means we introduce another dependency on another repo which means, something will change and we'll forget again :)
Other than that, I'd love to see the fixes you have to the URLs and the clipboard issues merged. One other thing is maybe fix the "feature-dates" in the same Example.md file while you're cleaning up the rest of it?
| for url in $(grep 'kubectl create -f.*https://raw.githubusercontent.com/' $example | sed -e 's;.*\(https://raw.githubusercontent.com[^ (]*\).*;\1;'); do | ||
| # split URL into pieces | ||
| eval $(echo $url | sed -e 's/\]$//' -e 's;https://raw.githubusercontent.com/\([^/]*/[^/]*\)/\([^/]*\)/\(.*\);repo=\1 rev=\2 file=\3;') | ||
| git fetch https://github.com/$repo refs/heads/master:refs/remotes/$repo/master |
There was a problem hiding this comment.
Instead of latest, would it make more sense to pull specific tags from released versions?
There was a problem hiding this comment.
@msau42 That makes more sense in a lot of ways, my concern though would be making sure we remember to update things like the docs repo. That being said, it would be easy to just set a version file and update it with each CSI release and update that way. Could even potentially automate it at some point?
There was a problem hiding this comment.
Yes, we could setup some fancy CI that automates a lot of this. In the meantime, it will have to be one of those things to remember to do with every release.
There was a problem hiding this comment.
I had considered a parameter to pull from a branch, but then didn't add that because I wasn't sure whether we would ever need it. If needed and easier than manually updating the Example.md, it can still be added at that time.
There was a problem hiding this comment.
Long term it would be nice if the release process of the sidecar containers could also include publishing of the .yaml files for that container at the same location as the container, i.e. quay.io. Not sure whether that is possible, though. The result (hopefully) would be URLs for the .yaml files that then match the container image name and thus would be easier to review.
Maintaining local copies of all the necessary files is unnecessary, we might as well tell kubectl to pull directly from the individual repos. But to ensure that this really pulls the right revision of each file, we have to specify the revision hashes instead of the "master" branch. The markup was changed for these reasons: - the "copy to clipboard" feature wasn't very useful for the commands because it copied the entire command and its output - the long commands weren't wrapped and the clipboard icon was shown on top of them - inlining the yaml file source into the html version isn't easy anymore (would require mdbook preprocessor?), so instead all "kubectl create -f" URLs are links to the non-raw version in GitHub (non-raw because then one has easy access to commit logs) - commands and output are now using a different background, which is a bit easier to tell them apart Updating those hashes manually can be cumbersome. The new hack/update-example.sh script automates that step. The remaining local .yaml files are the ones that have no corresponding upstream file. It might be worth pushing them upstream in the future, because then everything will be in one place (hostpath deployment + examples) and/or easier to update (first update in upstream repo, then update docs with the hashes).
|
@j-griffith I've updated the deployment section. Besides fixing the |
There was a problem hiding this comment.
Thanks for all the fix ups @pohly, lgtm and IMO any refinements on versioning etc could be added later.
|
/assign @lpabon |
|
let's go with your change for now, and then add a page with "Releases" which has many versions of these pulled yamls |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lpabon, pohly 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
Maintaining local copies of all the necessary files is unnecessary, we
might as well tell kubectl to pull directly from the individual
repos. But to ensure that this really pulls the right revision of each
file, we have to specify the revision hashes instead of the "master"
branch.
Updating those hashes manually can be cumbersome. The new
hack/update-example.sh script automates that step. The remaining local
.yaml files are the ones that have no corresponding upstream file. It
might be worth pushing them upstream in the future, because then
everything will be in one place (hostpath deployment + examples)
and/or easier to update (first update in upstream repo, then update
docs with the hashes).