Skip to content

Commit 508a137

Browse files
authored
CNVA with DEP clean up issues fixed
1 parent d980df9 commit 508a137

File tree

4 files changed

+541
-67
lines changed

4 files changed

+541
-67
lines changed

storage_drivers/ontap/ontap_common.go

Lines changed: 99 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -4982,64 +4982,126 @@ func removeExportPolicyRules(
49824982
exportPolicyMutex.Lock(exportPolicy)
49834983
defer exportPolicyMutex.Unlock(exportPolicy)
49844984

4985-
var removeRuleIndexes []int
4985+
// CNVA-AWARE EXPORT POLICY MANAGEMENT:
4986+
// Instead of blindly removing rules for the departing node, we implement "keep only active nodes" logic.
4987+
// This is critical for CNVA where multiple PVCs (primary + subordinates) share the same export policy.
4988+
// We must preserve rules for all remaining active nodes that still need access to the shared volume.
4989+
4990+
// When all nodes have been unpublished
4991+
if publishInfo.Nodes == nil || len(publishInfo.Nodes) == 0 {
4992+
Logc(ctx).WithFields(fields).Debug("No active nodes remaining, removing ALL export policy rules.")
4993+
4994+
// Get all existing rules and remove them
4995+
existingRules, err := clientAPI.ExportRuleList(ctx, exportPolicy)
4996+
if err != nil {
4997+
Logc(ctx).WithFields(LogFields{
4998+
"policyName": exportPolicy,
4999+
"error": err,
5000+
}).WithError(err).Error("Failed to list export policy rules for cleanup.")
5001+
return err
5002+
}
5003+
5004+
// Remove all existing rules
5005+
for ruleIndex := range existingRules {
5006+
if err := clientAPI.ExportRuleDestroy(ctx, exportPolicy, ruleIndex); err != nil {
5007+
Logc(ctx).WithFields(LogFields{
5008+
"policyName": exportPolicy,
5009+
"ruleIndex": ruleIndex,
5010+
}).WithError(err).Error("Failed to remove export policy rule during full cleanup.")
5011+
// Continue removing other rules even if one fails
5012+
} else {
5013+
Logc(ctx).WithFields(LogFields{
5014+
"policyName": exportPolicy,
5015+
"ruleIndex": ruleIndex,
5016+
}).Debug("Removed export policy rule during full cleanup.")
5017+
}
5018+
}
5019+
5020+
Logc(ctx).WithFields(LogFields{
5021+
"policyName": exportPolicy,
5022+
"totalRulesRemoved": len(existingRules),
5023+
}).Debug("All export policy rules removed for empty volume.")
5024+
return nil
5025+
}
49865026

4987-
nodeIPRules := make(map[string]struct{})
4988-
for _, ip := range publishInfo.HostIP {
4989-
ip = strings.TrimSpace(ip)
4990-
nodeIPRules[ip] = struct{}{}
5027+
// Build set of IPs that should be KEPT (all active nodes)
5028+
activeNodeIPs := make(map[string]struct{})
5029+
for _, node := range publishInfo.Nodes {
5030+
for _, ip := range node.IPs {
5031+
ip = strings.TrimSpace(ip)
5032+
activeNodeIPs[ip] = struct{}{}
5033+
}
49915034
}
49925035

4993-
// Get export policy rules from given policy
5036+
// Get existing export policy rules
49945037
existingExportRules, err := clientAPI.ExportRuleList(ctx, exportPolicy)
49955038
if err != nil {
5039+
Logc(ctx).WithFields(LogFields{
5040+
"policyName": exportPolicy,
5041+
"error": err,
5042+
}).WithError(err).Error("Failed to list existing export policy rules.")
49965043
return err
49975044
}
4998-
Logc(ctx).WithField("existingExportRules", existingExportRules).Debug("Existing export policy rules.")
49995045

5000-
// Match list of rules to rule index based on clientMatch address
5001-
// ONTAP expects the rule index to delete
5046+
var removeRuleIndexes []int
5047+
5048+
// Analyze each existing rule to determine if it should be kept or removed
50025049
for ruleIndex, clientMatch := range existingExportRules {
5003-
// For the policy, match the node IP addresses to the clientMatch to remove the matched items.
5004-
// Example:
5005-
// trident_pvc_123 is attached to node1 and node2. The policy is being unpublished from node1.
5006-
// node1 IP addresses [1.1.1.0, 1.1.1.1] node2 IP addresses [2.2.2.0, 2.2.2.2].
5007-
// export policy "trident_pvc_123" should have the export rules:
5008-
// index 1: "1.1.1.0"
5009-
// index 2: "1.1.1.1"
5010-
// index 3: "2.2.2.0"
5011-
// index 4: "2.2.2.2"
5012-
// When the clientMatch is the same as the node IP that export rule index will be added to the list of
5013-
// indexes to be removed. For this example indexes 1 and 2 will be removed.
5014-
5015-
// Legacy export policies created via ZAPI will have multiple clientMatch IPs for a node in a single rule.
5016-
// index 1: "1.1.1.0, 1.1.1.1"
5017-
// index 2: "2.2.2.0, 2.2.2.2"
5018-
// For this example, index 1 will be removed.
5019-
5020-
// Add a ruleIndex for deletion only if ALL the IPs in the clientMatch are in the list of IPs we are trying
5021-
// to delete
5022-
allMatch := true
5023-
for _, singleClientMatch := range strings.Split(clientMatch, ",") {
5024-
singleClientMatch = strings.TrimSpace(singleClientMatch)
5025-
if _, match := nodeIPRules[singleClientMatch]; !match {
5026-
allMatch = false
5050+
clientIPs := strings.Split(clientMatch, ",")
5051+
shouldKeepRule := false
5052+
5053+
// Check if ANY IP in this rule should be preserved (belongs to active nodes)
5054+
for _, singleClientIP := range clientIPs {
5055+
singleClientIP = strings.TrimSpace(singleClientIP)
5056+
if _, shouldKeep := activeNodeIPs[singleClientIP]; shouldKeep {
5057+
shouldKeepRule = true
50275058
break
50285059
}
50295060
}
5030-
if allMatch {
5061+
5062+
if !shouldKeepRule {
5063+
// This rule contains ONLY IPs that are no longer active - safe to remove
50315064
removeRuleIndexes = append(removeRuleIndexes, ruleIndex)
50325065
}
50335066
}
50345067

5035-
// Attempt to remove node IP addresses from export policy rules
5036-
Logc(ctx).WithField("removeRuleIndexes", removeRuleIndexes).Debug("Rule indexes to remove.")
5068+
// Remove rules that contain only inactive node IPs
50375069
for _, ruleIndex := range removeRuleIndexes {
5070+
ruleToRemove := existingExportRules[ruleIndex]
5071+
50385072
if err = clientAPI.ExportRuleDestroy(ctx, exportPolicy, ruleIndex); err != nil {
5039-
Logc(ctx).WithError(err).Error("Error deleting export policy rule.")
5073+
Logc(ctx).WithFields(LogFields{
5074+
"policyName": exportPolicy,
5075+
"ruleIndex": ruleIndex,
5076+
"ruleToRemove": ruleToRemove,
5077+
"error": err,
5078+
}).WithError(err).Error("Failed to remove export policy rule.")
5079+
} else {
5080+
Logc(ctx).WithFields(LogFields{
5081+
"policyName": exportPolicy,
5082+
"ruleIndex": ruleIndex,
5083+
"removedRule": ruleToRemove,
5084+
}).Debug("Removed export policy rule.")
50405085
}
50415086
}
50425087

5088+
// Get final rules after cleanup
5089+
finalExportRules, err := clientAPI.ExportRuleList(ctx, exportPolicy)
5090+
if err != nil {
5091+
Logc(ctx).WithFields(LogFields{
5092+
"policyName": exportPolicy,
5093+
"error": err,
5094+
}).WithError(err).Error("Could not list final export policy rules.")
5095+
} else {
5096+
Logc(ctx).WithFields(LogFields{
5097+
"policyName": exportPolicy,
5098+
"finalExportRules": finalExportRules,
5099+
"finalRuleCount": len(finalExportRules),
5100+
"originalRuleCount": len(existingExportRules),
5101+
"removedRuleCount": len(removeRuleIndexes),
5102+
}).Debug("Export policy cleanup completed.")
5103+
}
5104+
50435105
return nil
50445106
}
50455107

0 commit comments

Comments
 (0)