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.

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

.github/workflows/int-test-azure-workflow.yml

+      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

.github/workflows/int-test-gcp-workflow.yml

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


Please remember to remove this before merging

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

.github/workflows/int-test-gcp-workflow.yml

-          - { 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

test/appframework_aws/m4/appframework_aws_test.go

 	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

test/testenv/util.go

+		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

test/testenv/util.go

+	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

test/deploy-gcp-cluster.sh

+    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

test/trigger-tests.sh

+          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

test/testenv/cloud_storage_backend.go

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


Add copyright stmt

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

test/testenv/testcaseenv.go


 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

test/testenv/testcaseenv.go

+	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

test/testenv/testcaseenv.go

+	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

test/testenv/util.go

+		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

test/testenv/util.go

+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

test/testenv/verificationutils.go

+					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

test/appframework/c3/appframework_suite_test.go

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

.github/workflows/int-test-azure-workflow.yml

+        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

.github/workflows/int-test-azure-workflow.yml

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

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

.github/workflows/int-test-azure-workflow.yml

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


Remember to remove before merge

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

.github/workflows/int-test-azure-workflow.yml

+      - 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

test/appframework/m4/appframework_suite_test.go

-		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

test/testenv/appframework_utils.go


+// 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

test/testenv/util.go

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


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

test/testenv/verificationutils.go

-		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

test/appframework/c3/appframework_suite_test.go

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


Those are not used - should we remove them?

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

test/appframework/s1/appframework_suite_test.go

 )

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


Why we are not calling loadEnvFile here?

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

.github/workflows/int-test-azure-workflow.yml

        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?

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

		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