github-personal/kubernetes
1
0
Fork 0
mirror of https://github.com/outbackdingo/kubernetes.git synced 2026-01-27 10:19:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

kube-proxy: merge OnNodeAdd and OnNodeUpdate into OnNodeChange

Browse Source 
For kube-proxy, node addition and node update is semantically
considered as similar event, we have exactly same handler
logic for these two events resulting in duplicate code and
unit tests.
This merges the `NodeHandler` interface methods OnNodeAdd and
OnNodeUpdate into OnNodeChange along with the implementation
of the interface.

Signed-off-by: Daman Arora <aroradaman@gmail.com>
This commit is contained in:
Daman Arora
2025-05-11 22:48:25 +05:30
committed by Dan Winship
parent 0dc51b16f9
commit e2d37f3cd7
5 changed files with 21 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
pkg/proxy/config/config.go
View File

@@ -260,12 +260,9 @@ func (c *ServiceConfig) handleDeleteService(obj interface{}) {
// NodeHandler is an abstract interface of objects which receive
// notifications about node object changes.
type NodeHandler interface {
// OnNodeAdd is called whenever creation of new node object
// is observed.
OnNodeAdd(node *v1.Node)
// OnNodeUpdate is called whenever modification of an existing
// node object is observed.
OnNodeUpdate(oldNode, node *v1.Node)
// OnNodeChange is called whenever creation or modification
// of node object is observed.
OnNodeChange(node *v1.Node)
// OnNodeDelete is called whenever deletion of an existing node
// object is observed.
OnNodeDelete(node *v1.Node)
@@ -290,8 +287,8 @@ func NewNodeConfig(ctx context.Context, nodeInformer v1informers.NodeInformer, r
handlerRegistration, _ := nodeInformer.Informer().AddEventHandlerWithResyncPeriod(
cache.ResourceEventHandlerFuncs{
AddFunc: result.handleAddNode,
UpdateFunc: result.handleUpdateNode,
AddFunc: func(obj interface{}) { result.handleChangeNode(obj) },
UpdateFunc: func(_, newObj interface{}) { result.handleChangeNode(newObj) },
DeleteFunc: result.handleDeleteNode,
},
resyncPeriod,
@@ -321,32 +318,15 @@ func (c *NodeConfig) Run(stopCh <-chan struct{}) {
}
}
func (c *NodeConfig) handleAddNode(obj interface{}) {
func (c *NodeConfig) handleChangeNode(obj interface{}) {
node, ok := obj.(*v1.Node)
if !ok {
utilruntime.HandleError(fmt.Errorf("unexpected object type: %v", obj))
return
}
for i := range c.eventHandlers {
c.logger.V(4).Info("Calling handler.OnNodeAdd")
c.eventHandlers[i].OnNodeAdd(node)
}
}
func (c *NodeConfig) handleUpdateNode(oldObj, newObj interface{}) {
oldNode, ok := oldObj.(*v1.Node)
if !ok {
utilruntime.HandleError(fmt.Errorf("unexpected object type: %v", oldObj))
return
}
node, ok := newObj.(*v1.Node)
if !ok {
utilruntime.HandleError(fmt.Errorf("unexpected object type: %v", newObj))
return
}
for i := range c.eventHandlers {
c.logger.V(5).Info("Calling handler.OnNodeUpdate")
c.eventHandlers[i].OnNodeUpdate(oldNode, node)
c.logger.V(4).Info("Calling handler.OnNodeChange")
c.eventHandlers[i].OnNodeChange(node)
}
}

9
pkg/proxy/config/config_test.go
View File

@@ -454,14 +454,7 @@ func NewNodeHandlerMock() *NodeHandlerMock {
return h
}
func (h *NodeHandlerMock) OnNodeAdd(node *v1.Node) {
h.lock.Lock()
defer h.lock.Unlock()
h.state[node.Name] = node
h.sendNodes()
}
func (h *NodeHandlerMock) OnNodeUpdate(oldNode, node *v1.Node) {
func (h *NodeHandlerMock) OnNodeChange(node *v1.Node) {
h.lock.Lock()
defer h.lock.Unlock()
h.state[node.Name] = node

16
pkg/proxy/healthcheck/healthcheck_test.go
View File

@@ -495,7 +495,7 @@ func TestHealthzServer(t *testing.T) {
testProxyHealthUpdater(hs, hsTest, fakeClock, ptr.To(true), t)
// Should return 200 "OK" if we've synced a node, tainted in any other way
nodeManager.OnNodeUpdate(nil, makeNode(tweakTainted("other")))
nodeManager.OnNodeChange(makeNode(tweakTainted("other")))
expectedPayload = ProxyHealth{
CurrentTime: fakeClock.Now(),
LastUpdated: fakeClock.Now(),
@@ -509,7 +509,7 @@ func TestHealthzServer(t *testing.T) {
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
// Should return 503 "ServiceUnavailable" if we've synced a ToBeDeletedTaint node
nodeManager.OnNodeUpdate(nil, makeNode(tweakTainted(ToBeDeletedTaint)))
nodeManager.OnNodeChange(makeNode(tweakTainted(ToBeDeletedTaint)))
expectedPayload = ProxyHealth{
CurrentTime: fakeClock.Now(),
LastUpdated: fakeClock.Now(),
@@ -523,7 +523,7 @@ func TestHealthzServer(t *testing.T) {
testHTTPHandler(hsTest, http.StatusServiceUnavailable, expectedPayload, t)
// Should return 200 "OK" if we've synced a node, tainted in any other way
nodeManager.OnNodeUpdate(nil, makeNode(tweakTainted("other")))
nodeManager.OnNodeChange(makeNode(tweakTainted("other")))
expectedPayload = ProxyHealth{
CurrentTime: fakeClock.Now(),
LastUpdated: fakeClock.Now(),
@@ -537,7 +537,7 @@ func TestHealthzServer(t *testing.T) {
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
// Should return 503 "ServiceUnavailable" if we've synced a deleted node
nodeManager.OnNodeUpdate(nil, makeNode(tweakDeleted()))
nodeManager.OnNodeChange(makeNode(tweakDeleted()))
expectedPayload = ProxyHealth{
CurrentTime: fakeClock.Now(),
LastUpdated: fakeClock.Now(),
@@ -575,7 +575,7 @@ func TestLivezServer(t *testing.T) {
testProxyHealthUpdater(hs, hsTest, fakeClock, nil, t)
// Should return 200 "OK" irrespective of node syncs
nodeManager.OnNodeUpdate(nil, makeNode(tweakTainted("other")))
nodeManager.OnNodeChange(makeNode(tweakTainted("other")))
expectedPayload = ProxyHealth{
CurrentTime: fakeClock.Now(),
LastUpdated: fakeClock.Now(),
@@ -588,7 +588,7 @@ func TestLivezServer(t *testing.T) {
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
// Should return 200 "OK" irrespective of node syncs
nodeManager.OnNodeUpdate(nil, makeNode(tweakTainted(ToBeDeletedTaint)))
nodeManager.OnNodeChange(makeNode(tweakTainted(ToBeDeletedTaint)))
expectedPayload = ProxyHealth{
CurrentTime: fakeClock.Now(),
LastUpdated: fakeClock.Now(),
@@ -601,7 +601,7 @@ func TestLivezServer(t *testing.T) {
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
// Should return 200 "OK" irrespective of node syncs
nodeManager.OnNodeUpdate(nil, makeNode(tweakTainted("other")))
nodeManager.OnNodeChange(makeNode(tweakTainted("other")))
expectedPayload = ProxyHealth{
CurrentTime: fakeClock.Now(),
LastUpdated: fakeClock.Now(),
@@ -614,7 +614,7 @@ func TestLivezServer(t *testing.T) {
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
// Should return 200 "OK" irrespective of node syncs
nodeManager.OnNodeUpdate(nil, makeNode(tweakDeleted()))
nodeManager.OnNodeChange(makeNode(tweakDeleted()))
expectedPayload = ProxyHealth{
CurrentTime: fakeClock.Now(),
LastUpdated: fakeClock.Now(),

14
pkg/proxy/node.go
View File

@@ -171,18 +171,8 @@ func (n *NodeManager) NodeInformer() v1informers.NodeInformer {
return n.nodeInformer
}
// OnNodeAdd is a handler for Node creates.
func (n *NodeManager) OnNodeAdd(node *v1.Node) {
n.onNodeChange(node)
}
// OnNodeUpdate is a handler for Node updates.
func (n *NodeManager) OnNodeUpdate(_, node *v1.Node) {
n.onNodeChange(node)
}
// onNodeChange functions helps to implement OnNodeAdd and OnNodeUpdate.
func (n *NodeManager) onNodeChange(node *v1.Node) {
// OnNodeChange is a handler for Node creation and update.
func (n *NodeManager) OnNodeChange(node *v1.Node) {
// update the node object
n.mu.Lock()
n.node = node

4
pkg/proxy/node_test.go
View File

@@ -294,7 +294,7 @@ func TestNodeManagerOnNodeChange(t *testing.T) {
nodeManager, err := newNodeManager(ctx, client, 30*time.Second, testNodeName, tc.watchPodCIDRs, exitFunc, 10*time.Millisecond, time.Second, time.Second)
require.NoError(t, err)
nodeManager.onNodeChange(makeNode(tweakNodeIPs(tc.updatedNodeIPs...), tweakPodCIDRs(tc.updatedPodCIDRs...)))
nodeManager.OnNodeChange(makeNode(tweakNodeIPs(tc.updatedNodeIPs...), tweakPodCIDRs(tc.updatedPodCIDRs...)))
require.Equal(t, tc.expectedExitCode, exitCode)
})
}
@@ -328,7 +328,7 @@ func TestNodeManagerNode(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "1", nodeManager.Node().ResourceVersion)
nodeManager.OnNodeUpdate(nil, makeNode(tweakResourceVersion("2")))
nodeManager.OnNodeChange(makeNode(tweakResourceVersion("2")))
require.NoError(t, err)
require.Equal(t, "2", nodeManager.Node().ResourceVersion)
}