Skip to content

Conversation

@rajkumar-palani
Copy link
Contributor

@rajkumar-palani rajkumar-palani commented Aug 26, 2025

Description

Set the volume identifier on the remote volume to be same as local volume. Additional details for the changes involved is present in the comment.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
https://github.com/dell/csm/issues/1941

Checklist:

  • Have you run format,vet & lint checks against your submission?
  • Have you made sure that the code compiles?
  • Did you run the unit & integration tests successfully?
  • Have you maintained at least 90% code coverage?
  • Have you commented your code, particularly in hard-to-understand areas
  • Have you done corresponding changes to the documentation
  • Did you run tests in a real Kubernetes cluster?
  • Backward compatibility is not broken

How Has This Been Tested?

  1. Volume operations like creation and expansion - Passed.
  2. Replication Tests with Auth - Passed
  3. Replication Tests without Auth - Passed.
  4. Snapshots Tests - Passed.

@SinuChacko
Copy link

Problem

When modifying an R2 (replicated) volume, the operation was blocked by the Authorization service.

The root cause: The R2 volume was created directly by PowerMax (via a replication policy on R1’s storage group) and not by the CSI driver.

As a result, the Authorization database (Redis) did not have ownership information about the R2 volume.

When the CSI driver later detected the R1 was replicated, it located the R2 and tried to give it the same volume label as R1. This label update failed because Authorization denied the request due to the missing Redis entry.

Fix

We introduced two key changes:

  1. Bypass the premature Authorization check
  • The driver now updates the R2 volume label only after it is added to its default storage group.

  • This ensures that the SRP and storage group context are already established, allowing the volume to be recognized properly.

  1. Resequencing the workflow
  • Previously: Volume rename (label update) happened too early → Authorization denied it.

Before Fix:

Step 1: Powermax creates and adds R2 to its replication storage group.
Step 2: Rename R2 to match R1’s identifier. ==> Fails, as Auth has no R2 entry in redis.
Step 3: Add R2 to its default SRP storage group. == Step fails
Step 3: Seed the Redis database with R2’s details (name, capacity, etc.). ==> Step missed

After Fix:

Step 1: Powermax creates and adds R2 to its replication storage group.
Step 2: Add R2 to its default SRP storage group.
Step 3: Rename R2 to match R1’s identifier.
Step 3: Seed the Redis database with R2’s details (name, capacity, etc.).

This guarantees Redis has the necessary information before any further operations run.

Outcome

  1. With this fix, volume modifications on R2 succeed without Authorization blocking.
  2. Redis is properly seeded with R2’s details.

We validated the end-to-end workflows (replication, snapshot, and expansion). All tests passed successfully.

@rajkumar-palani rajkumar-palani changed the title PowerMax Rename Volume Logic update. Rename Remote volume after adding it to default storage group Aug 28, 2025
Copy link

@SinuChacko SinuChacko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TEST OK

@rajkumar-palani rajkumar-palani changed the title Rename Remote volume after adding it to default storage group Rename Remote volume after adding it to default storage group - don't Merge Aug 28, 2025
@rajkumar-palani rajkumar-palani changed the title Rename Remote volume after adding it to default storage group - don't Merge Rename Remote volume after adding it to default storage group Aug 28, 2025
@github-actions
Copy link
Contributor

Merging this branch will not change overall coverage

Impacted Packages Coverage Δ 🤖
github.com/dell/csi-powermax/service 0.00% (ø)

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/dell/csi-powermax/service/controller.go 0.00% (ø) 0 0 0

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

@rajkumar-palani rajkumar-palani merged commit 5cab0f1 into main Aug 29, 2025
6 checks passed
@rajkumar-palani rajkumar-palani deleted the usr/raj/interop-powermax branch August 29, 2025 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants