Add PR based override for machine-os-content #21919
Conversation
openshift-ci-robot
added
the
size/S
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: smarterclayton The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
@cgwalters testing this with ci-operator manually, hopefully will be able to confirm I can add that |
|
/retest |
| yumdownloader --destdir=/tmp/rpms origin-node origin-clients && \ | ||
| for i in $(find /tmp/rpms/ -name origin-*); do rpm2cpio $i | cpio -div; done && \ | ||
| ostree --repo=/srv/repo commit --parent=$commit --tree=dir=. --orphan --selinux-policy / \ | ||
| -s "origin-ci-dev overlay RPMs" --branch=origin-ci-dev |
There was a problem hiding this comment.
This needs to include the previous tree too so that it's a complete OSTree. See similar code in rpm-ostree: https://github.com/projectatomic/rpm-ostree/blob/master/tests/vmcheck/overlay.sh. It's a mouthful, but essentially:
ostree checkout <parent-tree> /tmp/working
<overlay binaries>
ostree commit /tmp/working --branch=origin-ci-dev
There was a problem hiding this comment.
Actually, just ostree commit --tree=ref=<parent-tree> --tree=dir=. should work too.
There was a problem hiding this comment.
Ok, add --tree-ref
There was a problem hiding this comment.
--tree-ref=$commit?
There was a problem hiding this comment.
--tree=ref=$commit --tree=dir=. will basically start off with the same tree as $commit and then overlay . on top of that.
There was a problem hiding this comment.
(Another minor thing: --orphan and --branch are contradictory. We should probably error out on that. :) You can drop the --orphan and we can tweak pivot as mentioned to just default to the one branch that exists if there's no com.coreos.ostree-commit)
smarterclayton
force-pushed
the
os_image
branch
from
5b7fa62 to
1486315
Compare
|
This looks pausible to me. One minor thing is to add Bigger picture I'd like to roll this functionality into rpm-ostree which will fix various things (e.g. the rpm database will be correct). But we don't need to do that right now. It'd be nice to test this though before landing; but on the other hand today nothing will actually apply the updated oscontainer...once we land that work we can circle back here and e.g. add a test case that verifies that the kubelet's git hash is the same as the PR? |
images/os/Dockerfile
Outdated
| commit=$( find /srv -name *.commit | sed -Ee 's|.*objects/(.+)/(.+)\.commit|\1\2|' | head -1 ) && \ | ||
| mkdir /tmp/working && cd /tmp/working && \ | ||
| yumdownloader --destdir=/tmp/rpms origin-node origin-clients && \ | ||
| for i in $(find /tmp/rpms/ -name origin-*); do rpm2cpio $i | cpio -div; done && \ |
There was a problem hiding this comment.
Minor: I'd add a -iname '*.rpm' there to be extra explicit.
| yumdownloader --destdir=/tmp/rpms origin-node origin-clients && \ | ||
| for i in $(find /tmp/rpms/ -name origin-*); do rpm2cpio $i | cpio -div; done && \ | ||
| ostree --repo=/srv/repo commit --parent=$commit --tree=ref=$commit --tree=dir=. --selinux-policy / \ | ||
| -s "origin-ci-dev overlay RPMs" --branch=origin-ci-dev |
There was a problem hiding this comment.
So, --selinux-policy / here is using the file contexts from the f29 buildroot, but we're running in RHEL. Probably fine if none of the files need special labels, but why not use a CentOS buildroot with CentOS' selinux-policy-targeted?
There was a problem hiding this comment.
Well...that makes it a bit harder to find ostree unfortunately 😢
There was a problem hiding this comment.
Hmm, yeah. So was looking at ostree ls -RX origin-ci-dev and it's basically all etc_t and bin_t (we're also including manpages right now, but meh... don't need to overoptimize here). Anyway, seems safe enough!
| yumdownloader --destdir=/tmp/rpms origin-node origin-clients && \ | ||
| for i in $(find /tmp/rpms/ -name origin-*); do rpm2cpio $i | cpio -div; done && \ | ||
| ostree --repo=/srv/repo commit --parent=$commit --tree=ref=$commit --tree=dir=. --selinux-policy / \ | ||
| -s "origin-ci-dev overlay RPMs" --branch=origin-ci-dev |
There was a problem hiding this comment.
Ahh OK, so one final gotcha here is that we also need to do the /etc -> /usr/etc translation (yet another reason why we should teach this to rpm-ostree). So basically just:
for i in $(find /tmp/rpms/ -name origin-*); do rpm2cpio $i | cpio -div; done && \
mv etc usr/ && \
...
This would take the machine-os-content base image and override it with our changes in the PR
smarterclayton
force-pushed
the
os_image
branch
from
1486315 to
4de957b
Compare
|
Merging so I can test jobs, if there's anything else me know and I'll fix. |
jlebon
left a comment
jlebon
left a comment
There was a problem hiding this comment.
Looks good to me! We can follow up if we discover anything else.
|
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
6 participants
This would take the machine-os-content base image and override it
with our changes in the PR