usage of public and private ip addresses #115
Conversation
| rootCmd.Flags().StringVar(&cfg.publicIPv4, "public-ipv4", "", "public IPv4 address for this machine") | ||
| rootCmd.Flags().StringVar(&cfg.privateIPv4, "private-ipv4", "127.0.0.1", "private IPv4 address for this machine") | ||
| rootCmd.Flags().StringVar(&cfg.publicIPv4, "public-ipv4", "", "public IPv4 address for this machine (required)") | ||
| rootCmd.Flags().StringVar(&cfg.privateIPv4, "private-ipv4", "", "private IPv4 address for this machine (required)") |
There was a problem hiding this comment.
the default we had (localhost) for private IP worked really well for the Ubuntu single node scenario, where you don't have to pass a single param. I liked that, is there anyway we can still support that?
There was a problem hiding this comment.
my thought for the dev case is that you would put the env vars in your bashrc
ROOKD_PUBLIC_IPV4
ROOKD_PRIVATE_IPV4
| @@ -65,7 +65,7 @@ func TestOSDAgentWithDevices(t *testing.T) { | |||
| } | |||
|
|
|||
| // prep the etcd keys that would have been discovered by inventory | |||
| disksKey := path.Join(inventory.GetNodeConfigKey(context.NodeID), inventory.DisksKey) | |||
| disksKey := path.Join(inventory.GetNodeConfigKey(context.NodeID), "disks") | |||
There was a problem hiding this comment.
this change is a bit unexpected for this PR, what caused it?
There was a problem hiding this comment.
this was a side effect of making DisksKey a private disksKey. As I'm touching code, I look for places where we don't need public access. Since this is just a single test, it wasn't worth keeping the property public.
| for _, reqFlag := range requiredFlags { | ||
| val, err := cmd.Flags().GetString(reqFlag) | ||
| if err != nil || val == "" { | ||
| return createRequiredFlagError(cmd, reqFlag) | ||
| missingFlags = append(missingFlags, reqFlag) |
There was a problem hiding this comment.
ah nice, i like how multiple missing flags can be bubbled up in the error to the user now
travisn
force-pushed
the
public-private-ip
branch
from
c889275
to
12a195a
Compare
|
I dont think the ip address should be required. We should have some good default if nothing else but the simple dev scenario. |
|
what should the default ip addresses be? Should we look for eth0 and set the public and private ips both to the same address? I worry about automating that or setting a default for the dev environment such as 127.0.0.1 since you would never want that in a production env. |
|
the default should be to listen on all network interfaces. |
|
the ip addresses are also used for members of the cluster to know how to talk to each other, so we need specific ips. |
|
What does etcd default to? It comes up with no Params. Maybe local host? I still don't think the params should be required. |
…ecret and CM (rook#115) * delete k8s resources after Delete/Revoke return non-nil err Signed-off-by: jeffvance <jeff.h.vance@gmail.com> * add ownerReference Signed-off-by: jeffvance <jeff.h.vance@gmail.com> * review comments & gofmt Signed-off-by: jeffvance <jeff.h.vance@gmail.com>
The public and private ip addresses are now required parameters to castled.
rook node lswill show both the public and private ip, though we may want to change that.