Skip to content

Commit 0752eb5

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

File tree

4 files changed

+155
-76
lines changed

4 files changed

+155
-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: 124 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,27 @@ 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+
2302+
containerFound = true
2303+
23172304
break
23182305
}
23192306
}
23202307

2308+
if !containerFound {
2309+
return fmt.Errorf("container %q not found in daemonset %s/%s", containerName, NVSentinelNamespace, daemonsetName)
2310+
}
2311+
23212312
return client.Resources().Update(ctx, daemonSet)
23222313
})
23232314
if err != nil {
2324-
return nil, err
2315+
return err
23252316
}
23262317

23272318
t.Logf("Waiting for daemonset %s/%s rollout to complete", NVSentinelNamespace, daemonsetName)
@@ -2330,34 +2321,43 @@ func UpdateDaemonSetProcessingStrategy(ctx context.Context, t *testing.T,
23302321
t.Logf("Waiting 10 seconds for daemonset pods to start")
23312322
time.Sleep(10 * time.Second)
23322323

2333-
return originalDaemonSet, nil
2324+
return nil
23342325
}
23352326

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

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)
2333+
t.Logf("Removing args %v from daemonset %s/%s", args, NVSentinelNamespace, daemonsetName)
23472334

23482335
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
23492336
daemonSet := &appsv1.DaemonSet{}
23502337
if err := client.Resources().Get(ctx, daemonsetName, NVSentinelNamespace, daemonSet); err != nil {
23512338
return err
23522339
}
23532340

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

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

23622362
t.Logf("Waiting for daemonset %s/%s rollout to complete after restoration", NVSentinelNamespace, daemonsetName)
23632363
waitForDaemonSetRollout(ctx, t, client, daemonsetName)
@@ -2367,6 +2367,88 @@ func RestoreDaemonSet(ctx context.Context, t *testing.T, client klient.Client,
23672367
return nil
23682368
}
23692369

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