Skip to content

Commit 0de00bd

Browse files
tanishagoyal2tanishagoyal2
committed
chore: review changes
Signed-off-by: Tanisha goyal <[email protected]>
1 parent 65d6b49 commit 0de00bd

File tree

4 files changed

+33
-169
lines changed

4 files changed

+33
-169
lines changed

‎health-monitors/csp-health-monitor/pkg/csp/aws/aws.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,11 @@ func NewClient(
164164

165165
slog.Info("AWS Client: Kubernetes clientset initialized successfully.")
166166

167-
nodeInformer := NewNodeInformer(k8sClient)
167+
nodeInformer, err := NewNodeInformer(k8sClient)
168+
if err != nil {
169+
return nil, fmt.Errorf("failed to create node informer: %w", err)
170+
}
171+
168172
nodeInformer.Start(ctx)
169173

170174
slog.Info("AWS Client: Node informer started successfully")

‎health-monitors/csp-health-monitor/pkg/csp/aws/aws_test.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func createTestClient(t *testing.T) (*AWSClient, *MockAWSHealthClient, *fake.Cli
101101

102102
nodeInformer := &NodeInformer{
103103
k8sClient: fakeK8sClient,
104-
instanceIDs: map[string]string{
104+
nodeNameToInstanceIDMap: map[string]string{
105105
testInstanceID: testNodeName,
106106
},
107107
}
@@ -307,7 +307,7 @@ func TestMultipleAffectedEntities(t *testing.T) {
307307
// Setup test channel and instance IDs
308308
eventChan := make(chan model.MaintenanceEvent, 10)
309309

310-
client.nodeInformer.instanceIDs = map[string]string{
310+
client.nodeInformer.nodeNameToInstanceIDMap = map[string]string{
311311
testInstanceID: testNodeName,
312312
testInstanceID1: testNodeName1,
313313
testInstanceID2: testNodeName2,
@@ -430,7 +430,7 @@ func TestCompletedEvent(t *testing.T) {
430430

431431
// Setup test channel and instance IDs
432432
eventChan := make(chan model.MaintenanceEvent, 10)
433-
client.nodeInformer.instanceIDs = map[string]string{
433+
client.nodeInformer.nodeNameToInstanceIDMap = map[string]string{
434434
testInstanceID: testNodeName,
435435
testInstanceID1: testNodeName1,
436436
testInstanceID2: testNodeName2,
@@ -479,7 +479,7 @@ func TestErrorScenario(t *testing.T) {
479479

480480
// Setup test channel and test instance IDs
481481
eventChan := make(chan model.MaintenanceEvent, 10)
482-
client.nodeInformer.instanceIDs = map[string]string{
482+
client.nodeInformer.nodeNameToInstanceIDMap = map[string]string{
483483
testInstanceID: testNodeName,
484484
}
485485

@@ -523,7 +523,7 @@ func TestTimeWindowFiltering(t *testing.T) {
523523

524524
// Setup test channel and test instance IDs
525525
eventChan := make(chan model.MaintenanceEvent, 10)
526-
client.nodeInformer.instanceIDs = map[string]string{
526+
client.nodeInformer.nodeNameToInstanceIDMap = map[string]string{
527527
testInstanceID: testNodeName,
528528
}
529529

@@ -611,7 +611,7 @@ func TestInstanceFiltering(t *testing.T) {
611611

612612
// Setup test channel with our cluster's instance IDs only
613613
eventChan := make(chan model.MaintenanceEvent, 10)
614-
client.nodeInformer.instanceIDs = map[string]string{
614+
client.nodeInformer.nodeNameToInstanceIDMap = map[string]string{
615615
testInstanceID: testNodeName,
616616
}
617617

@@ -712,7 +712,7 @@ func TestInvalidEntityData(t *testing.T) {
712712

713713
// Setup test channel and test instance IDs
714714
eventChan := make(chan model.MaintenanceEvent, 10)
715-
client.nodeInformer.instanceIDs = map[string]string{
715+
client.nodeInformer.nodeNameToInstanceIDMap = map[string]string{
716716
testInstanceID: testNodeName,
717717
}
718718

@@ -789,7 +789,7 @@ func TestInstanceRebootEvent(t *testing.T) {
789789
}, nil)
790790
// Setup test channel and test instance IDs
791791
eventChan := make(chan model.MaintenanceEvent, 10)
792-
client.nodeInformer.instanceIDs = map[string]string{
792+
client.nodeInformer.nodeNameToInstanceIDMap = map[string]string{
793793
testInstanceID: testNodeName,
794794
}
795795

@@ -887,7 +887,7 @@ func TestIgnoredEventTypes(t *testing.T) {
887887
}, nil)
888888

889889
eventChan := make(chan model.MaintenanceEvent, 10)
890-
client.nodeInformer.instanceIDs = map[string]string{
890+
client.nodeInformer.nodeNameToInstanceIDMap = map[string]string{
891891
testInstanceID: testNodeName,
892892
testInstanceIDIgnored: testNodeNameIgnored,
893893
}

‎health-monitors/csp-health-monitor/pkg/csp/aws/informer.go‎

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package aws
1616

1717
import (
1818
"context"
19+
"fmt"
1920
"log/slog"
2021
"strings"
2122
"sync"
@@ -29,22 +30,21 @@ import (
2930
// NodeInformer watches Kubernetes nodes and maintains an up-to-date mapping
3031
// of AWS instance IDs to node names
3132
type NodeInformer struct {
32-
k8sClient kubernetes.Interface
33-
informer cache.SharedIndexInformer
34-
stopCh chan struct{}
35-
instanceIDs map[string]string
36-
mu sync.RWMutex
37-
stopOnce sync.Once
33+
k8sClient kubernetes.Interface
34+
informer cache.SharedIndexInformer
35+
stopCh chan struct{}
36+
nodeNameToInstanceIDMap map[string]string
37+
mu sync.RWMutex
38+
stopOnce sync.Once
3839
}
3940

40-
func NewNodeInformer(k8sClient kubernetes.Interface) *NodeInformer {
41+
func NewNodeInformer(k8sClient kubernetes.Interface) (*NodeInformer, error) {
4142
ni := &NodeInformer{
42-
k8sClient: k8sClient,
43-
stopCh: make(chan struct{}),
44-
instanceIDs: make(map[string]string),
43+
k8sClient: k8sClient,
44+
stopCh: make(chan struct{}),
45+
nodeNameToInstanceIDMap: make(map[string]string),
4546
}
4647

47-
// Use informer factory which works with both real and fake clients
4848
factory := informers.NewSharedInformerFactory(k8sClient, 0)
4949
informer := factory.Core().V1().Nodes().Informer()
5050

@@ -60,11 +60,12 @@ func NewNodeInformer(k8sClient kubernetes.Interface) *NodeInformer {
6060
})
6161
if err != nil {
6262
slog.Error("Failed to add event handlers to informer", "error", err)
63+
return nil, fmt.Errorf("failed to add event handlers to informer: %w", err)
6364
}
6465

6566
ni.informer = informer
6667

67-
return ni
68+
return ni, nil
6869
}
6970

7071
func (ni *NodeInformer) Start(ctx context.Context) {
@@ -77,13 +78,7 @@ func (ni *NodeInformer) Start(ctx context.Context) {
7778
return
7879
}
7980

80-
slog.Info("AWS node informer cache synced successfully", "nodesMap", ni.instanceIDs)
81-
82-
// Log the initial node count
83-
ni.mu.RLock()
84-
count := len(ni.instanceIDs)
85-
ni.mu.RUnlock()
86-
slog.Info("AWS node informer ready", "nodeCount", count)
81+
slog.Info("AWS node informer cache synced successfully", "nodesMap", ni.nodeNameToInstanceIDMap)
8782

8883
go func() {
8984
<-ctx.Done()
@@ -103,8 +98,8 @@ func (ni *NodeInformer) GetInstanceIDs() map[string]string {
10398
defer ni.mu.RUnlock()
10499

105100
// Return a copy to avoid concurrent access issues
106-
instanceIDsCopy := make(map[string]string, len(ni.instanceIDs))
107-
for k, v := range ni.instanceIDs {
101+
instanceIDsCopy := make(map[string]string, len(ni.nodeNameToInstanceIDMap))
102+
for k, v := range ni.nodeNameToInstanceIDMap {
108103
instanceIDsCopy[k] = v
109104
}
110105

@@ -115,7 +110,7 @@ func (ni *NodeInformer) GetNodeName(instanceID string) (string, bool) {
115110
ni.mu.RLock()
116111
defer ni.mu.RUnlock()
117112

118-
nodeName, ok := ni.instanceIDs[instanceID]
113+
nodeName, ok := ni.nodeNameToInstanceIDMap[instanceID]
119114

120115
return nodeName, ok
121116
}
@@ -127,7 +122,7 @@ func (ni *NodeInformer) handleNodeAdd(node *v1.Node) {
127122
}
128123

129124
ni.mu.Lock()
130-
ni.instanceIDs[instanceID] = node.Name
125+
ni.nodeNameToInstanceIDMap[instanceID] = node.Name
131126
ni.mu.Unlock()
132127

133128
slog.Info("Node added to AWS instance map",
@@ -142,7 +137,7 @@ func (ni *NodeInformer) handleNodeDelete(node *v1.Node) {
142137
}
143138

144139
ni.mu.Lock()
145-
delete(ni.instanceIDs, instanceID)
140+
delete(ni.nodeNameToInstanceIDMap, instanceID)
146141
ni.mu.Unlock()
147142

148143
slog.Info("Node removed from AWS instance map",

‎health-monitors/csp-health-monitor/pkg/csp/aws/informer_test.go‎

Lines changed: 0 additions & 135 deletions
This file was deleted.

0 commit comments

Comments
 (0)