cloud storage dedup by Igor-splunk · Pull Request #1800 · splunk/splunk-operator

Igor-splunk · 2026-03-29T20:15:29Z

Description

What does this PR have in it?

Key Changes

Highlight the updates in specific files

Testing and Verification

How did you test these changes? What automated tests are added?

Related Issues

Jira tickets, GitHub issues, Support tickets...

PR Checklist

Code changes adhere to the project's coding standards.
Relevant unit and integration tests are included.
Documentation has been updated accordingly.
All tests pass locally.
The PR description follows the project's guidelines.

* CSPL-4397 Remove time dependency from tests * CSPL-4397 Removing test reliance on time * CSPL-4397 Adjusting timeouts * CSPL-4354 Fixing time dependencies in unit tests and fixing timeouts in integ tests * CSPL-4397 Move to using watch * CSPL-4397 Refactor pod reset detection to use UID-based tracking instead of time-based comparison, add fail-fast test prerequisites validation, and improve code clarity with variable renaming * CSPL-4397 Refactoring * CSPL-4397 Addressing comments

…1768) * CSPL-4602 Moving duplicated code for LM tests to a shared codebase * CSPL-4602 Addinf license manager tests to workflows * CSPL-4602 Address comments * CSPL-4602 Moving LM shared test utils to licensemanager package * CSPL-4602 Addressing base branch changes

* CSPL-4377 Get rid of not expected panic in tests * CSPL-4377 Check error for app disablement in tests

* Enhance error handling in WaitforPhaseChange and fix variable name in NewTestCaseEnv * Updated WaitforPhaseChange to log an error if the phase transition is not observed within the timeout. * Corrected the variable name check in NewTestCaseEnv to use the provided name instead of an uninitialized envName. * Changed Kind from "ClusterManager" to "ClusterMaster" in ClusterMaster struct. * Update error logging in WaitforPhaseChange to use Error level for timeout observations

kasiakoziol · 2026-03-30T08:24:22Z

