Refactor: Abstract Node Info #1739
Conversation
This prepares the way for broader refactors in the way that we handle nodes by: * Separating frequently used node logic from the controller creation steps * Keeping reused code DRY-er * Adding interface abstractions for key groups of node data and starting to rely on those more rather than concrete types * Separating node data from the rest of the controller data structure so that it smaller definitions of data can be passed around to functions that need it rather than always passing the entire controller which contains more data / surface area than most functions need.
|
Refactors are always a bit of a slog to review. But the gist of this one is that instead of storing all of the physical node data as individual data sets like The ultimate goal of this refactor is to stop the need of constantly either attaching functions to the main controllers ( Right now, my mid-term goal is to refactor injectRoute() in order to resolve #1738 in a way that doesn't perpetrate bad coding practices. But eventually I think that more abstractions are going to be needed like this as there is quite a bit of cruft in the code base. |
pkg/utils/node.go
Outdated
|
|
||
| // GetNodeIPv4AddrsTypeMap returns the node's IPv4 addresses grouped by Kubernetes Node Object address type (internal / | ||
| // external). | ||
| func (n *KRNode) GetNodeIPv4AddrsTypeMap() addressMap { |
There was a problem hiding this comment.
AFAICT this method and its sibling GetNodeIPv4AddrsTypeMap aren't used anywhere. Maybe remove them until they're actually needed?
There was a problem hiding this comment.
Good catch here as well. It used to be used before I exposed other better functions, then I didn't realize that they were no longer used. I'll remove them.
pkg/utils/node.go
Outdated
| NodeIPv6Addrs addressMap | ||
| NodeName string | ||
| linkQ LocalLinkQuerier | ||
| PrimaryNodeSubnet net.IPNet |
There was a problem hiding this comment.
I think I prematurely added this to the interface because of the usage of utils.GetNodeSubnet(nodeIP, nil) in network_routes_controller.go.
You're right, it isn't used anywhere, I'll remove it.
pkg/utils/node.go
Outdated
|
|
||
| // GetNodeIPv4AddrsTypeMap returns the node's IPv4 addresses grouped by Kubernetes Node Object address type (internal / | ||
| // external). | ||
| func (n *KRNode) GetNodeIPv4AddrsTypeMap() addressMap { |
There was a problem hiding this comment.
Good catch here as well. It used to be used before I exposed other better functions, then I didn't realize that they were no longer used. I'll remove them.
Co-authored-by: Tom Wieczorek <twz123@users.noreply.github.com>
aauren
force-pushed
the
fact/abstract_node_info
branch
from
883d2b4
to
f4766d6
Compare
aauren
force-pushed
the
fact/abstract_node_info
branch
from
69ed717
to
9e9fb73
Compare
@jnummelin @rbrtbnfgl @mrueg
This prepares the way for broader refactors in the way that we handle nodes by:
Eventually the changes here should better support the work that will need to happen to fix #1738 #676