Closed 1468ca0b-2a64-4fb4-8e52-ea5806644b4c closed 8 months ago
Created by: nc6
LGTM
Created by: qnikst
Successfully bootstrapped on a cluster.
Created by: qnikst
Addressed comments, not tested on a cluster, yet.
Created by: nc6
I don't think so, really. Per Halon both clients and servers are very similar - it's really only for SSPL purposes that we distinguish them. So I think satellite
with remain the general term.
Created by: qnikst
Afaiu this is temporary requirement that should go away in future, right? So I mean satellite
is the thing that should be definitely called by halond, but right server
is fine here.
Created by: qnikst
Because I have forgot that!! (or forgot to push)
Created by: qnikst
Right, though I think we need more clear solution here..
Created by: qnikst
Ok
Created by: nc6
Sure. I don't think we need to address as part of this PR.
Created by: nc6
Right, but to start a client, we have to call halonctl bootstrap satellite
. So I just think the terminology is unclear.
Created by: qnikst
I think we need to gather requirements for halon description and create file to describe that.
HalonConfig
is intended to be passed as a file in future.
Created by: qnikst
We have clear distinction between clients
and satellites
. And there is an option to not start clients
.
Created by: nc6
Review done, mostly minor naming issues, plus a couple of questions and one "will need to be done at some point".
Created by: nc6
Any reason why not? Looks like you could just add it to ValidatedConfig
...
Created by: nc6
Move these timeouts to config, with a default there?
Created by: nc6
Parse the config here in the same way, for consistency?
Created by: nc6
Could do this as a separate PR, though.
Created by: nc6
This is probably fine for now, but we will need to introduce an additional role for this, I think (may need 'halon roles' rather than 'mero roles'), since we will want multiple station nodes and Mero does not yet support multiple confds. We could then move the "default configuration" from being hardcoded in this file to being specified in the role maps file.
Created by: nc6
Seems badly named, as clients are also satellites. (clients, servers)
?
Created by: qnikst
lnidToIP?
Created by: nc6
Rename this? takeString
is very generic. takeIP
, perhaps?
Created by: qnikst
Ok, those are fqdn and IP address.
Created by: nc6
Maybe document these parameters? What are these arrays of pairs of strings of?
Closing as an obsolete
Created by: qnikst