+      matrix:
+        test:
+          [
+            masterappframeworkc3,


We have to agree whether we want to run master tests. I had the same concern on license manager/master tests and as of now, they are not run for master.

kasiakoziol · 2026-03-30T08:25:44Z

    branches:
      - develop
      - main
+      - CSPL-4601-rebased


Please remember to remove this before merging

kasiakoziol · 2026-03-30T08:26:13Z

-          - { order: 3, name: "m4_gcp_sanity" }
-          - { order: 4, name: "m4_mgr_gcp_sanity" }
-          - { order: 5, name: "s1_gcp_sanity" }
+          - { order: 1, name: "masterappframeworkc3" }


Same concern here

kasiakoziol · 2026-03-30T08:31:16Z

 	BeforeEach(func() {
 		var err error
 		name := fmt.Sprintf("%s-%s", "master"+testenvInstance.GetName(), testenv.RandomDNSName(3))
+		fmt.Printf("[DEBUG] M4 BeforeEach starting, name=%s\n", name)


Are you planning to keep these "logs"?

kasiakoziol · 2026-03-30T08:37:11Z

+		return
+	}
+	logf.Log.Info("[DEBUG] Pod status (wide)", "namespace", ns)
+	for _, line := range strings.Split(string(output), "\n") {


This could be moved to a separate function

kasiakoziol · 2026-03-30T08:37:52Z

+	lines := strings.Split(string(output), "\n")
+	logf.Log.Info("[DEBUG] Recent events", "namespace", ns, "total", len(lines))
+	start := 0
+	if len(lines) > 30 {


30 could be moved to a constant and referenced from there

kasiakoziol · 2026-03-30T08:39:07Z

+    return 0
+  fi
+
+  kubectl config set-credentials ci-test-runner --token="${TOKEN}"


We need to make sure that it doesn't get printed out in the pipelines

kasiakoziol · 2026-04-13T09:54:20Z

+          echo "License path not set. Changing to GCP default"
          export ENTERPRISE_LICENSE_LOCATION="${GCP_ENTERPRISE_LICENSE_LOCATION}"
        fi
        if [[ -z "${ENTERPRISE_LICENSE_LOCATION}" ]]; then


This is the same if as above, in line 103

kasiakoziol · 2026-04-13T09:56:43Z

@@ -0,0 +1,146 @@
+package testenv


Add copyright stmt

kasiakoziol · 2026-04-13T10:06:56Z


 func (testenv *TestCaseEnv) setup() error {
-	testenv.Log.Info("testenv initializing.\n")
+	testenv.Log.Info("[DEBUG] testenv setup starting", "namespace", testenv.namespace, "clusterProvider", ClusterProvider, "clusterWide", installOperatorClusterWide)


Do we want to keep this DEBUG here and below?

kasiakoziol · 2026-04-13T10:07:07Z

+	testenv.Log.Info("[DEBUG] Creating namespace...")
 	err = testenv.createNamespace()
 	if err != nil {
+		testenv.Log.Info("[DEBUG] Failed to create namespace", "error", err)


kasiakoziol · 2026-04-13T10:07:17Z

+	testenv.Log.Info("[DEBUG] Creating service account...")
 	err = testenv.createSA()
 	if err != nil {
+		testenv.Log.Info("[DEBUG] Failed to create service account", "error", err)


kasiakoziol · 2026-04-13T10:08:31Z

+		return
+	}
+	logf.Log.Info("[DEBUG] PVC status", "namespace", ns)
+	for _, line := range strings.Split(string(output), "\n") {


This could be moved to a separate function

kasiakoziol · 2026-04-13T10:08:40Z

+func DumpGetEvents(ns string) {
+	output, err := exec.Command("kubectl", "get", "events", "-n", ns, "--sort-by=.lastTimestamp").Output()
+	if err != nil {
+		logf.Log.Info("[DEBUG] Failed to get events", "namespace", ns, "error", err)


kasiakoziol · 2026-04-13T10:09:57Z

+					gomega.Eventually(func() string {
+						_, ver, err := GetPodAppStatus(ctx, deployment, podName, ns, appName, clusterWideInstall)
+						if err != nil {
+							testenv.Log.Info("Retrying app version check", "pod", podName, "app", appName, "error", err)


kasiakoziol · 2026-04-13T10:12:35Z

If these tests are meant to be applicable to all cloud providers, then I would remove any s3 references in names. (It applies to all files under test/appframework/).

kasiakoziol · 2026-04-13T10:16:26Z

+        run: |
+          az aks get-credentials --resource-group ${{ secrets.AZURE_RESOURCE_GROUP_NAME }} --name ${{ env.TEST_CLUSTER_NAME }} --admin --overwrite-existing
+      - name: Setup long-lived service account auth
+        run: |


It is already in test tools. Could you move it to a separate file and reference it there and here, e.g. as a script file?

kubabuczak

Really nice job. Can you update PR description?

kubabuczak · 2026-04-13T09:31:45Z

We have to check with @vivekr-splunk - he mentioned that we should not change workflow tests

kubabuczak · 2026-04-13T09:32:03Z

    branches:
      - develop
      - main
+      - CSPL-4601-rebased


Remember to remove before merge

kubabuczak · 2026-04-13T09:34:00Z

+      - name: Get AKS credentials
+        run: |
+          az aks get-credentials --resource-group ${{ secrets.AZURE_RESOURCE_GROUP_NAME }} --name ${{ env.TEST_CLUSTER_NAME }} --admin --overwrite-existing
+      - name: Setup long-lived service account auth


I would say that such code should not be placed here - workflow should only execute such scripts or contain very simple logic.

kubabuczak · 2026-04-13T09:51:23Z

-		appListV1 = testenv.BasicApps
-		appFileList := testenv.GetAppFileList(appListV1)
+	ctx := context.TODO()
+	cloudBackend = testenv.NewCloudStorageBackend(testS3Bucket, testDataS3Bucket)


s3 is AWS specific - maybe testDataBucket?

kubabuczak · 2026-04-13T10:05:19Z


+// GenerateAppFrameworkVolumeSpec returns a VolumeSpec appropriate for the current ClusterProvider.
+// Use this instead of calling GenerateIndexVolumeSpec directly with hardcoded provider values.
+func GenerateAppFrameworkVolumeSpec(ctx context.Context, testenvInstance *TestCaseEnv, volumeName string) enterpriseApi.VolumeSpec {


[nit] maybe this should be defined as method on *TestCaseEnv?

kubabuczak · 2026-04-13T10:13:11Z

+		return
+	}
+	logf.Log.Info("[DEBUG] Pod status (wide)", "namespace", ns)
+	for _, line := range strings.Split(string(output), "\n") {


kubabuczak · 2026-04-13T10:14:39Z

-		DumpGetSplunkVersion(ctx, testenv.GetName(), deployment, "monitoring-console")
-		return monitoringConsole.Status.Phase
-	}, ConsistentDuration, ConsistentPollInterval).Should(gomega.Equal(enterpriseApi.PhaseReady))
+	DumpGetSplunkVersion(ctx, testenv.GetName(), deployment, "monitoring-console")


I assume that this and other Consistently code blocks were added to detect phase flip-flopping. Do we check it somehow now?

The original function was flaky or too slow?

kubabuczak · 2026-04-13T10:21:20Z

 	ConsistentPollInterval = 200 * time.Millisecond
 	ConsistentDuration     = 2000 * time.Millisecond


Those are not used - should we remove them?

kubabuczak · 2026-04-13T10:24:56Z

 )

 // TestBasic is the main entry point
 func TestBasic(t *testing.T) {


Why we are not calling loadEnvFile here?

kubabuczak · 2026-04-13T10:25:46Z

        with:
          azcliversion: ${{ steps.dotenv.outputs.AZ_CLI_VERSION }}
          inlineScript: |
-            az aks delete --name  ${{ env.TEST_CLUSTER_NAME }}  --resource-group ${{ secrets.AZURE_RESOURCE_GROUP_NAME }} -y


Why remove TEST_CLUSTER_NAME?

kasiakoziol and others added 7 commits March 16, 2026 07:05

Merge branch 'develop' into feature/test-refactoring

03bf955

Feature/cspl 4377 remove panic from test (#1777)

064e240

* CSPL-4377 Get rid of not expected panic in tests * CSPL-4377 Check error for app disablement in tests

Merge branch 'develop' into feature/test-refactoring

f3c886f

cloud storage dedup

95f58d3

kasiakoziol reviewed Mar 30, 2026

View reviewed changes

Igor-splunk added 12 commits March 30, 2026 11:07

move to a single file

96c9c47

use cloud provider

aee9362

update gcputils

22ba21e

find approot gcp

f5534f2

pass bucket name

f65111d

replace

1814352

Up the timeout

308a6b4

Eventually

f3b7bd7

metrics server

1ea92b4

clean up isolation

dade2c8

Run azure one by one

547b647

Add case for empty status

d41381c

vivekr-splunk self-requested a review April 10, 2026 14:17

vivekr-splunk approved these changes Apr 10, 2026

View reviewed changes

Igor-splunk added 3 commits April 10, 2026 18:17

fix

32372f2

up the timeout

ad9b26c

use v3

594400e

kasiakoziol reviewed Apr 13, 2026

View reviewed changes

kubabuczak reviewed Apr 13, 2026

View reviewed changes

Igor-splunk added 3 commits April 13, 2026 16:53

pool eventually

88777a0

up the timeout

4b6b215

up the timeout

7cc5715

Igor-splunk added 2 commits April 14, 2026 17:13

check for flip-flop

54d16d8

rename

f0921c1

Base automatically changed from feature/test-refactoring to develop April 14, 2026 17:57

Igor-splunk added 2 commits April 15, 2026 00:00

remove mid checks

cb92728

Merge conflicts

2805bd3

		ConsistentPollInterval = 200 * time.Millisecond
		ConsistentDuration = 2000 * time.Millisecond

Conversation

Igor-splunk commented Mar 29, 2026

Description

Key Changes

Testing and Verification

Related Issues

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kubabuczak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants