From e18ee67cec3ed0ea2b77d2d0569a0589abd7676d Mon Sep 17 00:00:00 2001 From: Adil Fulara Date: Tue, 13 Aug 2024 17:23:43 -0700 Subject: [PATCH] MESH-5373: Node locality is missing occasionally. - Add support for update event on NodeController Signed-off-by: Adil Fulara --- admiral/pkg/controller/admiral/node.go | 22 +++++++++++++------ admiral/pkg/controller/admiral/node_test.go | 24 +++++++-------------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/admiral/pkg/controller/admiral/node.go b/admiral/pkg/controller/admiral/node.go index a19a43a7..0d261ce6 100644 --- a/admiral/pkg/controller/admiral/node.go +++ b/admiral/pkg/controller/admiral/node.go @@ -57,19 +57,29 @@ func NewNodeController(stopCh <-chan struct{}, handler NodeHandler, config *rest return &nodeController, nil } -func (p *NodeController) Added(ctx context.Context, obj interface{}) error { +func process(ctx context.Context, obj interface{}) (string, error) { node, ok := obj.(*k8sV1.Node) if !ok { - return fmt.Errorf("type assertion failed, %v is not of type *v1.Node", obj) + return "", fmt.Errorf("type assertion failed, %v is not of type *v1.Node", obj) } - if p.Locality == nil { - p.Locality = &Locality{Region: common.GetNodeLocality(node)} + return common.GetNodeLocality(node), nil +} + +func (d *NodeController) Added(ctx context.Context, obj interface{}) error { + region, err := process(ctx, obj) + if err != nil { + return err } + d.Locality = &Locality{Region: region} return nil } -func (p *NodeController) Updated(ctx context.Context, obj interface{}, oldObj interface{}) error { - //ignore +func (d *NodeController) Updated(ctx context.Context, obj interface{}, oldObj interface{}) error { + region, err := process(ctx, obj) + if err != nil { + return err + } + d.Locality = &Locality{Region: region} return nil } diff --git a/admiral/pkg/controller/admiral/node_test.go b/admiral/pkg/controller/admiral/node_test.go index 502b3a7b..2048b581 100644 --- a/admiral/pkg/controller/admiral/node_test.go +++ b/admiral/pkg/controller/admiral/node_test.go @@ -102,31 +102,23 @@ func TestNodeAddUpdateDelete(t *testing.T) { if nodeController == nil { t.Errorf("Node controller should never be nil without an error thrown") + return } region := "us-west-2" nodeObj := &k8sV1.Node{Spec: k8sV1.NodeSpec{}, ObjectMeta: v1.ObjectMeta{Labels: map[string]string{common.NodeRegionLabel: region}}} ctx := context.Background() - nodeController.Added(ctx, nodeObj) + _ = nodeController.Added(ctx, nodeObj) + assert.Equal(t, "us-west-2", nodeController.Locality.Region, "region expected %v, got: %v", region, nodeController.Locality.Region) - locality := nodeController.Locality + nodeObj.Labels[common.NodeRegionLabel] = "us-east-2" + _ = nodeController.Updated(ctx, nodeObj, nodeObj) + assert.Equal(t, "us-east-2", nodeController.Locality.Region, "region expected %v, got: %v", region, nodeController.Locality.Region) - if locality.Region != region { - t.Errorf("region expected %v, got: %v", region, locality.Region) - } - - nodeController.Updated(ctx, nodeObj, nodeObj) - //update should make no difference - if locality.Region != region { - t.Errorf("region expected %v, got: %v", region, locality.Region) - } - - nodeController.Deleted(ctx, nodeObj) + _ = nodeController.Deleted(ctx, nodeObj) //delete should make no difference - if locality.Region != region { - t.Errorf("region expected %v, got: %v", region, locality.Region) - } + assert.Equal(t, "us-east-2", nodeController.Locality.Region, "region expected %v, got: %v", region, nodeController.Locality.Region) } // TODO: This is just a placeholder for when we add diff check for other types