On Mon, Jan 23, 2023 at 02:19:23PM +0100, Erik Skultety wrote:
...
> > Although an option, the main motivation here to remain consistent with how it
> > works in GitLab. Since we define SCRATCH_DIR under the 'vars' section,
IIRC you
> > can only use a scalar value, not a command (if we can, I retract my argument)
> > and hence we'd have to export and define the variable under each script,
> > before_script, after_script sections.
> >
> > So, I preferred consistency here, since I wouldn't realistically expect
an
> > engineer to have /tmp/scratch prior to executing script given the main
> > motivation here is to quickly get a throwaway test machine to run the script
> > and THEN debug if the tests fail. So, while I agree you're right here, I
don't
> > think it's a likely scenario.
>
> I'm wondering why I put it at /tmp/scratch in the first place when we
> started using gitlab, and struggling to come up with a justification.
>
> In fact I'm not entirely convinced we really need a SCRATCH_DIR env
> variable at all, since AFAICT, we only use it one place for creating
> $VROOT.
>
> In terms of standalone reproduction of build env, it would be saner
> to use VOORT=$CWD/vroot, which I think would probably work under
> GitLab ok too, and do away with SCRATCH_DIR.
That would be just a replacement of one var for another and we'd still have to
keep clearing/removing vroot anyway - one thing I didn't mention, because it
was irrelevant up until this point, is that in the future we should improve the
local experience even more by following the logic we have for local container
envs where we mount the git directory inside the container as a volume. Using
the same mantra, I can imagine us using e.g. virtiofs to mount the git dir to the
VM so that the developer can run ninja build immediately without having to wait
for gitlab and test directly with their own development branch in a safe
environment.
The difference is that while /tmp would be cleared on VM destroy, vroot's
content as you propose would remain as a side-effect after destroying the VM
unless we clear it in which case it's no different to the current SCRATCH_DIR
solution, so with this in mind I think we should keep it the way we have.
The /tmp directory is often a ram disk though, hence my aversion to
using /tmp, as you can't be confident it has sufficient space for
without impacting other aspects of the system. With $CWD/vroot you
can expect access to the full extent of the user's storage.
If we're passing through a host git checkout though, that's sketchy,
unless we did a git clone --reflink, to get a pristine checkout for
each job in an efficient way.
With regards,
Daniel
--
|:
https://berrange.com -o-
https://www.flickr.com/photos/dberrange :|
|:
https://libvirt.org -o-
https://fstop138.berrange.com :|
|:
https://entangle-photo.org -o-
https://www.instagram.com/dberrange :|