FactomProject / factomd

Factom Daemon
https://www.factomprotocol.org/
Other
201 stars 92 forks source link

Show hostname on console and control panel #608

Closed stackdump closed 5 years ago

stackdump commented 6 years ago

From Discord:

[Bedrock] jcheroskeToday at 12:57 PM Not sure if this is the right place to ask this, but I was wondering if the 8090 console could have the hostname added to the display? All my nodes look alike! ... [Bedrock] jcheroskeToday at 1:33 PM It's obviously not a big deal, but it's a feature I'd like to see in the console. That way, no matter how you connect to your node, you'll see the name that the node itself is called. Seems useful

PaulBernier commented 6 years ago

Am I understanding correctly that html files in https://github.com/FactomProject/factomd/tree/master/controlPanel/Web/templates get "compiled" into this go file: https://github.com/FactomProject/factomd/blob/master/controlPanel/files/templates/templates.go? If so, why? And if one edits an html file how does he produce the go compiled version?

sambarnes commented 6 years ago

The enterprise wallet does the same thing, and this is it's explanation:

This compiles our HTML, CSS, JS, and images into golang .go files. That way we don't need to worry about pathing while serving files as everything is compiled into the binary.

To compile the modified templates and js for the control panel, there's some instructions here: https://github.com/FactomProject/factomd/blob/master/controlPanel/README.md

This section of the codebase is mostly from Steven I believe. I'm not all that familiar with it yet, I just know the basic gist of it from when I was looking at enterprise-wallet stuff

stackdump commented 6 years ago

FYI currently would be OK to branch from Master to work on this - when you are ready someone can create an FD-XXX branch for you to merge into

Emyrk commented 6 years ago

@PaulBernier The compile script is here: https://github.com/FactomProject/factomd/blob/master/controlPanel/compile.sh This is the dependency for building: https://github.com/FactomProject/staticfiles

I need to update the readme, but we decided to not minify things, as the extra build step wasn't buying us much, as most of the files are served over localhost. We can move back to a minified build as the control panels are being used more remotely, but for now the 'uncompressed' part of that readme is out of date.

PaulBernier commented 6 years ago

Ok I managed to make the code change to display the info in the control panel this way:

screenshot from 2018-11-17 11-26-25

I asked jcheroske and operators and Discord and it was fine. The title of this issue also mentions display in console, what place is this referring to exactly?

Also that works fine if factond is running directly on the OS... but I experimented with the docker container and the hostname of a docker container is the container id (not the hostname of the host OS)... Maybe there is an option to force the hostname of the container, would need to be investigated.

PaulBernier commented 6 years ago

