Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[snap] need to reach out of $SNAP for host properties #1781

Closed
Saviq opened this issue Oct 6, 2020 · 9 comments · Fixed by #2105
Closed

[snap] need to reach out of $SNAP for host properties #1781

Saviq opened this issue Oct 6, 2020 · 9 comments · Fixed by #2105

Comments

@Saviq
Copy link
Collaborator

Saviq commented Oct 6, 2020

All of our instances are reported as launched on ubuntu-core-18 now, due to strict confinement and base: core18. We should try and find a way to reach out to get the actual host OS.

pollinate_user_agent_string += fmt::format("multipass/host/{}-{} # written by Multipass\n", QSysInfo::productType(),
QSysInfo::productVersion());

@townsend2010
Copy link
Contributor

Qt reads /etc/os-release when it exists and uses that. Of course, this is the Ubuntu Core info. However, /etc/lsb-release within the snap does have actual host OS info. Sadly, I don't see a way to force Qt to use this ☹️ so it looks like we'll need our own solution for the Linux case.

@Saviq
Copy link
Collaborator Author

Saviq commented Oct 20, 2020

I wonder if it's a snapd bug, then, and it should actually expose host's /etc/os-release

@Saviq
Copy link
Collaborator Author

Saviq commented Oct 20, 2020

@townsend2010
Copy link
Contributor

Given that the output of /etc/os-release looks like this:

NAME="Ubuntu Core"
VERSION="18"
ID=ubuntu-core
PRETTY_NAME="Ubuntu Core 18"
VERSION_ID="18"
HOME_URL="https://snapcraft.io/"
BUG_REPORT_URL="http://bugs.launchpad.net/snappy/"

I would say that's pretty intentional. That said, I agree that if it's not the actual Ubuntu Core distribution running (and not the core snap), then it should probably have what the host has in there.

Right about LXD, but that still doesn't help with Qt (it reads a hard-coded /etc/os-release) which is why I mentioned that we'd need our own Linux solution.

@Saviq
Copy link
Collaborator Author

Saviq commented Oct 20, 2020

I would say that's pretty intentional.

Well, that would be true on a Ubuntu Core 18 system, it is not, on a Classic system with the core18 snap, IMO. And then the discrepancy between lsb- and os-release…

@townsend2010
Copy link
Contributor

Well, that would be true on a Ubuntu Core 18 system, it is not, on a Classic system with the core18 snap, IMO. And then the discrepancy between lsb- and os-release…

Right, what I was trying to say 😄

@townsend2010
Copy link
Contributor

Looking at the core18 environment and code, /etc/os-release is a link to /usr/lib/os-release which is set up by https://github.com/snapcore/core18/blob/master/hooks/018-set-os-release.chroot. The strange thing is that I can't find anywhere in the code where that link is created 🤔

@surahman
Copy link
Contributor

surahman commented May 6, 2021

Is the proposed solution to parse the file in /etc/lsb-release when a specific OS hostname is returned by QSysInfo::productType(), and if so what string pattern should I be on the lookout for? Sorry if I am misunderstanding or am oversimplifying things.

I am not running a core-18 image but would it be safe to assume that reading the /etc/lsb-release on such a system would in fact have the core-18 distro information specified there as well?

Parsing the /etc/lsb-release file would require extraction of the two fields DISTRIB_ID and DISTRIB_RELEASE, in my case:

DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=21.04
DISTRIB_CODENAME=hirsute
DISTRIB_DESCRIPTION="Ubuntu 21.04"

@surahman
Copy link
Contributor

surahman commented May 7, 2021

What I am thinking is we try to open the /etc/os-release file if it exists, and if not we failover to the original solution. Psuedocode:

QFile fileDescriptor("/etc/lsb-release")
if (!fileDescriptor.open(read-only-mode))
{
    return make_pair(QSysInfo::productType(), QSysInfo::productVersion())
}
else
{
    // read-in and parse file, return pair.
}

The real question is if the /etc/os-release files have a standard format that would be predictably parsed. I would also be assuming that the /etc/os-release file could be treated as ground truth for version info on Linux based systems.

bors bot added a commit that referenced this issue Jun 9, 2021
2105: [platform] Read Snap Linux OS Release r=ricab a=surahman

**_Issue #1781:_**
Creating changes requested in #2096 on a new branch instead of doing a hard reset on the original branch to maintain the changes in the original files whilst I work.

Tests have been added to the `PlatformLinux` fixture and an empty value in a field is treated as `unknown`:
```
NAME=""
...
VERSION_ID=""
```
I have not wired into `/src/daemon/daemon.cpp` string pollination because the suggested format of `distro_name-distro_rel` does not match the original.

Co-authored-by: Saad Ur Rahman <saad.ur.rahman@gmail.com>
@Saviq Saviq closed this as completed Jun 23, 2021
@Saviq Saviq linked a pull request Jun 23, 2021 that will close this issue
@Saviq Saviq added this to the v1.7.0 milestone Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants