devicestate: keep log from install-mode on installed system #9545
Conversation
| // is no systemd-journal-remote on core{,18,20} | ||
| // | ||
| // XXX: or only log if persistant journal is enabled? | ||
| logPath := filepath.Join(rootdir, "var/log/install-mode.log") |
There was a problem hiding this comment.
small suggestion to store this on the FAT partition appropriate for the device. We could also zip-compress it with a bunch of other files, like seed.yaml. This would be excellent for easy (windows) diagnostic of failed startup.
There was a problem hiding this comment.
That's an interessting idea. I wonder if there is any risk in storing this on a unencrypted partition, i.e. if it could contain sensitive information. This is why I opted for ubuntu-data but maybe this is more a development helper?
There was a problem hiding this comment.
I think the log could contain sensitive data so I think it should go on ubuntu-data
There was a problem hiding this comment.
how long does this take? do we know how big those logs are for example for our default amd64 install?
There was a problem hiding this comment.
The log is 214K on my nested-google amd64 install with debug. I have no exact timing data from the install bug running the equivalent command journalctl > file takes: real 0m0.049s.
There was a problem hiding this comment.
I agree that for now storing it on ubuntu-data is the safest approach.
This commit ensures that we capture the full log of the ephemeral install mode on the writable disk when installing the system. This is useful for debugging if install mode worked as expected or to get logs about why things are different than they should have been.
mvo5
force-pushed
the
install-mode-keep-logs-2
branch
from
b525993
to
afcef60
Compare
There was a problem hiding this comment.
I like this, I think it's good, I don't think we need to care right now whether persistent journal is enabled or not, this will be helpful even for folks trying uc20 on i.e. a stock rpi image where we can ask them for install mode logs from just putting their SD card into their main machine and looking at the specified file (assuming that install mode finishes up to that point).
I would really like to see the logs get integrated into the journald properly, is it really just /etc/machine-id we need to copy over from install mode to run mode for that to work? If so I think we could copy the ephemeral one generated at install time to run mode at the same place in devicestate
| // is no systemd-journal-remote on core{,18,20} | ||
| // | ||
| // XXX: or only log if persistant journal is enabled? | ||
| logPath := filepath.Join(rootdir, "var/log/install-mode.log") |
There was a problem hiding this comment.
how long does this take? do we know how big those logs are for example for our default amd64 install?
| } | ||
| defer f.Close() | ||
|
|
||
| cmd := exec.Command("journalctl") |
There was a problem hiding this comment.
there should always be only the log from current but, but maybe as extra safety we should call journalctl -b 0.
| defer f.Close() | ||
|
|
||
| cmd := exec.Command("journalctl") | ||
| cmd.Stdout = f |
There was a problem hiding this comment.
Just thinking, would this work?
logPath := filepath.Join(rootdir, "var/log/install-mode.log.gz")
...
f, err := os.Create(logPath)
...
gw := gzip.NewWriter(f)
defer gw.Close()
cmd.Stdout = gw
if err := cmd.Run(); err != nil {
return err
}
return gw.Flush()
There was a problem hiding this comment.
For reference, the log gets compressed quite vell:
216 -rw-r--r-- 1 root root 220057 Jan 12 13:37 install-mode.log
36 -rw-r--r-- 1 root root 35052 Jan 12 13:37 install-mode.log.gz
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
|
Thanks @bboozzoo for adding the gzip in there. |
This commit ensures that we capture the full log of the ephemeral
install mode on the writable disk when installing the system.
This is useful for debugging if install mode worked as expected
or to get logs about why things are different than they should
have been.
This is an initial PR - if this approach looks reasonable I will add
the missing unit tests (spread is there already).
We recently got asked to add this for QA purposes.