Skip to content
This repository has been archived by the owner on Jun 12, 2023. It is now read-only.

updating miner info to grab firmware distro name #1263

Closed
wants to merge 3 commits into from

Conversation

jeffgrunewald
Copy link
Contributor

Updates the miner cli to pull the "firmware" linux disto pretty name from the now-standard /etc/os-release file which is in effect across all SystemD distros of linux (our current runtime option being Alpine) and the /etc/lsb_release being deprecated and not present on most distros any longer.

I've verified this file is present in most standard distros (Alpine, Ubuntu, Debian, Fedora) so this command should likely succeed in any supported version of Linux that runs miner but I can have it conditionally check for the older lsb_release file/command if this file isn't present as well for maximum compatibility.

@Vagabond
Copy link
Contributor

We need to fix our firmware image to have the right information in this file, too

@jeffgrunewald
Copy link
Contributor Author

We need to fix our firmware image to have the right information in this file, too

which firmware image is this? if it's the deb package build wouldn't that imply it runs on any debian-compatible system that would already have this file?

@jeffgrunewald
Copy link
Contributor Author

@Vagabond i believe i found the change you're recommending in the firmware image repo; tagged you for review. Please advise if there's anything else i'm missing or other changes you'd like

Copy link
Contributor

@evanmcc evanmcc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems reasonable, does it also need to be changed in the jsonrpc stuff?

@jeffgrunewald
Copy link
Contributor Author

this seems reasonable, does it also need to be changed in the jsonrpc stuff?

Didn't realize that was there and didn't use the same code; fixed @evanmcc !

@shawaj
Copy link
Contributor

shawaj commented Dec 20, 2021

Will this not just pull the docker base image info rather than the host OS? Or is that the intention?

@jeffgrunewald
Copy link
Contributor Author

Will this not just pull the docker base image info rather than the host OS? Or is that the intention?

For dockerized miners this will query the file inside the base image unless it’s being mounted over at runtime by the user. This is consistent with the current behavior.

@shawaj
Copy link
Contributor

shawaj commented Dec 21, 2021

Reason I ask, is mainly to understand what this would be used for in the future? Is there a need for us to show the base OS and expose this into the container or its just a sort of nice to have?

@jeffgrunewald
Copy link
Contributor Author

Reason I ask, is mainly to understand what this would be used for in the future? Is there a need for us to show the base OS and expose this into the container or its just a sort of nice to have?

Since this is an artifact from firmware builds I would argue that it’s more valuable to reflect the Linux distro/filesystem providing the miner’s runtime, so the value supplied by the base container image. As this base image changes the file will reflect the version of Linux powering the containerized miner. I would not recommend overriding this file, personally.

@PaulVMo
Copy link
Contributor

PaulVMo commented Jan 11, 2022

The same logic is also used in miner_utils to pull the version for gossip metadata. Shouldn't it be revised there as well?

iolist_to_binary(string:trim(os:cmd(?RELEASE_CMD)));

Also, is it intention that the OS version is being in the gossip instead of the miner version with something like:

    case erlang:hd(Releases) of
        {_,ReleaseVersion,_,_} -> ReleaseVersion;
        {error,_} -> undefined
    end.```

@jeffgrunewald
Copy link
Contributor Author

@PaulVMo the firmware readouts in the cli and jsonrpc info functions are really designed around the legacy functionality of helium's own firmware images, which constitute a custom-built linux distribution and included the release version number of the miner application. i've re-worked this PR to be more generic and append the miner release version number to the firmware name, taken from the linux distro's standard /etc/os-release if the number isn't already present in the output.

This will assume the /etc/os-release file bundled into the container image that we are moving toward as our standard package format but would allow manufacturers to mount their own custom /etc/os-release file into the container if they so choose. I believe this approach captures the best picture of the packaged software (miner and its OS runtime) we can reason about.

As for the gossip metadata, since the existing miner_util functions collecting that metadata were already extracting explicitly the miner version tag from the old /etc/lsb_release file i don't see any reason to collect that information from the filesystem anyway so i've simplified the request here.

@jeffgrunewald jeffgrunewald deleted the jg/update_cli_info branch October 6, 2022 14:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants