build: Require an explicit fetch stage

[cgwalters]

The recent "no network for builds" change broke the case where
one hadn't done at least one fetch; previously this worked
because rpm-ostree would fall back to fetching, but it can't
without networking.

Explicitly error out if a fetch hasn't been done.

[jlebon]

I think it's OK (and maybe better) if we explicitly split the fetch and build stages in CI/pipelines (in pipelines you probably always want them separate for cleaner no-op handling). OTOH, for the developer case, always requiring cosa fetch first can be annoying if you're twiddling with the manifest. Hmm, how about:

• add a cosa build --fetch which makes it run cosa fetch first
• have cosa fetch write the checksum of the flattened manifest to tmp/
• have cosa build imply --fetch if the flattened manifest checksum changed (and a missing checksum also implies --fetch)

?

That way, the developer case should be pretty unaffected.

[cgwalters]

I think we're in agreement it we should change the pipelines/CI to call both cosa fetch and cosa build explicitly in general.

have cosa build imply --fetch if the flattened manifest checksum changed (and a missing checksum also implies --fetch)

First, I don't think we should delay a tested fix for a rewrite for to a more complicated fix. Second, that would mean that if e.g. I end up editing a postprocess in the manifest I suddenly re-download packages, possibly changing what I was testing, so I don't think that's what we want either.

[dustymabe]

I think we're in agreement it we should change the pipelines/CI to call both cosa fetch and cosa build explicitly in general.

Yep. We started doing that with coreos/coreos-ci-lib#24

No - what's going on here is that rpm-ostree if passed --cache-only will still fetch if it doesn't have metadata, which IMO is a bug. But it's one we were relying on.

Maybe let's just fix the rpm-ostree bug and require people to slightly change their workflows to cosa fetch && cosa build.

That would mean we just change the pipelines and fix the rpm-ostree bug and don't merge this PR or change cosa build in any other way (i.e. no implied fetch).

[cgwalters]

Maybe let's just fix the rpm-ostree bug and require people to slightly change their workflows to cosa fetch && cosa build

Hmm....maybe. I think we need at least a transitional period though; which could look like this PR except we e.g. add

echo warning: automatically fetching; please explicitly `coreos-assembler fetch` in the future
sleep 1

or something. But I dunno if it's worth it.

[cgwalters]

On a procedural level I think we need to keep master working. So as a general rule when things break we should either merge a PR fixing it (even if not optimal), or revert the PR that broke things right away. Keeping master broken while we debate architecture isn't right. (Also, my time to work on things is highly variable and so if we block on me it could block for hours)

[cgwalters]

(Although, I guess in this case it was relatively easy to fix the pipelines and CI)

[jlebon]

Right, I don't think this is a pressing fire. CI is fixed (or to be more correct, it now does what it should've been doing already), pipelines were already split, and this PR doesn't apply to developer workdirs which already have builds. And we do advertise split fetch and build steps in the README for new developers.

One remaining thing affected I can think of is the developer workflow of hacking on the manifest and just running cosa build directly as mentioned in #1379 (comment).

Second, that would mean that if e.g. I end up editing a postprocess in the manifest I suddenly re-download packages, possibly changing what I was testing, so I don't think that's what we want either.

We could try to be smarter here, but meh... maybe let's just always require a separate fetch and build then. Which I guess is what we're aligning on already. :)

[cgwalters]

meh... maybe let's just always require a separate fetch and build then. Which I guess is what we're aligning on already. :)

OK done.

[dustymabe]

update comment to not mention auto-fetch

[dustymabe]

/approve

[dustymabe]

Let's hold merges to COSA briefly until we run our stable release for coreos/fedora-coreos-streams#83

[jlebon]

/approve

[dustymabe]

The pipeline run is done now.

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, dustymabe, jlebon

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:

• OWNERS [cgwalters,dustymabe,jlebon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment