-
-
Notifications
You must be signed in to change notification settings - Fork 122
fix: container os still sometimes reported erroneously #1271
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
base: dev
Are you sure you want to change the base?
Conversation
MauriceNino
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I am having a bit of doubt, that this is the best solution to the problem. There is a lot of manipulation going on, just so systeminformation can read the data from those specific files - maybe it would be better to just hook into the static information gathering here and manually read the values from the hosts files, similar to what systeminformation does, just at the correct base directory (here you can see what the lib does).
What do you think?
| ]; | ||
|
|
||
| export const setupOsVersion = async () => { | ||
| try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting up the OS version linking should not fail the entire startup. Adding a warning to the logs and going on is enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I can add back a warning to logs. I don't want to impact startup.
|
First, sorry for the delay, life has been busy in September.
I think you are correct. The main cause of the issue I am seeing is that NixOS uses absolute-path symbolic links for its However, there is still an issue with my earlier code which is that the use of
I'm not especially interested in rewriting the functionality in I think the code looks more complex than it is. Mainly, what is happening is that the host files are resolved (with a special link resolver which can handle absolute symlinks from inside Docker but also works outside of Docker), then they are linked appropriately within the container keeping their original names. The code initially would link both The code now seeks out all
I would be happy to clean up any errors that can occur on startup since I don't want this change to break startup behavior (or anything critical really). I just want to see the correct OS info, and ultimately this bug is more the fault of the NixOS maintainers for using absolute paths in the symbolic links to os-release, but they aren't fixing it. We can work around that bug here. Ultimately, you get the final say on dashdot. Do you want me to make the tweaks regarding startup failures and keep the behavior I outlined or is this too much complexity for a niche problem? What do you think? |
7d82c5e to
a9beff5
Compare
|
First of all, sorry for the huge delay in my answer! I have a hard time following why this is necessary. The way I see it, there are a few locations where the host OS can save its information and just checking and symlinking them should be enough, right? I am just not sure, if merging this is worth the added complexity. Because let's be real - if something breaks, I am the one who is going to maintain it. And I don't really get what's going on here. |
Description
Handle each possible OS file:
/etc/os-release/etc/lsb-release/etc/openwrt_release/usr/lib/os-releaseFollow broken links from the host (
/mnt/host) and link them to the same file within the container.In other words,
/mnt/host/etc/os-releasemay link to something else, like/mnt/host/nix/store/znmly1f5ahwz9yrl7h89ljqxyr0xrk94-etc-os-releaseand the container establishes/etc/os-releaseto link to the same file by resolving the host links.If at least one file from the host can be found and successfully linked, now the other candidate files from the container are cleaned up so that they do not incorrectly report the container's OS information.
I also improved the logging so you can see which candidate files were linked and which were not:
In this case, my host uses both
/etc/os-releaseand/etc/lsb-release, but neither of/etc/openwrt_releasenor/usr/lib/os-release. As such:Related Issue(s)
#1232