Bugfix/link volume to volume #324

MaksimDell · 2024-08-28T12:09:44Z

Description

Issue: When a bootable source volume is cloned, the cloned (target) volume is seen to be empty, so the resulting VM cannot boot.

Cause: The array's delayed response to the REST API request causes the initial clone workflow to fail. The retry process does not adequately check if the target volume has been properly created and linked to the snapshot. This leads to an empty volume being created

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
dell/csm#1425

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?

Unit test results:

standaloneproxy results:

RESPONSE_BODY: 
time="2024-08-28T11:14:00Z" level=info msg="Request ID: 8 - GET /univmax/restapi/private/91/sloprovisioning/symmetrix/000000000001"
RESPONSE_BODY: 
PASS
ok      revproxy/v2     0.253s

unit-test:

--- PASS: TestLocks (0.16s)
=== RUN   TestGetVolSize
=== RUN   TestGetVolSize/#00
=== PAUSE TestGetVolSize/#00
=== RUN   TestGetVolSize/#01
=== PAUSE TestGetVolSize/#01
=== RUN   TestGetVolSize/#02
=== PAUSE TestGetVolSize/#02
=== RUN   TestGetVolSize/#03
=== PAUSE TestGetVolSize/#03
=== RUN   TestGetVolSize/#04
=== PAUSE TestGetVolSize/#04
=== RUN   TestGetVolSize/#05
=== PAUSE TestGetVolSize/#05
=== RUN   TestGetVolSize/#06
=== PAUSE TestGetVolSize/#06
=== RUN   TestGetVolSize/#07
=== PAUSE TestGetVolSize/#07
=== RUN   TestGetVolSize/#08
=== PAUSE TestGetVolSize/#08
=== RUN   TestGetVolSize/#09
=== PAUSE TestGetVolSize/#09
=== RUN   TestGetVolSize/#10
=== PAUSE TestGetVolSize/#10
=== CONT  TestGetVolSize/#00
=== CONT  TestGetVolSize/#06
=== CONT  TestGetVolSize/#05
=== CONT  TestGetVolSize/#04
=== CONT  TestGetVolSize/#10
=== CONT  TestGetVolSize/#03
=== CONT  TestGetVolSize/#02
=== CONT  TestGetVolSize/#01
=== CONT  TestGetVolSize/#08
=== CONT  TestGetVolSize/#07
=== CONT  TestGetVolSize/#09
--- PASS: TestGetVolSize (0.00s)
    --- PASS: TestGetVolSize/#00 (0.00s)
    --- PASS: TestGetVolSize/#05 (0.00s)
    --- PASS: TestGetVolSize/#06 (0.00s)
    --- PASS: TestGetVolSize/#04 (0.00s)
    --- PASS: TestGetVolSize/#10 (0.00s)
    --- PASS: TestGetVolSize/#02 (0.00s)
    --- PASS: TestGetVolSize/#01 (0.00s)
    --- PASS: TestGetVolSize/#03 (0.00s)
    --- PASS: TestGetVolSize/#08 (0.00s)
    --- PASS: TestGetVolSize/#07 (0.00s)
    --- PASS: TestGetVolSize/#09 (0.00s)
=== RUN   TestVolumeIdentifier
--- PASS: TestVolumeIdentifier (0.00s)
=== RUN   TestMetroCSIDeviceID
--- PASS: TestMetroCSIDeviceID (0.00s)
=== RUN   TestStringSliceComparison
--- PASS: TestStringSliceComparison (0.00s)
=== RUN   TestStringSliceRegexMatcher
--- PASS: TestStringSliceRegexMatcher (0.00s)
=== RUN   TestExecCommandHelper
--- PASS: TestExecCommandHelper (0.00s)
=== RUN   TestAppendIfMissing
--- PASS: TestAppendIfMissing (0.00s)
=== RUN   TestTruncateString
--- PASS: TestTruncateString (0.00s)
=== RUN   TestFibreChannelSplitInitiatorID
--- PASS: TestFibreChannelSplitInitiatorID (0.00s)
=== RUN   TestPending
--- PASS: TestPending (0.00s)
=== RUN   TestGobrickInitialization
--- PASS: TestGobrickInitialization (0.00s)
=== RUN   TestSetGetLogFields
--- PASS: TestSetGetLogFields (0.00s)
=== RUN   TestEsnureISCSIDaemonIsStarted
--- PASS: TestEsnureISCSIDaemonIsStarted (0.00s)
PASS
status 0
ok      github.com/dell/csi-powermax/v2/service 210.290s        coverage: 75.7% of statements

godog result:

681 scenarios (681 passed)
4578 steps (4578 passed)
3m29.953490046s
godog finished
--- PASS: TestGoDog (210.03s)
PASS
coverage: 74.5% of statements
status 0
ok      github.com/dell/csi-powermax/v2/service 210.447s        coverage: 74.5% of statements

Int test results:
ok command-line-arguments 86.931s coverage: 11.0% of statements in ../../service

MaksimDell · 2024-08-28T12:36:09Z

csireverseproxy/pkg/standaloneproxy/standaloneproxy.go

