installer: Inject stamp file /etc/coreos-legacy-installer-initramfs

[cgwalters]

This is needed for coreos/coreos-installer#220
in the PXE case to distinguish whether or not we're in the initramfs
image designed for the legacy installer.

[cgwalters]

Ah yes, so if anyone wants to shed a tear for the 40 minutes I spent chasing down

diff --git a/src/cmd-buildextend-installer b/src/cmd-buildextend-installer
index 93c441bd..a0f2eb29 100755
--- a/src/cmd-buildextend-installer
+++ b/src/cmd-buildextend-installer
@@ -130,7 +130,7 @@ def generate_initramfs_stampfile(tmpdir, destpath, stampname):
         run_verbose(['cpio', '-o', '-H', 'newc', '-R', 'root:root',
             '--quiet', '--reproducible', '--force-local',
             '-D', tmproot, '-O', destpath + '.tmp'],
-            input=stamppath.encode())
+            input=stampname.encode())
         run_verbose(['gzip', destpath + '.tmp'])
         os.rename(destpath + '.tmp.gz', destpath)

that might console me or something.

[dustymabe]

I'm not sure I'm following everything so bear with me. Why not just use the opposite logic since we already have the equivalent of a stamp file in the "fat" initramfs? i.e. [! -f /root.squashfs]

[cgwalters]

Why not just use the opposite logic since we already have the equivalent of a stamp file in the "fat" initramfs? i.e. [! -f /root.squashfs]

I think you're right, that would work. I can change the other PR to do that.

But...it seems a lot better to me to be explicit rather than implicit. Among other things, it's quite possible we change where the rootfs squashfs is at some point, and I wouldn't expect to need to change the coreos-installer legacy branch too.

[dustymabe]

But...it seems a lot better to me to be explicit rather than implicit. Among other things, it's quite possible we change where the rootfs squashfs is at some point, and I wouldn't expect to need to change the coreos-installer legacy branch too.

Right. I think I'd be less on the fence about explicit vs implicit if the "legacy installer" weren't going away. Since it's going away I'd be fine with an implicit relationship, but interested in what others think.

[cgwalters]

What's the problem with injecting an explicit stamp in the legacy path? I don't know a timeline for removing it from the RHCOS path, it may be harder than we think.

That said, at this point it might be easier to fork buildextend-{installer,live} into two commands that share code (and kill off fulliso while we're there); but before we do that we should probably land #1244

[cgwalters]

Why not just use the opposite logic since we already have the equivalent of a stamp file in the "fat" initramfs? i.e. [! -f /root.squashfs]

Wait, I remembered the real reason now. It's because the legacy coreos-installer module ends up in all initramfs images and so if someone e.g. booted a qemu image with coreos.install we'd actually run it, which I don't think we should do.

[cgwalters]

(Now...with some more hackery to the build system we could actually just inject the legacy installer code into the legacy initramfs but...)

[cgwalters]

(Side note, I am so happy that we now have CI coverage for this! Was really useful to have the PR fail.)

[dustymabe]

Wait, I remembered the real reason now. It's because the legacy coreos-installer module ends up in all initramfs images and so if someone e.g. booted a qemu image with coreos.install we'd actually run it, which I don't think we should do.

With the legacy installer today we allow this. It was or could be viewed as one of the benefits of the old approach: "Want to reinstall your system? Just add some kargs to grub and go!"

IOW this would be a behvaior change. So we should probably consider the implications and if it's worth the behavior change or not.

[cgwalters]

With the legacy installer today we allow this. It was or could be viewed as one of the benefits of the old approach: "Want to reinstall your system? Just add some kargs to grub and go!"

Sorry, I wasn't clear. I actually more meant we want to always defer installation via kernel arguments to the new installer except in the case where the old initramfs or ISO are booted.

[dustymabe]

With the legacy installer today we allow this. It was or could be viewed as one of the benefits of the old approach: "Want to reinstall your system? Just add some kargs to grub and go!"

Sorry, I wasn't clear. I actually more meant we want to always defer installation via kernel arguments to the new installer except in the case where the old initramfs or ISO are booted.

Ok. That makes more sense. In a scenario where we were to provide both installers (ongoing discussion about that) then it would make sense to disable the old installer if not booted specifically from the old installer artifacts.

[dustymabe]

/approve

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

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

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]

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

[cgwalters]

OK CI is green here and now have coreos/coreos-installer#220 fully working!

[jlebon]

OK took some time to digest this given the multiple branches, artifacts, distros, and repos going on here. One last thing I don't quite follow is the commit message says "in the PXE case" -- the logic in coreos/coreos-installer#220 kicks in equally well in the ISO boot case, no?

[cgwalters]

One last thing I don't quite follow is the commit message says "in the PXE case" -- the logic in coreos/coreos-installer#220 kicks in equally well in the ISO boot case, no?

Yes, I'd think so, but for whatever reason only PXE seemed to consistently hit it here.

[cgwalters]

Anything else blocking a /lgtm on this?

[jlebon]

Sorry, forgot to circle back on this.
/lgtm