[CSTACKEX-216] Review Comments for Igroup Name Length

[suryag1201]

Description

Issue: "getIgroupName() truncates the full "cs__" string as a whole. If svmName is long, this can truncate away most or all of the hostUuid portion, increasing the chance of igroup name collisions across different hosts (and also throws a NullPointerException if hostUuid is null). Consider validating inputs and truncating svmName first so the host UUID remains intact (or, if hostUuid itself is too long, truncate only the UUID)."

Fix: In typical deployments, truncation likely never triggers bcz total length will remain less than 96. But to avoid any issues for future where svm limits get exceeded, will change the igroup format to cs_hostUuid_svmName

Note: This is a breaking change for existing deployments. Igroups already on ONTAP use the old format (cs_svm1_hostname). After this change, CloudStack will look for cs__svm1 and may not find existing igroup

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)
• Build/CI
• Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

Screenshots (if appropriate):

How Has This Been Tested?

1- Created 1st VM on ISCSI Storage pool - Igroup got create with new name format attached a screen shot and lun got mapped
2- Create 2nd VM on same storage pool - Igroup got reused and lun got mapped
3- Storage Pool to enter maintenance mode - All Luns got unmapped and igroup also got deleted
4- Storage Pool to cancel maintenance mode - New Igroup got created with same name format and All Luns got mapped again.
5-Create a new CloudStack volume and attched to the instance - used the existing igroup and mapped the lun
6- Delete All VMs - Igroup got deleted

How did you try to break this feature and the system with this change?

[Copilot]

Pull request overview

This PR updates the ONTAP igroup naming logic (and its unit tests) to change the generated name format, likely to better handle the 96-character igroup name limit by placing the host UUID earlier in the string.

Changes:

• Changed igroup name format from cs_<svmName>_<hostUuid> to cs_<hostUuid>_<svmName> in OntapStorageUtils#getIgroupName.
• Updated OntapStorageUtilsTest expected values and length-calculation comments to match the new format.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java	Changes igroup name construction order and updates the in-code format comment.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java	Updates unit tests to assert the new igroup naming format and related length assumptions.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[suryag1201]

we will try to push this change in 4.23 so that we will not see this issue during upgrade

[suryag1201]

For normal CloudStack UUIDs (hex + hyphens), sanitization is a no-op, so 550e8400-e29b-41d4-a716-446655440000 stays as-is. It only matters if the input has characters outside [a-zA-Z0-9_-].