Remove NodeIP configurable option from microshift config #1063
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dinhxuanvu The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
dinhxuanvu
force-pushed
the
nodeIP
branch
2 times, most recently
from
10e7933 to
aaffa68
Compare
|
@benluddy PTAL |
The NodeIP is always the host IP. Signed-off-by: Vu Dinh <vudinh@outlook.com>
dinhxuanvu
changed the title
|
@dinhxuanvu: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
| } | ||
|
|
||
| func findNodeIP() string { | ||
| hostIP, err := util.GetHostIP() |
There was a problem hiding this comment.
We expect hosts to have multiple IPs. How does the user influence how this function picks the IP to use?
There was a problem hiding this comment.
It seems the nodeIP option is not repected in main branch, even it is user configurable. We probably want to fix it. This influence approach shall also work for microshift-ovs-init service script, which sets up the nodeIP interface for ovn-kubernetes CNI plugin.
There was a problem hiding this comment.
In which scenario would a user want to pick a specific IP? The IP to which core components of microshift bind to shouldn't really matter to the end-user since their applications will most likely only communicate via the different services.
There was a problem hiding this comment.
I remember that some popular distros also supported specifying the interface name or something like IP(eth0) to make it work with DHCP too
There was a problem hiding this comment.
If I am not mistaken, the nodeIP is not even configurable in OCP today. The closest thing I could find is this effort: https://github.com/openshift/enhancements/blob/ef85659d01738b9f89958d5f0da31cff05bb1182/enhancements/network/ip-interface-selection.md but this seem way too complex for MicroShift to need something like that right now.
I remember that some popular distros also supported specifying the interface name or something like IP(eth0) to make it work with DHCP too
I am not sure why that would be beneficial to do for control plane components.
Another concern I have is what would happen if microshift was first started with a nodeIP, but then a user decided to change it on the fly. Everything that used to depend on the previous nodeIP will not be able to reach the new nodeIP if we don't have a way to propagate the update. I don't think that's supported in k8s in general since one would most likely have to delete the node to update the IP on the various components.
There was a problem hiding this comment.
Unlike OCP hosts, we have to deal with the fact that a host running MicroShift may change its IP or move between networks. So we either need a way to pick a node IP that will never change, or we need to support it being updated. Could we always use a localhost IP or the hardware's IPv6 local address, for example? What effect would that have? What needs to use the node IP?
|
@dinhxuanvu: PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-merge-robot
added
the
needs-rebase
| var microshiftDataDir = config.GetDataDir() | ||
| var ( | ||
| microshiftDataDir = config.GetDataDir() | ||
| nodeIP = config.GetNodeIP() |
There was a problem hiding this comment.
instead of defining that globally every time, could we perhaps create a structure that could be passed on to the various init functions alongside the configuration?
| } | ||
|
|
||
| func findNodeIP() string { | ||
| hostIP, err := util.GetHostIP() |
There was a problem hiding this comment.
In which scenario would a user want to pick a specific IP? The IP to which core components of microshift bind to shouldn't really matter to the end-user since their applications will most likely only communicate via the different services.
|
I believe NodeIP is important to let the services know which specific IP we will be listening to if it's not the one on default route. Or if we want it to listen to 0.0.0.0 ? Also bear in mind that IPv6 values should be accepted here. |
|
It looks like the opinion here is that we will need to keep this NodeIP option configurable given the possibility of multiple IP options and/or changes. That sounds fair. I would recommend folks to comment on #1030 PR on where to keep this option in the new config. Thanks. |
6 participants
The NodeIP is always the host IP.
Signed-off-by: Vu Dinh vudinh@outlook.com
Which issue(s) this PR addresses:
Closes #