Skip to content

Commit 1df5558

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

File tree

4 files changed

+147
-76
lines changed

4 files changed

+147
-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: 116 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,23 @@ 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+
for i := range containers {
2298+
if containers[i].Name == containerName {
2299+
setArgsOnContainer(t, &containers[i], args)
2300+
containerFound = true
23172301
break
23182302
}
23192303
}
2304+
if !containerFound {
2305+
return fmt.Errorf("container %q not found in daemonset %s/%s", containerName, NVSentinelNamespace, daemonsetName)
2306+
}
23202307

23212308
return client.Resources().Update(ctx, daemonSet)
23222309
})
23232310
if err != nil {
2324-
return nil, err
2311+
return err
23252312
}
23262313

23272314
t.Logf("Waiting for daemonset %s/%s rollout to complete", NVSentinelNamespace, daemonsetName)
@@ -2330,34 +2317,39 @@ func UpdateDaemonSetProcessingStrategy(ctx context.Context, t *testing.T,
23302317
t.Logf("Waiting 10 seconds for daemonset pods to start")
23312318
time.Sleep(10 * time.Second)
23322319

2333-
return originalDaemonSet, nil
2320+
return nil
23342321
}
23352322

2336-
func RestoreDaemonSet(ctx context.Context, t *testing.T, client klient.Client,
2337-
originalDaemonSet *appsv1.DaemonSet, daemonsetName string,
2323+
func RemoveDaemonSetArgs(ctx context.Context, t *testing.T, client klient.Client,
2324+
daemonsetName string,
2325+
containerName string, args map[string]string,
23382326
) error {
23392327
t.Helper()
23402328

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

23482331
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
23492332
daemonSet := &appsv1.DaemonSet{}
23502333
if err := client.Resources().Get(ctx, daemonsetName, NVSentinelNamespace, daemonSet); err != nil {
23512334
return err
23522335
}
23532336

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

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

23622354
t.Logf("Waiting for daemonset %s/%s rollout to complete after restoration", NVSentinelNamespace, daemonsetName)
23632355
waitForDaemonSetRollout(ctx, t, client, daemonsetName)
@@ -2367,6 +2359,88 @@ func RestoreDaemonSet(ctx context.Context, t *testing.T, client klient.Client,
23672359
return nil
23682360
}
23692361

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