@@ -382,6 +382,7 @@ func (revProxy *StandAloneProxy) GetRouter() http.Handler {
 	router.PathPrefix(utils.Prefix + "/{version}/system/symmetrix/{symid}").HandlerFunc(revProxy.ServeReverseProxy)
 	router.PathPrefix(utils.Prefix + "/{version}/sloprovisioning/symmetrix/{symid}").HandlerFunc(revProxy.ServeReverseProxy)
 	router.PathPrefix(utils.PrivatePrefix + "/{version}/replication/symmetrix/{symid}").HandlerFunc(revProxy.ServeReverseProxy)
+	router.PathPrefix(utils.PrivatePrefix + "/{version}/sloprovisioning/symmetrix/{symid}").HandlerFunc(revProxy.ServeReverseProxy)


There was a missing private handler for sloprovisioning and our request was dropped

This seems to indicate that all requests using that API will get dropped. Was this the original problem? Any idea why it took this long to be found?

When we did the testing, we saw a 404 error because we used a private prefix for the request but there was no appropriate handler. @delldubey Could you provide more details for this Handler?

MaksimDell · 2024-08-28T12:38:13Z

service/snap.go

@@ -470,7 +471,11 @@ func (s *service) LinkVolumeToVolume(ctx context.Context, symID string, vol *typ
 	// Link the Target to the created snapshot
 	err = s.LinkVolumeToSnapshot(ctx, symID, vol.VolumeID, tgtDevID, snapID, reqID, isCopy, pmaxClient)
 	if err != nil {
-		return err
+		if strings.Contains(err.Error(), errDesiredState) {


We don't need to return an error message if the error message contains "already in the desired state or mode" which is part of idempotency step

MaksimDell · 2024-08-28T12:39:39Z

test/integration/step_defs_test.go

@@ -1093,6 +1093,18 @@ func (f *feature) iCallLinkVolumeToSnapshot() error {
 	return nil
 }

+func (f *feature) iCallLinkVolumeToSnapshotAgain() error {


The difference between iCallLinkVolumeToSnapshotAgain() and iCallLinkVolumeToSnapshot() is that we do not add a prefix to req. name to simulate slow Unisphere and test the idempotency step

MaksimDell · 2024-08-28T12:40:00Z

test/integration/step_defs_test.go

@@ -1106,6 +1118,18 @@ func (f *feature) iCallLinkVolumeToVolume() error {
 	return nil
 }

+func (f *feature) iCallLinkVolumeToVolumeAgain() error {


The difference between iCallLinkVolumeToVolumeAgain() andiCallLinkVolumeToVolume() is that we do not add a prefix to req. name to simulate slow Unisphere and test the idempotency step

delldubey

LGTM

…si-powermax into bugfix/LinkVolumeToVolume

delldubey

LGTM

AkshaySainiDell

LGTM

Add idempotency check and private handler for slow provisioning

MaksimDell added 6 commits August 14, 2024 17:19

add linking when replication is disabled

72f3f84

add idempotency check. Add unit and int tests

8e9559f

add test for standaloneproxy private handler

126eafc

add private handler for sloprovisioning

bdd0531

refactor snapID variable

b0d2921

update test version

30df851

MaksimDell linked an issue Aug 28, 2024 that may be closed by this pull request

[BUG]: Incorrect Volume Creation Due to Idempotency in CreateVolume dell/csm#1425

Closed

Merge branch 'main' into bugfix/LinkVolumeToVolume

156830e

MaksimDell marked this pull request as ready for review August 28, 2024 12:32

MaksimDell requested review from abhi16394, adarsh-dell, AkshaySainiDell, bpjain2004, donatwork and tdawe as code owners August 28, 2024 12:32

MaksimDell commented Aug 28, 2024

View reviewed changes

delldubey previously approved these changes Aug 28, 2024

View reviewed changes

MaksimDell added 2 commits August 28, 2024 17:54

update test version

ecb085d

Merge branch 'bugfix/LinkVolumeToVolume' of https://github.com/dell/c…

527ab9b

…si-powermax into bugfix/LinkVolumeToVolume

MaksimDell dismissed delldubey’s stale review via 527ab9b August 28, 2024 15:56

delldubey approved these changes Aug 29, 2024

View reviewed changes

AkshaySainiDell approved these changes Aug 29, 2024

View reviewed changes

MaksimDell merged commit 37d00dd into main Aug 29, 2024
5 checks passed

MaksimDell deleted the bugfix/LinkVolumeToVolume branch August 29, 2024 08:57

Sahiba-Gupta pushed a commit that referenced this pull request Oct 11, 2024

Bugfix/link volume to volume (#324)

ffa79af

Add idempotency check and private handler for slow provisioning

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bugfix/link volume to volume #324

Bugfix/link volume to volume #324

MaksimDell commented Aug 28, 2024 •

edited

Loading

MaksimDell Aug 28, 2024

donatwork Aug 28, 2024 •

edited

Loading

MaksimDell Aug 28, 2024

MaksimDell Aug 28, 2024

MaksimDell Aug 28, 2024

MaksimDell Aug 28, 2024

delldubey left a comment

delldubey left a comment

AkshaySainiDell left a comment

Bugfix/link volume to volume #324

Bugfix/link volume to volume #324

Conversation

MaksimDell commented Aug 28, 2024 • edited Loading

Description

GitHub Issues

Checklist:

How Has This Been Tested?

MaksimDell Aug 28, 2024

Choose a reason for hiding this comment

donatwork Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

MaksimDell Aug 28, 2024

Choose a reason for hiding this comment

MaksimDell Aug 28, 2024

Choose a reason for hiding this comment

MaksimDell Aug 28, 2024

Choose a reason for hiding this comment

MaksimDell Aug 28, 2024

Choose a reason for hiding this comment

delldubey left a comment

Choose a reason for hiding this comment

delldubey left a comment

Choose a reason for hiding this comment

AkshaySainiDell left a comment

Choose a reason for hiding this comment

MaksimDell commented Aug 28, 2024 •

edited

Loading

donatwork Aug 28, 2024 •

edited

Loading