build+metal: Use shared ostree-importing code #2487
Conversation
cgwalters
force-pushed
the
more-oci
branch
2 times, most recently
from
32fb87e
to
78a9075
Compare
| @@ -161,10 +161,7 @@ if [ -n "${previous_commit}" ]; then | |||
| commitpartial=${tmprepo}/state/${previous_commit}.commitpartial | |||
| if [ ! -f "${commitpath}" ] || [ -f "${commitpartial}" ]; then | |||
| if [ -f "${previous_builddir}/${previous_ostree_tarfile_path}" ]; then | |||
| mkdir tmp/prev_repo | |||
| tar -C tmp/prev_repo -xf "${previous_builddir}/${previous_ostree_tarfile_path}" | |||
| ostree pull-local --repo="${tmprepo}" tmp/prev_repo "${previous_commit}" | |||
There was a problem hiding this comment.
is ${tmprepo} here tmp/repo? I'm just wondering if we should still have the path to the tmprepo be an argument to the new import_ostree_commit_for_build() function.
There was a problem hiding this comment.
Yeah, but we hardcode tmp/repo in a bunch of places.
| ostree_repo=$PWD/tmp/repo | ||
| fi | ||
| # Ensure that we have the cached unpacked commit | ||
| import_ostree_commit_for_build "${build}" |
There was a problem hiding this comment.
previously we were only doing the import if we needed to. if we needed to then we got a nice message about Cache for build ${build} is gone... - should we only import if we need to?
There was a problem hiding this comment.
The Python code will print out stuff when it's actually importing, and is also a no-op if the commit exists
coreos-assembler/src/cosalib/cmdlib.py
Line 242 in f5d003d
Ah...but actually testing this shows we were importing into the wrong place. Fixed.
There was a problem hiding this comment.
The Python code will print out stuff when it's actually importing, and is also a no-op if the commit exists
coreos-assembler/src/cosalib/cmdlib.py
Line 242 in f5d003d
Ahh. I see that now. Thanks for the link.
Only thing I would improve there is instead of just returning it would be nice to log that we're doing a no-op. i.e. inside the python fuction right before we return.
The metal case broke with gangplank+aarch64, because it is running separate pods for the build or something. In the Jenkins (x86_64) flow I think we bypass this because we have a single pod which already has the cached commit in `tmp/repo`. Anyways, dedup all this code using a shared helper.
The metal case broke with gangplank+aarch64, because it is
running separate pods for the build or something.
In the Jenkins (x86_64) flow I think we bypass this because we have a single
pod which already has the cached commit in
tmp/repo.Anyways, dedup all this code using a shared helper.