Updated: [rhinstaller/anaconda/pulls/209 master] fix storage writing for live and ostree installs (#1236937)

[dashea]

3e800998 broke storage writing in two cases: regular live installs and ostree installs. The old write_storage_late check checked for flags.livecdInstall and ksdata.ostreesetup.seen as well as ksdata.method.method == "liveimg". The new check in writeStorageLate() dropped those two, only keeping the method check. The method is only 'liveimg' when doing a kickstart-driven install with the 'liveimg' parameter, which is a fairly niche case - the method is not 'liveimg' on a regular live install (it's 'harddrive').

[master 1/1] fix storage writing for live and ostree installs (#1236937)

[dashea]

From: Adam Williamson

3e800998 broke storage writing in two cases: regular live
installs and ostree installs. The old write_storage_late check
checked for flags.livecdInstall and ksdata.ostreesetup.seen as
well as ksdata.method.method == "liveimg". The new check in
writeStorageLate() dropped those two, only keeping the method
check.

Since the idea here is that payloads choose which writeStorage
method to use and override the other with a pass, let's change
the conditionals in the writeStorage methods to only check for
dirinstall, and tweak the payloads to match the behaviour we
want: LiveImagePayload, LiveImageKSPayload and RPMOSTreePayload
choose writeStorageLate(), DNFPayload chooses
writeStorageEarly(). Note - LiveImageKSPayload inherits from
LiveImagePayload. I left TarPayload alone since it's incomplete,
but I guess it'd want Late.
---
 pyanaconda/packaging/__init__.py    | 4 ++--
 pyanaconda/packaging/dnfpayload.py  | 3 +++
 pyanaconda/packaging/livepayload.py | 6 +++---
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/pyanaconda/packaging/__init__.py b/pyanaconda/packaging/__init__.py
index ada75ff..870d289 100644
--- a/pyanaconda/packaging/__init__.py
+++ b/pyanaconda/packaging/__init__.py
@@ -605,7 +605,7 @@ def writeStorageEarly(self):
            just the dnfpayload.  Payloads should only implement one of these
            methods by overriding the unneeded one with a pass.
         """
-        if self.data.method.method != "liveimg" and not flags.dirInstall:
+        if not flags.dirInstall:
             self.storage.write()

def writeStorageLate(self):
@@ -614,7 +614,7 @@ def writeStorageLate(self):
            every payload except for dnf.  Payloads should only implement one of
            these methods by overriding the unneeded one with a pass.
         """
-        if self.data.method.method == "liveimg" and not flags.dirInstall:
+        if not flags.dirInstall:
             if iutil.getSysroot() != iutil.getTargetPhysicalRoot():
                 setSysroot(iutil.getTargetPhysicalRoot(), iutil.getSysroot())

diff --git a/pyanaconda/packaging/dnfpayload.py b/pyanaconda/packaging/dnfpayload.py
index 818eac4..311c7f8 100644
--- a/pyanaconda/packaging/dnfpayload.py
+++ b/pyanaconda/packaging/dnfpayload.py
@@ -848,3 +848,6 @@ def postInstall(self):
                 log.error(e)

super(DNFPayload, self).postInstall()
+
+    def writeStorageLate(self):
+        pass
diff --git a/pyanaconda/packaging/livepayload.py b/pyanaconda/packaging/livepayload.py
index c0508a5..5056fed 100644
--- a/pyanaconda/packaging/livepayload.py
+++ b/pyanaconda/packaging/livepayload.py
@@ -201,6 +201,9 @@ def _updateKernelVersionList(self):
     def kernelVersionList(self):
         return self._kernelVersionList

+    def writeStorageEarly(self):
+        pass
+
 class DownloadProgress(object):
     """ Provide methods for download progress reporting."""

@@ -539,6 +542,3 @@ def kernelVersionList(self):
             # Strip out vmlinuz- from the names
             return sorted((n.split("/")[-1][8:] for n in names if "boot/vmlinuz-" in n),
                     key=functools.cmp_to_key(versionCmp))
-
-    def writeStorageEarly(self):
-        pass

[rhinstaller/anaconda/pulls/209 master] fix storage writing for live and ostree installs (#1236937)

[dashea]

Reopened.

[rhinstaller/anaconda/pulls/209 master] fix storage writing for live and ostree installs (#1236937)

[dashea]

OK, how about this version? It does what I suggested in the last comment. Locally I added a couple of debug messages right above the `self.storage.write()` calls so I could tell for sure when the storage write happened; I then tested a regular live install and a boot.iso install. As intended, only `writeStorageEarly()` ran on the boot.iso install and only `writeStorageLate()` on the live install. Both installed systems had a `/etc/fstab` written by anaconda once install was complete.

I also thought of an even simpler alternative to this: simply run both `Early` and `Late` for all installs. AFAICS, we have `Early` because for a package-based install, some package scripts need `/etc/fstab` to exist, so we want to write it before the package install process starts...but there would be no harm caused by writing it *again* after the install process was complete. We have `Late` because on live-style installs where we dump an entire disk image to the system that overwrites the `/etc/fstab` written by `Early`...but again, there'd be no harm caused by writing it Early as well as Late in that case. So, wouldn't it actually be fine to just write it twice for all installs? (well, we can keep the `if not flags.dirInstall` checks and not do storage writing at all when `flags.dirInstall` is set, I presume that's there for a good reason).

[rhinstaller/anaconda/pulls/209 master] fix storage writing for live and ostree installs (#1236937)

[dashea]

Merged by hand.

[rhinstaller/anaconda/pulls/209 master] fix storage writing for live and ostree installs (#1236937)

[dashea]

Closed.