Skip to content

Commit 44d3344

Browse files
committed
refactor: require stored references for CR status
Signed-off-by: Alex Jun <aljun@nvidia.com>
1 parent b818d92 commit 44d3344

5 files changed

Lines changed: 8 additions & 69 deletions

File tree

fault-remediation/pkg/crstatus/checker.go

Lines changed: 4 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,11 @@ import (
2323
"sigs.k8s.io/controller-runtime/pkg/client"
2424

2525
"github.com/nvidia/nvsentinel/fault-remediation/pkg/annotation"
26-
"github.com/nvidia/nvsentinel/fault-remediation/pkg/config"
2726
)
2827

2928
type CRStatusChecker struct {
30-
client client.Client
31-
remediationActions map[string]config.MaintenanceResource
32-
dryRun bool
29+
client client.Client
30+
dryRun bool
3331
}
3432

3533
type CRState string
@@ -43,39 +41,14 @@ const (
4341

4442
func NewCRStatusChecker(
4543
client client.Client,
46-
remediationActions map[string]config.MaintenanceResource,
4744
dryRun bool,
4845
) *CRStatusChecker {
4946
return &CRStatusChecker{
50-
client: client,
51-
remediationActions: remediationActions,
52-
dryRun: dryRun,
47+
client: client,
48+
dryRun: dryRun,
5349
}
5450
}
5551

56-
// ShouldSkipCRCreation returns true if an existing CR should suppress creation of a new CR.
57-
func (c *CRStatusChecker) ShouldSkipCRCreation(ctx context.Context, actionName string, crName string) bool {
58-
state := c.GetCRState(ctx, actionName, crName)
59-
return state == CRStateInProgress || state == CRStateSucceeded
60-
}
61-
62-
func (c *CRStatusChecker) GetCRState(ctx context.Context, actionName string, crName string) CRState {
63-
resource, exists := c.remediationActions[actionName]
64-
if !exists {
65-
slog.ErrorContext(ctx, "No remediation configuration found for action", "action", actionName)
66-
return CRStateNotFound
67-
}
68-
69-
resourceRef := annotation.MaintenanceResourceReference{
70-
Namespace: resource.Namespace,
71-
Version: resource.Version,
72-
ApiGroup: resource.ApiGroup,
73-
Kind: resource.Kind,
74-
}
75-
76-
return c.GetCRStateForReference(ctx, crName, resourceRef, resource.CompleteConditionType)
77-
}
78-
7952
func (c *CRStatusChecker) GetCRStateForReference(
8053
ctx context.Context,
8154
crName string,
@@ -110,10 +83,6 @@ func (c *CRStatusChecker) GetCRStateForReference(
11083
return c.checkConditionType(obj, completeConditionType)
11184
}
11285

113-
func (c *CRStatusChecker) checkCondition(obj *unstructured.Unstructured, resource config.MaintenanceResource) CRState {
114-
return c.checkConditionType(obj, resource.CompleteConditionType)
115-
}
116-
11786
func (c *CRStatusChecker) checkConditionType(obj *unstructured.Unstructured, completeConditionType string) CRState {
11887
status, found, err := unstructured.NestedMap(obj.Object, "status")
11988
if err != nil || !found {

fault-remediation/pkg/crstatus/crstatus_interface.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,5 @@ import (
2323
)
2424

2525
type CRStatusCheckerInterface interface {
26-
ShouldSkipCRCreation(context.Context, string, string) bool
27-
GetCRState(context.Context, string, string) CRState
2826
GetCRStateForReference(context.Context, string, annotation.MaintenanceResourceReference, string) CRState
2927
}

fault-remediation/pkg/crstatus/crstatus_test.go

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,12 @@ import (
2323
"sigs.k8s.io/controller-runtime/pkg/client/fake"
2424

2525
"github.com/nvidia/nvsentinel/fault-remediation/pkg/annotation"
26-
"github.com/nvidia/nvsentinel/fault-remediation/pkg/config"
2726
)
2827

2928
func TestCheckCondition(t *testing.T) {
30-
testResource := config.MaintenanceResource{
31-
CompleteConditionType: "Completed",
32-
}
33-
cfg := map[string]config.MaintenanceResource{
34-
"test": testResource,
35-
}
29+
completeConditionType := "Completed"
3630

37-
checker := NewCRStatusChecker(nil, cfg, false)
31+
checker := NewCRStatusChecker(nil, false)
3832

3933
tests := []struct {
4034
name string
@@ -118,7 +112,7 @@ func TestCheckCondition(t *testing.T) {
118112

119113
for _, tt := range tests {
120114
t.Run(tt.name, func(t *testing.T) {
121-
result := checker.checkCondition(tt.cr, testResource)
115+
result := checker.checkConditionType(tt.cr, completeConditionType)
122116
assert.Equal(t, tt.expected, result)
123117
})
124118
}
@@ -145,15 +139,7 @@ func TestGetCRStateForReferenceUsesStoredReference(t *testing.T) {
145139
}
146140

147141
fakeClient := fake.NewClientBuilder().WithObjects(storedCR).Build()
148-
checker := NewCRStatusChecker(fakeClient, map[string]config.MaintenanceResource{
149-
"RESTART_BM": {
150-
ApiGroup: "config.example.com",
151-
Version: "v1",
152-
Kind: "ConfigMaintenance",
153-
Namespace: "config-namespace",
154-
CompleteConditionType: "NodeReady",
155-
},
156-
}, false)
142+
checker := NewCRStatusChecker(fakeClient, false)
157143

158144
state := checker.GetCRStateForReference(context.Background(), "stored-cr", annotation.MaintenanceResourceReference{
159145
ApiGroup: "stored.example.com",

fault-remediation/pkg/reconciler/reconciler_test.go

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -65,26 +65,13 @@ func (m *MockK8sClient) GetStatusChecker() crstatus.CRStatusCheckerInterface {
6565
}
6666

6767
type mockStatusChecker struct {
68-
getCRStateFn func(context.Context, string, string) crstatus.CRState
6968
getCRStateForReferenceFn func(context.Context, string, annotation.MaintenanceResourceReference, string) crstatus.CRState
7069
shouldSkip []bool
7170
states []crstatus.CRState
7271
stateByCR map[string]crstatus.CRState
7372
callCount int
7473
}
7574

76-
func (statusChecker *mockStatusChecker) ShouldSkipCRCreation(context.Context, string, string) bool {
77-
return statusChecker.GetCRState(context.Background(), "", "") != crstatus.CRStateFailed
78-
}
79-
80-
func (statusChecker *mockStatusChecker) GetCRState(ctx context.Context, actionName string, crName string) crstatus.CRState {
81-
if statusChecker.getCRStateFn != nil {
82-
return statusChecker.getCRStateFn(ctx, actionName, crName)
83-
}
84-
85-
return statusChecker.nextState(crName)
86-
}
87-
8875
func (statusChecker *mockStatusChecker) GetCRStateForReference(
8976
ctx context.Context,
9077
crName string,

fault-remediation/pkg/remediation/remediation.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,6 @@ func NewRemediationClient(
127127

128128
ctrlRuntimeRemediationClient.statusChecker = crstatus.NewCRStatusChecker(
129129
client,
130-
remediationConfig.RemediationActions,
131130
dryRun,
132131
)
133132

0 commit comments

Comments
 (0)