For Docker container hostname, seems it is as easy as adding a --hostname parameter when launching the container (https://www.digitalocean.com/community/tutorials/naming-docker-containers-3-tips-for-beginners), so all good!

jcheroske commented 6 years ago

Yes, being able to set the name manually via a startup parameter or via a conf file setting will work just as well. I can do the hostname lookup in ansible and then pass it in.

Jay

On Fri, Nov 16, 2018 at 5:09 PM Paul Bernier notifications@github.com wrote:

For Docker container, seems it is as easy as adding a --hostname parameter when launching the container ( https://www.digitalocean.com/community/tutorials/naming-docker-containers-3-tips-for-beginners), so all good!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/FactomProject/factomd/issues/608#issuecomment-439573823, or mute the thread https://github.com/notifications/unsubscribe-auth/ADgUyvHQ6KeDbn__rEftv2HgVdFY-H2jks5uv2HGgaJpZM4YeeZB .

PaulBernier commented 5 years ago

@stackdump so if someone can create a branch I could push to show you my little change to the control panel.

carryforward commented 5 years ago

yes, this is awesome. This will be totally useful all over the place. I look forward to using this.

I think I understand. I like @jcheroske 's idea of setting it on the command line. maybe with the --nodename flag? In AWS the hostname is generally not useful, as it is a non-intuitive IP address.

A couple niggles.

  1. there might be multiple nodes on the same host. Might this be better called a node name?

(In a similar line of thought, the simulator has multiple control panels in a single node. I don't want to get into naming different control panels on the simulator just yet though. that would get overly complicated for the value it would provide.)

  1. I know it will involve a lot more plumbing, but that big "YOUR NODE" has been staring us in the face for years. It seems like that is begging to be set as a name of the node. This in combination with the version string being arbitrarily set at compile time, that might get confused with the version being tested.

We can also print this name out onto the console when the node boot, similar to how the version is printed out. That will be valuable keeping logs identified as well.

Please use this branch to pull against: FD-758_node_name_on_ctrl_panel

I am looking forward to using this. It will add a lot of value.

jcheroske commented 5 years ago

Brian, you're reading my mind. I was going to suggest it be just a human readable name that the admin can set to whatever they need. Goodbye "Your Node"

On Mon, Nov 19, 2018 at 6:25 PM Brian Deery notifications@github.com wrote:

yes, this is awesome. This will be totally useful all over the place. I look forward to using this.

I think I understand. I like @jcheroske https://github.com/jcheroske 's idea of setting it on the command line. maybe with the --nodename flag? In AWS the hostname is generally not useful, as it is a non-intuitive IP address.

A couple niggles.

  1. there might be multiple nodes on the same host. Might this be better called a node name?

(In a similar line of thought, the simulator has multiple control panels in a single node. I don't want to get into naming different control panels on the simulator just yet though. that would get overly complicated for the value it would provide.)

  1. I know it will involve a lot more plumbing, but that big "YOUR NODE" has been staring us in the face for years. It seems like that is begging to be set as a name of the node. This in combination with the version string being arbitrarily set at compile time, that might get confused with the version being tested.

We can also print this name out onto the console when the node boot, similar to how the version is printed out. That will be valuable keeping logs identified as well.

Please use this branch to pull against: FD-758_node_name_on_ctrl_panel

I am looking forward to using this. It will add a lot of value.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/FactomProject/factomd/issues/608#issuecomment-440113986, or mute the thread https://github.com/notifications/unsubscribe-auth/ADgUym8TLVCJMJczNmQpXh8CjR28gCYEks5uw2gUgaJpZM4YeeZB .

PaulBernier commented 5 years ago

Ok if I sum up the requirements so far:

carryforward commented 5 years ago

These both sound good: Display the nodename value in place of "YOUR NODE" in the control panel Display the nodename in the console when factomd boot

This sounds desirable, yes: introduce a new variable nodename that can be set via a factomd flag nodename.

This one I am not so sure of. we can get the same effect by using bash/environment variables. making it a default might get complex with different operating systems. There is only a limited amount of space, so I don't know if it will mess things up with unusually named machines. We could compensate by truncating, but that gets into more complex management that might be beyond the scope of this update. If that flag is not set, fallback to the hostname of the machine? I would be happy with simply specifying the node name instead of trying to deal with host names. Is the host name by default something that should be in there @PaulBernier?

jcheroske commented 5 years ago

1) You could always display the hostname in smaller text to the right, like in the image above. If the -nodename flag is not present, you could just default back to 'YOUR NODE' 2) Yes 3) Yes

So, for example, if no `-nodename flag was passed, it would read:

YOUR NODE v6.0.1@hostname.com

If you pass in -nodename, then you get:

NODENAME v6.0.1@hostname.com
carryforward commented 5 years ago

From my perspective at least from the portainer viewpoint, hostname is not terribly useful. On AWS it looks like ip-172-99-6-70. I'm hearing on docker it would require special flags to make work. On some VM systems it would show the name of the VM, but that is kind of specialized.

I was trying to stay away from combining the version field with other things, since this can be set to arbitrary values already: https://github.com/FactomProject/factomd/commit/169813760948eea4fe4c4eee803b4c87d39f09a7

How valuable is having both hostname and a nodename?

jcheroske commented 5 years ago

It's not necessary. I had forgotten about the previous discussion, and how getting the hostname in a docker environment is tricky. Just having a way to override the "YOUR NODE" display with an arbitrary value is all I really need.

PaulBernier commented 5 years ago

So after looking a bit at the code I noticed that the State object has an attribute FactomNodeName (Usually FNode0). I'm assuming I should not try to re use that variable right? Because I see it's already used in many places that seem non trivial.

stackdump commented 5 years ago

Yea it think mostly it would affect simulation where there would be several nodes running in the same go processes - but I agree it's safer not to use that.

PaulBernier commented 5 years ago

https://github.com/FactomProject/factomd/pull/620

carryforward commented 5 years ago

yah, definitely don't use that name Fnode0. There are tons of very weird dependencies on that name.

carryforward commented 5 years ago

This has been passed onto testing. the code review looks good.

carryforward commented 5 years ago

merged into master. Thank you paul!