Skip to content

Commit 12d0643

Browse files
use const for labels, include timeout and fix race condition
1 parent 41745ec commit 12d0643

File tree

3 files changed

+22
-20
lines changed

3 files changed

+22
-20
lines changed

internal/service/alarms/internal/alertmanager/converter.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
api "github.com/openshift-kni/oran-o2ims/internal/service/alarms/api/generated"
1717
"github.com/openshift-kni/oran-o2ims/internal/service/alarms/internal/db/models"
1818
"github.com/openshift-kni/oran-o2ims/internal/service/alarms/internal/infrastructure"
19+
"github.com/openshift-kni/oran-o2ims/internal/service/common"
1920
)
2021

2122
// ConvertAmToAlarmEventRecordModels get alarmEventRecords based on the alertmanager notification and AlarmDefinition
@@ -133,9 +134,9 @@ func getClusterID(labels map[string]string) *uuid.UUID {
133134

134135
// isHardwareAlert checks if an alert is a hardware alert by checking for type=hardware AND component=ironic labels
135136
func isHardwareAlert(labels map[string]string) bool {
136-
alertType, hasType := labels["type"]
137-
component, hasComponent := labels["component"]
138-
return hasType && alertType == "hardware" && hasComponent && component == "ironic"
137+
alertType, hasType := labels[common.HardwareAlertTypeLabel]
138+
component, hasComponent := labels[common.HardwareAlertComponentLabel]
139+
return hasType && alertType == common.HardwareAlertTypeValue && hasComponent && component == common.HardwareAlertComponentValue
139140
}
140141

141142
// getResourceID extracts resource ID from instance_uuid label for hardware alerts

internal/service/alarms/internal/infrastructure/resourceserver.go

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,11 @@ func (r *ResourceServer) FetchAll(ctx context.Context) error {
126126
slog.Info("Mapping resource type ID to alarm definitions", "resourceTypeID", resourceType.ResourceTypeId, "alarmDictionaryID", resp.JSON200.AlarmDictionaryId, "definitionCount", len(alarmDefinitions))
127127
}
128128

129-
// Atomically update the map while holding lock
129+
// Atomically update the maps while holding lock
130130
r.Lock()
131131
r.resourceTypeIDToAlarmDefinitions = newResourceTypeIDToAlarmDefinitions
132+
// Clear the resource ID cache to prevent memory leak from deleted/stale resources
133+
r.resourceIDToResourceTypeID = make(map[uuid.UUID]uuid.UUID)
132134
r.Unlock()
133135

134136
slog.Info("Successfully synced ResourceServer objects")
@@ -139,9 +141,9 @@ func (r *ResourceServer) FetchAll(ctx context.Context) error {
139141
// It uses the cache if available, otherwise fetches from resource server
140142
func (r *ResourceServer) GetObjectTypeID(ctx context.Context, resourceID uuid.UUID) (uuid.UUID, error) {
141143
r.Lock()
142-
resourceTypeID, ok := r.resourceIDToResourceTypeID[resourceID]
143-
r.Unlock()
144+
defer r.Unlock()
144145

146+
resourceTypeID, ok := r.resourceIDToResourceTypeID[resourceID]
145147
if ok {
146148
return resourceTypeID, nil
147149
}
@@ -154,9 +156,8 @@ func (r *ResourceServer) GetObjectTypeID(ctx context.Context, resourceID uuid.UU
154156
}
155157

156158
// Cache the mapping
157-
r.Lock()
158159
r.resourceIDToResourceTypeID[resourceID] = resource.ResourceTypeId
159-
r.Unlock()
160+
slog.Info("Mapping resource ID to resource type ID", "resourceID", resourceID, "resourceTypeID", resource.ResourceTypeId)
160161

161162
return resource.ResourceTypeId, nil
162163
}
@@ -165,9 +166,9 @@ func (r *ResourceServer) GetObjectTypeID(ctx context.Context, resourceID uuid.UU
165166
// It uses the cache if available, otherwise it fetches the data from the server
166167
func (r *ResourceServer) GetAlarmDefinitionID(ctx context.Context, resourceTypeID uuid.UUID, name, severity string) (uuid.UUID, error) {
167168
r.Lock()
168-
alarmDefinitions, ok := r.resourceTypeIDToAlarmDefinitions[resourceTypeID]
169-
r.Unlock()
169+
defer r.Unlock()
170170

171+
alarmDefinitions, ok := r.resourceTypeIDToAlarmDefinitions[resourceTypeID]
171172
if !ok {
172173
slog.Info("Resource type ID not found in cache, fetching from resource server", "resourceTypeID", resourceTypeID)
173174

@@ -189,10 +190,7 @@ func (r *ResourceServer) GetAlarmDefinitionID(ctx context.Context, resourceTypeI
189190
alarmDefinitions = buildAlarmDefinitionsFromDictionary(*resp.JSON200)
190191

191192
// Cache the alarm definitions
192-
r.Lock()
193193
r.resourceTypeIDToAlarmDefinitions[resourceTypeID] = alarmDefinitions
194-
r.Unlock()
195-
196194
slog.Info("Mapping resource type ID to alarm definitions", "resourceTypeID", resourceTypeID, "alarmDictionaryID", resp.JSON200.AlarmDictionaryId, "definitionCount", len(alarmDefinitions))
197195
}
198196

@@ -258,7 +256,10 @@ func (r *ResourceServer) Sync(ctx context.Context) {
258256

259257
// getResourceTypes fetches all resource types from the resource server
260258
func (r *ResourceServer) getResourceTypes(ctx context.Context) ([]generated.ResourceType, error) {
261-
resp, err := r.client.GetResourceTypesWithResponse(ctx, &generated.GetResourceTypesParams{})
259+
ctxWithTimeout, cancel := context.WithTimeout(ctx, clients.ListRequestTimeout)
260+
defer cancel()
261+
262+
resp, err := r.client.GetResourceTypesWithResponse(ctxWithTimeout, &generated.GetResourceTypesParams{})
262263
if err != nil {
263264
return nil, fmt.Errorf("failed to get resource types: %w", err)
264265
}
@@ -271,12 +272,16 @@ func (r *ResourceServer) getResourceTypes(ctx context.Context) ([]generated.Reso
271272
return nil, fmt.Errorf("empty response from resource server")
272273
}
273274

275+
slog.Info("Got resource types", "count", len(*resp.JSON200))
274276
return *resp.JSON200, nil
275277
}
276278

277279
// getResource fetches a resource by ID from the resource server
278280
func (r *ResourceServer) getResource(ctx context.Context, resourceID uuid.UUID) (*generated.Resource, error) {
279-
resp, err := r.client.GetInternalResourceByIdWithResponse(ctx, resourceID)
281+
ctxWithTimeout, cancel := context.WithTimeout(ctx, clients.SingleRequestTimeout)
282+
defer cancel()
283+
284+
resp, err := r.client.GetInternalResourceByIdWithResponse(ctxWithTimeout, resourceID)
280285
if err != nil {
281286
return nil, fmt.Errorf("failed to get resource: %w", err)
282287
}
@@ -289,6 +294,7 @@ func (r *ResourceServer) getResource(ctx context.Context, resourceID uuid.UUID)
289294
return nil, fmt.Errorf("empty response from resource server")
290295
}
291296

297+
slog.Info("Got resource", "resourceID", resourceID)
292298
return resp.JSON200, nil
293299
}
294300

internal/service/resources/db/repo/repository.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,6 @@ func (r *ResourcesRepository) GetResource(ctx context.Context, id uuid.UUID) (*m
9292
return svcutils.Find[models.Resource](ctx, r.Db, id)
9393
}
9494

95-
// GetResources retrieves all Resource tuples or returns an empty array if no tuples are found
96-
func (r *ResourcesRepository) GetResources(ctx context.Context) ([]models.Resource, error) {
97-
return svcutils.FindAll[models.Resource](ctx, r.Db)
98-
}
99-
10095
// CreateResource creates a new Resource tuple
10196
func (r *ResourcesRepository) CreateResource(ctx context.Context, resource *models.Resource) (*models.Resource, error) {
10297
return svcutils.Create[models.Resource](ctx, r.Db, *resource)

0 commit comments

Comments
 (0)