Skip to content

Commit 19df68e

Browse files
lukeatdelllukeatdell
authored
Fix reprotect time out bug (#412)
1 parent 8a63422 commit 19df68e

File tree

8 files changed

+484
-12
lines changed

8 files changed

+484
-12
lines changed

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,4 @@ semver.mk
2121
goisilon
2222
gocsi
2323
.idea
24+
csm-common.mk

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ dev-build-image-push: dev-build
5656

5757
# Windows or Linux; requires no hardware
5858
unit-test:
59-
( cd service; go clean -cache; go test -v -coverprofile=c.out ./... )
59+
( cd service; go test -race -coverprofile=c.out ./... )
6060

6161
coverage:
6262
cd service; go tool cover -html=c.out -o coverage.html

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ require (
1717
github.com/dell/dell-csi-extensions/volumeGroupSnapshot v1.8.1
1818
github.com/dell/gocsi v1.14.0
1919
github.com/dell/gofsutil v1.19.0
20-
github.com/dell/goisilon v1.19.0
20+
github.com/dell/goisilon v1.19.1-0.20250702203446-019e249ac658
2121
github.com/fsnotify/fsnotify v1.9.0
2222
github.com/google/uuid v1.6.0
2323
github.com/gorilla/mux v1.8.1

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,8 @@ github.com/dell/gocsi v1.14.0 h1:DinuonGUvr4P898+xX0zk/ZHAvetJrr48GhKyHS3PL0=
121121
github.com/dell/gocsi v1.14.0/go.mod h1:YhF+LnH5S8t96qotoLk6wUjMzCwSy8rmnqvx7h4Nc3s=
122122
github.com/dell/gofsutil v1.19.0 h1:41Hx24Tlie37UbOwatHgNAV0nq+KQysrhABXCnpOs+E=
123123
github.com/dell/gofsutil v1.19.0/go.mod h1:yOuHhBa+iI/QXhZGm7BgCEdf+ji1nIFXzkObv1x0SRs=
124-
github.com/dell/goisilon v1.19.0 h1:KlGxcc0ajnvtdQjeUnyFPhA8CYlikkTmhKvqzgrA/BY=
125-
github.com/dell/goisilon v1.19.0/go.mod h1:U3x/bpzk44sltcynlF5sQ0An0v9STgA5PCctdcv5u5U=
124+
github.com/dell/goisilon v1.19.1-0.20250702203446-019e249ac658 h1:auPujV6jR9UW7gUbRFISP16Gz0TI7eTkc+k/uP7APsw=
125+
github.com/dell/goisilon v1.19.1-0.20250702203446-019e249ac658/go.mod h1:U3x/bpzk44sltcynlF5sQ0An0v9STgA5PCctdcv5u5U=
126126
github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ=
127127
github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no=
128128
github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3ebgob9U8Nd0kOddGdZWjyMGR8Wziv+TBNwSE=

‎service/node_test.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ import (
3030
"golang.org/x/net/context"
3131
)
3232

33+
const testTargetPath = "/tmp/csi-powerscale-test"
34+
3335
func TestNodeGetVolumeStats(t *testing.T) {
3436
// Original function references
3537
originalGetIsVolumeExistentFunc := getIsVolumeExistentFunc
@@ -337,6 +339,7 @@ func TestEphemeralNodePublish(t *testing.T) {
337339
VolumeContext: map[string]string{
338340
"csi.storage.k8s.io/ephemeral": "true",
339341
},
342+
TargetPath: testTargetPath,
340343
},
341344
setup: func() {
342345
getCreateVolumeFunc = func(_ *service) func(_ context.Context, _ *csi.CreateVolumeRequest) (*csi.CreateVolumeResponse, error) {
@@ -360,6 +363,7 @@ func TestEphemeralNodePublish(t *testing.T) {
360363
VolumeContext: map[string]string{
361364
"csi.storage.k8s.io/ephemeral": "true",
362365
},
366+
TargetPath: testTargetPath,
363367
},
364368
setup: func() {
365369
// make s a service with no nodeID
@@ -397,6 +401,7 @@ func TestEphemeralNodePublish(t *testing.T) {
397401
VolumeContext: map[string]string{
398402
"csi.storage.k8s.io/ephemeral": "true",
399403
},
404+
TargetPath: testTargetPath,
400405
},
401406
setup: func() {
402407
getCreateVolumeFunc = func(_ *service) func(_ context.Context, _ *csi.CreateVolumeRequest) (*csi.CreateVolumeResponse, error) {
@@ -427,6 +432,7 @@ func TestEphemeralNodePublish(t *testing.T) {
427432
VolumeContext: map[string]string{
428433
"csi.storage.k8s.io/ephemeral": "true",
429434
},
435+
TargetPath: testTargetPath,
430436
},
431437
setup: func() {
432438
getCreateVolumeFunc = func(_ *service) func(_ context.Context, _ *csi.CreateVolumeRequest) (*csi.CreateVolumeResponse, error) {
@@ -460,6 +466,7 @@ func TestEphemeralNodePublish(t *testing.T) {
460466
VolumeContext: map[string]string{
461467
"csi.storage.k8s.io/ephemeral": "true",
462468
},
469+
TargetPath: testTargetPath,
463470
},
464471
setup: func() {
465472
getControllerPublishVolume = func(_ *service) func(_ context.Context, _ *csi.ControllerPublishVolumeRequest) (*csi.ControllerPublishVolumeResponse, error) {
@@ -496,6 +503,7 @@ func TestEphemeralNodePublish(t *testing.T) {
496503
VolumeContext: map[string]string{
497504
"csi.storage.k8s.io/ephemeral": "true",
498505
},
506+
TargetPath: testTargetPath,
499507
},
500508
setup: func() {
501509
getControllerPublishVolume = func(_ *service) func(_ context.Context, _ *csi.ControllerPublishVolumeRequest) (*csi.ControllerPublishVolumeResponse, error) {
@@ -535,6 +543,7 @@ func TestEphemeralNodePublish(t *testing.T) {
535543
VolumeContext: map[string]string{
536544
"csi.storage.k8s.io/ephemeral": "true",
537545
},
546+
TargetPath: testTargetPath,
538547
},
539548
setup: func() {
540549
publishVolumeFunc = func(_ context.Context, _ *csi.NodePublishVolumeRequest, _ string) error {
@@ -579,6 +588,7 @@ func TestEphemeralNodePublish(t *testing.T) {
579588
VolumeContext: map[string]string{
580589
"csi.storage.k8s.io/ephemeral": "true",
581590
},
591+
TargetPath: testTargetPath,
582592
},
583593
setup: func() {
584594
statFileFunc = func(_ string) (fs.FileInfo, error) {
@@ -633,6 +643,7 @@ func TestEphemeralNodePublish(t *testing.T) {
633643
VolumeContext: map[string]string{
634644
"csi.storage.k8s.io/ephemeral": "true",
635645
},
646+
TargetPath: testTargetPath,
636647
},
637648
setup: func() {
638649
statFileFunc = func(_ string) (fs.FileInfo, error) {
@@ -687,6 +698,7 @@ func TestEphemeralNodePublish(t *testing.T) {
687698
VolumeContext: map[string]string{
688699
"csi.storage.k8s.io/ephemeral": "true",
689700
},
701+
TargetPath: testTargetPath,
690702
},
691703
setup: func() {
692704
publishVolumeFunc = func(_ context.Context, _ *csi.NodePublishVolumeRequest, _ string) error {
@@ -738,6 +750,7 @@ func TestEphemeralNodePublish(t *testing.T) {
738750
VolumeContext: map[string]string{
739751
"csi.storage.k8s.io/ephemeral": "true",
740752
},
753+
TargetPath: testTargetPath,
741754
},
742755
setup: func() {
743756
publishVolumeFunc = func(_ context.Context, _ *csi.NodePublishVolumeRequest, _ string) error {
@@ -789,6 +802,7 @@ func TestEphemeralNodePublish(t *testing.T) {
789802
VolumeContext: map[string]string{
790803
"csi.storage.k8s.io/ephemeral": "true",
791804
},
805+
TargetPath: testTargetPath,
792806
},
793807
setup: func() {
794808
closeFileFunc = func(_ *os.File) error {

‎service/replication.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package service
22

33
/*
4-
Copyright (c) 2019-2023 Dell Inc, or its subsidiaries.
4+
Copyright (c) 2019-2025 Dell Inc, or its subsidiaries.
55
66
Licensed under the Apache License, Version 2.0 (the "License");
77
you may not use this file except in compliance with the License.
@@ -750,9 +750,9 @@ func reprotect(ctx context.Context, localIsiConfig *IsilonClusterConfig, remoteI
750750
if err != nil {
751751
return status.Errorf(codes.Internal, "reprotect: create protection policy on the local site failed %s", err.Error())
752752
}
753-
err = localIsiConfig.isiSvc.client.WaitForPolicyLastJobState(ctx, ppName, isi.FINISHED)
753+
err = localIsiConfig.isiSvc.client.WaitForPolicyLastJobState(ctx, ppName, isi.RUNNING, isi.FINISHED)
754754
if err != nil {
755-
return status.Errorf(codes.Internal, "reprotect: policy job couldn't reach FINISHED state %s", err.Error())
755+
return status.Errorf(codes.Internal, "reprotect: policy job did not return one of RUNNING or FINISHED state %s", err.Error())
756756
}
757757

758758
log.Info("Reprotect action completed")
@@ -987,7 +987,9 @@ func getRemoteCSIVolume(ctx context.Context, exportID int, volName, accessZone s
987987
*/
988988
func getGroupLinkState(localP isi.Policy, localTP isi.TargetPolicy, remoteP isi.Policy, remoteTP isi.TargetPolicy, isSyncInProgress bool) csiext.StorageProtectionGroupStatus_State {
989989
var state csiext.StorageProtectionGroupStatus_State
990-
if (localP != nil && localP.Enabled && remoteP == nil && localTP == nil && remoteTP != nil && remoteTP.FailoverFailbackState == WritesDisabled) || // Synchronized state - source side
990+
if isSyncInProgress { // sync-in-progress state
991+
state = csiext.StorageProtectionGroupStatus_SYNC_IN_PROGRESS
992+
} else if (localP != nil && localP.Enabled && remoteP == nil && localTP == nil && remoteTP != nil && remoteTP.FailoverFailbackState == WritesDisabled) || // Synchronized state - source side
991993
(localP == nil && remoteP != nil && remoteP.Enabled && localTP != nil && localTP.FailoverFailbackState == WritesDisabled && remoteTP == nil) { // target side
992994
state = csiext.StorageProtectionGroupStatus_SYNCHRONIZED
993995
} else if (localP != nil && !localP.Enabled && remoteP == nil && localTP == nil && remoteTP != nil && remoteTP.FailoverFailbackState == WritesDisabled) || // Suspended state - source side
@@ -1003,11 +1005,9 @@ func getGroupLinkState(localP isi.Policy, localTP isi.TargetPolicy, remoteP isi.
10031005
} else if (localP != nil && localP.Enabled && remoteP == nil && localTP == nil && remoteTP != nil && remoteTP.FailoverFailbackState == WritesEnabled) || // unplanned failover & source up now - source side
10041006
(localP == nil && remoteP != nil && remoteP.Enabled && localTP != nil && localTP.FailoverFailbackState == WritesEnabled && remoteTP == nil) { // target side
10051007
state = csiext.StorageProtectionGroupStatus_FAILEDOVER
1006-
} else if isSyncInProgress { // sync-in-progress state
1007-
state = csiext.StorageProtectionGroupStatus_SYNC_IN_PROGRESS
10081008
} else if (remoteTP != nil && remoteTP.LastJobState == "failed") || (localTP != nil && localTP.LastJobState == "failed") { // invalid state, sync job failed
10091009
state = csiext.StorageProtectionGroupStatus_INVALID
1010-
} else { // unknown state
1010+
} else {
10111011
state = csiext.StorageProtectionGroupStatus_UNKNOWN
10121012
}
10131013

0 commit comments

Comments
 (0)