Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use host ostree as localcache repo #94

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

wjt
Copy link
Member

@wjt wjt commented Nov 23, 2022

When running the image builder on Endless OS, there is a high chance that the host system will be running a similar, if not identical, OSTree commit to the one being built into the image. In my case, I run master and update essentially every morning, so running the image builder with its default arguments will use the same OSTree as is booted.

ostree pull supports a --localcache-repo PATH argument:

Like git's clone --reference. Reuse the provided OSTree repo as a
local object cache when doing HTTP fetches.

In this patch, we bind-mount the host's /ostree/repo into the buildroot, if it exists. The bind-mount is made read-only to reduce the risk that a bug in the image builder will mess up the host system.

This is obviously a hack but I thought I'd give it a try and it seems to make the build run somewhat faster locally. Of course from the second build on any given day it has no effect because the image builder's cache has the OS cached, so maybe it is not worthwhile. So this PR is more of an RFC.

@wjt wjt requested a review from dbnicholson November 23, 2022 15:47
@jprvita
Copy link
Contributor

jprvita commented Nov 23, 2022

I have not looked at the code too closely, but I think this is a good idea. IIUC the image builder's cache you are referring to is per-worker, so that one only helps on the second build running on the same worker on a given day.

@wjt
Copy link
Member Author

wjt commented Nov 24, 2022

I am not considering the case where this runs on Jenkins – only for local development. This will not make any difference on Jenkins because those workers are not running an OSTree version of Endless OS and so do not have /ostree/repo.

@wjt
Copy link
Member Author

wjt commented Nov 10, 2023

@dbnicholson I remembered this patch today because I found myself waiting on the "pull the ostree" stage of an image build during development. WDYT?

@wjt wjt force-pushed the use-host-ostree-as-localcache-repo branch from 03e2b52 to 1f522f2 Compare November 10, 2023 16:13
@dbnicholson
Copy link
Member

@dbnicholson I remembered this patch today because I found myself waiting on the "pull the ostree" stage of an image build during development. WDYT?

I think it's a good idea, although I have a patch to construct the buildroot with an ostree (to avoid needing to do the whole thing with mmdebstrap). In that case the ostree is pulled right in the beginning using the host's ostree and you wouldn't need to plumb the host repo into the buildroot.

run-build Outdated Show resolved Hide resolved
@dbnicholson
Copy link
Member

@dbnicholson I remembered this patch today because I found myself waiting on the "pull the ostree" stage of an image build during development. WDYT?

I think it's a good idea, although I have a patch to construct the buildroot with an ostree (to avoid needing to do the whole thing with mmdebstrap). In that case the ostree is pulled right in the beginning using the host's ostree and you wouldn't need to plumb the host repo into the buildroot.

Needs a bit of polish (and a proper issue to track), but 6cd8a23 is what I'm talking about.

@wjt wjt force-pushed the use-host-ostree-as-localcache-repo branch 2 times, most recently from 5d286e5 to 16f1652 Compare November 15, 2023 10:54
@wjt wjt marked this pull request as ready for review November 15, 2023 10:54
@wjt
Copy link
Member Author

wjt commented Nov 15, 2023

Triggered https://ci.endlessos.org/job/image-build-amd64/28092/console to test this on a non-OSTree system; I'll kill it once it successfully pulls the OS.

@wjt wjt force-pushed the use-host-ostree-as-localcache-repo branch from 16f1652 to 68da71a Compare November 15, 2023 10:56
@wjt wjt changed the title WIP: Use host ostree as localcache repo Use host ostree as localcache repo Nov 15, 2023
@wjt
Copy link
Member Author

wjt commented Nov 30, 2023

I left my system running a test build of #135 and after 35 minutes it had not yet finished pulling the master ref for its buildroot. This branch seems to work fine both on my laptop and (when I ran a test build above after rebasing on your changes to use ostree for the buildroot) on the build server. WDYT?

When running the image builder on Endless OS, there is a high chance
that the host system will be running a similar, if not identical, OSTree
commit to the one being built into the image. In my case, I run master
and update essentially every morning, so running the image builder with
its default arguments will use the same OSTree as is booted.

`ostree pull` supports a `--localcache-repo PATH` argument:

> Like git's clone --reference. Reuse the provided OSTree repo as a
> local object cache when doing HTTP fetches.

On my developer system & internet connection (80 Mbps according to
fast.com), in the case where my system repo has the latest commit but
the eos-image-builder cache repo does not, the pull takes 15 seconds
with this patch. Without it, the pull takes, let's say, substantially
longer. I gave up after 10 minutes, after which time 26% of objects have
been pulled, topping out at around 900 kB/s.
@wjt wjt force-pushed the use-host-ostree-as-localcache-repo branch from 68da71a to 1e4433c Compare December 19, 2023 09:53
log.info(f'Pulling OSTree ref {remote}:{ref}')
eib.retry(subprocess.check_call, [
'ostree',
f'--repo={repo_path}',
'pull',
*extra_pull_args,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the list within the list get flattened? I wouldn't expect it, but maybe subprocess does that and I've never tried.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what the * does:

>>> extra_pull_args = ['--localcache-repo', "/ostree/repo"]
>>> ["ostree", "pull", *extra_pull_args, "eos", "my-cool-ref"]
['ostree', 'pull', '--localcache-repo', '/ostree/repo', 'eos', 'my-cool-ref']
>>> ["ostree", "pull", extra_pull_args, "eos", "my-cool-ref"]
['ostree', 'pull', ['--localcache-repo', '/ostree/repo'], 'eos', 'my-cool-ref']

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I totally missed that. 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had no idea you could compose lists that way. That's a nice feature.

@dbnicholson dbnicholson merged commit fb01b74 into master Dec 19, 2023
2 checks passed
@dbnicholson dbnicholson deleted the use-host-ostree-as-localcache-repo branch December 19, 2023 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants