Skip to content

Commit e5a8295

Browse files
tanishagoyal2tanishagoyal2
committed
chore: coderabit comments changes
Signed-off-by: Tanisha goyal <[email protected]>
1 parent 009d4bd commit e5a8295

File tree

4 files changed

+153
-76
lines changed

4 files changed

+153
-76
lines changed

‎platform-connectors/pkg/connectors/kubernetes/k8s_platform_connector_test.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1604,10 +1604,12 @@ func TestProcessHealthEvents_StoreOnlyStrategy(t *testing.T) {
16041604
}
16051605

16061606
if tc.expectNodeConditions {
1607+
require.NotEmpty(t, nvsentinelConditions,
1608+
"Expected at least one NVSentinel node condition, but got none")
16071609
assert.Equal(t, tc.expectedConditionType, string(nvsentinelConditions[0].Type),
16081610
"Expected condition type %s, got %s", tc.expectedConditionType, nvsentinelConditions[0].Type)
16091611
} else {
1610-
assert.Equal(t, 0, len(nvsentinelConditions),
1612+
assert.Empty(t, nvsentinelConditions,
16111613
"Expected no NVSentinel node conditions for STORE_ONLY events, got %d", len(nvsentinelConditions))
16121614
}
16131615

@@ -1618,10 +1620,12 @@ func TestProcessHealthEvents_StoreOnlyStrategy(t *testing.T) {
16181620
require.NoError(t, err, "Failed to list events")
16191621

16201622
if tc.expectKubernetesEvents {
1623+
require.NotEmpty(t, events.Items,
1624+
"Expected at least one Kubernetes event, but got none")
16211625
assert.Equal(t, tc.expectedEventType, events.Items[0].Type,
16221626
"Expected event type %s, got %s", tc.expectedEventType, events.Items[0].Type)
16231627
} else {
1624-
assert.Equal(t, 0, len(events.Items),
1628+
assert.Empty(t, events.Items,
16251629
"Expected no Kubernetes events for STORE_ONLY events, got %d", len(events.Items))
16261630
}
16271631

‎tests/helpers/kube.go‎

Lines changed: 122 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -2272,34 +2272,16 @@ func waitForDaemonSetRollout(ctx context.Context, t *testing.T, client klient.Cl
22722272
t.Logf("DaemonSet %s/%s rollout completed successfully", NVSentinelNamespace, name)
22732273
}
22742274

2275-
// updateContainerProcessingStrategy updates the processing strategy argument for a specific container.
2276-
func updateContainerProcessingStrategy(container *v1.Container) {
2277-
processingStrategyArg := "--processing-strategy=STORE_ONLY"
2278-
2279-
for j, arg := range container.Args {
2280-
if strings.HasPrefix(arg, "--processing-strategy=") {
2281-
container.Args[j] = processingStrategyArg
2282-
return
2283-
}
2284-
2285-
if arg == "--processing-strategy" && j+1 < len(container.Args) {
2286-
container.Args[j+1] = "STORE_ONLY"
2287-
return
2288-
}
2289-
}
2290-
2291-
container.Args = append(container.Args, processingStrategyArg)
2292-
}
2293-
2294-
// UpdateDaemonSetProcessingStrategy updates the daemonset to use STORE_ONLY processing strategy.
2295-
func UpdateDaemonSetProcessingStrategy(ctx context.Context, t *testing.T,
2296-
client klient.Client, daemonsetName string, containerName string) (*appsv1.DaemonSet, error) {
2275+
// UpdateDaemonSetArgs updates the daemonset with the specified arguments and complete the rollout.
2276+
func UpdateDaemonSetArgs(ctx context.Context, t *testing.T,
2277+
client klient.Client, daemonsetName string, containerName string,
2278+
args map[string]string) error {
22972279
t.Helper()
22982280

2299-
t.Logf("Updating daemonset %s/%s to use STORE_ONLY processing strategy", NVSentinelNamespace, daemonsetName)
2300-
23012281
var originalDaemonSet *appsv1.DaemonSet
23022282

2283+
t.Logf("Updating daemonset %s/%s with args %v", NVSentinelNamespace, daemonsetName, args)
2284+
23032285
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
23042286
daemonSet := &appsv1.DaemonSet{}
23052287
if err := client.Resources().Get(ctx, daemonsetName, NVSentinelNamespace, daemonSet); err != nil {
@@ -2310,18 +2292,26 @@ func UpdateDaemonSetProcessingStrategy(ctx context.Context, t *testing.T,
23102292
originalDaemonSet = daemonSet.DeepCopy()
23112293
}
23122294

2313-
for i := range daemonSet.Spec.Template.Spec.Containers {
2314-
container := &daemonSet.Spec.Template.Spec.Containers[i]
2315-
if container.Name == containerName {
2316-
updateContainerProcessingStrategy(container)
2295+
containers := daemonSet.Spec.Template.Spec.Containers
2296+
containerFound := false
2297+
2298+
for i := range containers {
2299+
if containers[i].Name == containerName {
2300+
setArgsOnContainer(t, &containers[i], args)
2301+
containerFound = true
2302+
23172303
break
23182304
}
23192305
}
23202306

2307+
if !containerFound {
2308+
return fmt.Errorf("container %q not found in daemonset %s/%s", containerName, NVSentinelNamespace, daemonsetName)
2309+
}
2310+
23212311
return client.Resources().Update(ctx, daemonSet)
23222312
})
23232313
if err != nil {
2324-
return nil, err
2314+
return err
23252315
}
23262316

23272317
t.Logf("Waiting for daemonset %s/%s rollout to complete", NVSentinelNamespace, daemonsetName)
@@ -2330,34 +2320,42 @@ func UpdateDaemonSetProcessingStrategy(ctx context.Context, t *testing.T,
23302320
t.Logf("Waiting 10 seconds for daemonset pods to start")
23312321
time.Sleep(10 * time.Second)
23322322

2333-
return originalDaemonSet, nil
2323+
return nil
23342324
}
23352325

2336-
func RestoreDaemonSet(ctx context.Context, t *testing.T, client klient.Client,
2337-
originalDaemonSet *appsv1.DaemonSet, daemonsetName string,
2326+
func RemoveDaemonSetArgs(ctx context.Context, t *testing.T, client klient.Client,
2327+
daemonsetName string,
2328+
containerName string, args map[string]string,
23382329
) error {
23392330
t.Helper()
23402331

2341-
if originalDaemonSet == nil {
2342-
t.Log("No original daemonset to restore, skipping")
2343-
return nil
2344-
}
2345-
2346-
t.Logf("Restoring daemonset %s/%s to original state", NVSentinelNamespace, daemonsetName)
2332+
t.Logf("Removing args %v from daemonset %s/%s", args, NVSentinelNamespace, daemonsetName)
23472333

23482334
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
23492335
daemonSet := &appsv1.DaemonSet{}
23502336
if err := client.Resources().Get(ctx, daemonsetName, NVSentinelNamespace, daemonSet); err != nil {
23512337
return err
23522338
}
23532339

2354-
daemonSet.Spec.Template.Spec.Containers = originalDaemonSet.Spec.Template.Spec.Containers
2340+
containers := daemonSet.Spec.Template.Spec.Containers
2341+
containerFound := false
2342+
2343+
for i := range containers {
2344+
if containers[i].Name == containerName {
2345+
removeArgsFromContainer(&containers[i], args)
2346+
containerFound = true
2347+
2348+
break
2349+
}
2350+
}
2351+
2352+
if !containerFound {
2353+
return fmt.Errorf("container %q not found in daemonset %s/%s", containerName, NVSentinelNamespace, daemonsetName)
2354+
}
23552355

23562356
return client.Resources().Update(ctx, daemonSet)
23572357
})
2358-
if err != nil {
2359-
return err
2360-
}
2358+
require.NoError(t, err, "failed to remove args from daemonset %s/%s", NVSentinelNamespace, daemonsetName)
23612359

23622360
t.Logf("Waiting for daemonset %s/%s rollout to complete after restoration", NVSentinelNamespace, daemonsetName)
23632361
waitForDaemonSetRollout(ctx, t, client, daemonsetName)
@@ -2367,6 +2365,88 @@ func RestoreDaemonSet(ctx context.Context, t *testing.T, client klient.Client,
23672365
return nil
23682366
}
23692367

2368+
// tryUpdateExistingArg attempts to update an existing argument at position j.
2369+
// Returns true if the argument was found and updated.
2370+
func tryUpdateExistingArg(container *v1.Container, j int, flag, value string) bool {
2371+
existingArg := container.Args[j]
2372+
2373+
// Match --flag=value style
2374+
if strings.HasPrefix(existingArg, flag+"=") {
2375+
if value != "" {
2376+
container.Args[j] = flag + "=" + value
2377+
} else {
2378+
container.Args[j] = flag
2379+
}
2380+
2381+
return true
2382+
}
2383+
2384+
// Match --flag or --flag value style
2385+
if existingArg == flag {
2386+
if value != "" {
2387+
if j+1 < len(container.Args) && !strings.HasPrefix(container.Args[j+1], "-") {
2388+
container.Args[j+1] = value
2389+
} else {
2390+
container.Args = append(container.Args[:j+1], append([]string{value}, container.Args[j+1:]...)...)
2391+
}
2392+
}
2393+
2394+
return true
2395+
}
2396+
2397+
return false
2398+
}
2399+
2400+
func setArgsOnContainer(t *testing.T, container *v1.Container, args map[string]string) {
2401+
t.Helper()
2402+
t.Logf("Setting args %v on container %s", args, container.Name)
2403+
2404+
for flag, value := range args {
2405+
found := false
2406+
2407+
for j := 0; j < len(container.Args); j++ {
2408+
if tryUpdateExistingArg(container, j, flag, value) {
2409+
found = true
2410+
break
2411+
}
2412+
}
2413+
2414+
if !found {
2415+
if value != "" {
2416+
container.Args = append(container.Args, flag+"="+value)
2417+
} else {
2418+
container.Args = append(container.Args, flag)
2419+
}
2420+
}
2421+
}
2422+
}
2423+
2424+
func removeArgsFromContainer(container *v1.Container, args map[string]string) {
2425+
for flag := range args {
2426+
for j := 0; j < len(container.Args); j++ {
2427+
existingArg := container.Args[j]
2428+
2429+
// Match --flag=value style
2430+
if strings.HasPrefix(existingArg, flag+"=") {
2431+
container.Args = append(container.Args[:j], container.Args[j+1:]...)
2432+
break
2433+
}
2434+
2435+
// Match --flag or --flag value style
2436+
2437+
if existingArg == flag {
2438+
if j+1 < len(container.Args) && !strings.HasPrefix(container.Args[j+1], "-") {
2439+
container.Args = append(container.Args[:j], container.Args[j+2:]...)
2440+
} else {
2441+
container.Args = append(container.Args[:j], container.Args[j+1:]...)
2442+
}
2443+
2444+
break
2445+
}
2446+
}
2447+
}
2448+
}
2449+
23702450
func GetDaemonSetPodOnWorkerNode(ctx context.Context, t *testing.T, client klient.Client,
23712451
daemonsetName string, podNamePattern string) (*v1.Pod, error) {
23722452
t.Helper()

‎tests/helpers/syslog-health-monitor.go‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"testing"
2020

2121
"github.com/stretchr/testify/require"
22-
appsv1 "k8s.io/api/apps/v1"
2322
v1 "k8s.io/api/core/v1"
2423
"sigs.k8s.io/e2e-framework/klient"
2524
)
@@ -32,15 +31,13 @@ const (
3231

3332
// helper function to set up syslog health monitor and port forward to it
3433
func SetUpSyslogHealthMonitor(ctx context.Context, t *testing.T,
35-
client klient.Client, updatedDaemonSet bool) (string, *appsv1.DaemonSet, *v1.Pod, chan struct{}) {
36-
var originalDaemonSet *appsv1.DaemonSet
37-
34+
client klient.Client, args map[string]string) (string, *v1.Pod, chan struct{}) {
3835
var err error
3936

4037
var syslogPod *v1.Pod
4138

42-
if updatedDaemonSet {
43-
originalDaemonSet, err = UpdateDaemonSetProcessingStrategy(ctx, t, client, SyslogDaemonSetName, SyslogContainerName)
39+
if args != nil {
40+
err = UpdateDaemonSetArgs(ctx, t, client, SyslogDaemonSetName, SyslogContainerName, args)
4441
require.NoError(t, err, "failed to update syslog health monitor processing strategy")
4542
}
4643

@@ -70,18 +67,18 @@ func SetUpSyslogHealthMonitor(ctx context.Context, t *testing.T,
7067
err = SetNodeManagedByNVSentinel(ctx, client, testNodeName, false)
7168
require.NoError(t, err, "failed to set ManagedByNVSentinel label")
7269

73-
return testNodeName, originalDaemonSet, syslogPod, stopChan
70+
return testNodeName, syslogPod, stopChan
7471
}
7572

7673
// helper function to roll back syslog health monitor daemonset and stop the port forward
7774
func TearDownSyslogHealthMonitor(ctx context.Context, t *testing.T, client klient.Client,
78-
originalDaemonSet *appsv1.DaemonSet, nodeName string, stopChan chan struct{},
79-
updatedDaemonSet bool, podName string) {
75+
nodeName string, stopChan chan struct{},
76+
args map[string]string, podName string) {
8077
t.Log("Stopping port-forward")
8178
close(stopChan)
8279

83-
if updatedDaemonSet {
84-
err := RestoreDaemonSet(ctx, t, client, originalDaemonSet, "syslog-health-monitor-regular")
80+
if args != nil {
81+
err := RemoveDaemonSetArgs(ctx, t, client, SyslogDaemonSetName, SyslogContainerName, args)
8582
require.NoError(t, err, "failed to restore syslog health monitor daemon set")
8683
}
8784

0 commit comments

Comments
 (0)