Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Snapshot License cache fix #388

Merged
merged 6 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion service/features/snapshot.feature
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,32 @@ Feature: PowerMax CSI Interface
Scenario: Snapshot license
Given a PowerMax service
When I check the snapshot license
And I reset the license cache
Then no error was received
And the snapshot license cache has "1" array
And I reset the license cache
And the snapshot license cache has "0" array
@v1.2.0
Scenario: Snapshot license for unlicensed array
Given a PowerMax service
And I induce error "SnapshotNotLicensed"
When I check the snapshot license
And the snapshot license cache has "1" array
And I reset the license cache
Then the error contains "doesn't have Snapshot license"
@v1.2.0
Scenario: Check snapshot license but receive error
Given a PowerMax service
And I induce error "InvalidResponse"
When I check the snapshot license
And the snapshot license cache has "0" array
And I reset the license cache
Then the error contains "induced error"
@v1.2.0
Scenario: Check snapshot but get unisphere mismatch
Given a PowerMax service
And I induce error "UnisphereMismatchError"
When I check the snapshot license
And the snapshot license cache has "0" array
And I reset the license cache
Then the error contains "not being managed by Unisphere"
@v1.2.0
Expand Down
41 changes: 25 additions & 16 deletions service/snap.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright © 2021 Dell Inc. or its subsidiaries. All Rights Reserved.
Copyright © 2021-2024 Dell Inc. or its subsidiaries. All Rights Reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -43,8 +43,7 @@ const (

var (
cleanupStarted = false
licenseCached = false
symmRepCapabilities *types.SymReplicationCapabilities
symmRepCapabilities = make(map[string]types.SymmetrixCapability)
mutex sync.Mutex
snapCleaner *snapCleanupWorker
)
Expand Down Expand Up @@ -175,31 +174,41 @@ func (s *service) UnlinkTargets(ctx context.Context, symID, srcDevID string, pma
return nil
}

// IsSnapshotLicensed return true if the symmetrix array has
// SnapVX license
// This function checks if the PowerMax array has the SnapVX license.
// It returns an error if the array does not meet the expectations.
func (s *service) IsSnapshotLicensed(ctx context.Context, symID string, pmaxClient pmax.Pmax) (err error) {
if _, err := pmaxClient.IsAllowedArray(symID); err != nil {
return err
}
mutex.Lock()
defer mutex.Unlock()
if licenseCached == false {
symmRepCapabilities, err = pmaxClient.GetReplicationCapabilities(ctx)

symmRepCapability, ok := symmRepCapabilities[symID]
if !ok {
repCapabilities, err := pmaxClient.GetReplicationCapabilities(ctx)
if err != nil {
return err
}
licenseCached = true
log.Infof("License information with Powermax %s is cached", symID)
}
for i := range symmRepCapabilities.SymmetrixCapability {
if symmRepCapabilities.SymmetrixCapability[i].SymmetrixID == symID {
if symmRepCapabilities.SymmetrixCapability[i].SnapVxCapable {
return nil

for _, symmCapability := range repCapabilities.SymmetrixCapability {
if symmCapability.SymmetrixID == symID {
symmRepCapabilities[symID] = symmCapability
log.Infof("License information with PowerMax %s is cached", symID)
Copy link
Contributor

Choose a reason for hiding this comment

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

The log message does not seem to reflect what the code is doing. Someone in service may see this and not know we are checking a license.

break
}
return fmt.Errorf("PowerMax array (%s) doesn't have Snapshot license", symID)
}

symmRepCapability, ok = symmRepCapabilities[symID]
if !ok {
return fmt.Errorf("PowerMax array (%s) is not being managed by Unisphere", symID)
}
}
return fmt.Errorf("PowerMax array (%s) is not being managed by Unisphere", symID)

if symmRepCapability.SnapVxCapable {
return nil
}

return fmt.Errorf("PowerMax array (%s) doesn't have Snapshot license", symID)
santhoshatdell marked this conversation as resolved.
Show resolved Hide resolved
}

// UnlinkAndTerminate executes cleanup operation on the source/target volume
Expand Down
14 changes: 12 additions & 2 deletions service/step_defs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3928,9 +3928,18 @@ func (f *feature) noVolumeSource() error {
return nil
}

func (f *feature) validateSnapshotLicenseCache(count int) error {
if len(symmRepCapabilities) != count {
return fmt.Errorf("Expected %d array(s) in the license cache but got %d", count, len(symmRepCapabilities))
}
return nil
}

func (f *feature) iResetTheLicenseCache() error {
licenseCached = false
symmRepCapabilities = nil
for k := range symmRepCapabilities {
delete(symmRepCapabilities, k)
}

return nil
}

Expand Down Expand Up @@ -5143,6 +5152,7 @@ func FeatureContext(s *godog.ScenarioContext) {
s.Step(`^a valid DeleteSnapshotResponse is returned$`, f.aValidDeleteSnapshotResponseIsReturned)
s.Step(`^a non-existent volume$`, f.aNonexistentVolume)
s.Step(`^no volume source$`, f.noVolumeSource)
s.Step(`^the snapshot license cache has "(\d+)" array$`, f.validateSnapshotLicenseCache)
s.Step(`^I reset the license cache$`, f.iResetTheLicenseCache)
s.Step(`^I call IsSnapshotSource$`, f.iCallIsSnapshotSource)
s.Step(`^IsSnapshotSource returns "([^"]*)"$`, f.isSnapshotSourceReturns)
Expand Down
Loading