Bugfix/cstackex 216 Public PR Review Comment: Igroup Name Length truncation#13585
Closed
suryag1201 wants to merge 127 commits into
Closed
Bugfix/cstackex 216 Public PR Review Comment: Igroup Name Length truncation#13585suryag1201 wants to merge 127 commits into
suryag1201 wants to merge 127 commits into
Conversation
… added EOF fixes + correcting license header
Initial primary storage pool plugin skeleton
…POJOs" This reverts commit fe0f752.
…POJOs" This reverts commit 28faca1.
Feignconfiguration and volume feignClient along with desired POJOs with cstack 28
* CSTACKEX-29 Cluster, SVM and Aggr Feign Client * CSTACKEX-29 Change the endpoint method name in feign client * CSTACKEX-29 Make the alignment proper * CSTACKEX-29 Added License Info * CSTACKEX-29 Resolve Review Comments * CSTACKEX-29 Remove Component Annotation from datastoredriverclass * CSTACKEX-29 Resolve Style check issues * CSTACKEX-29 Resolve ALL Style issues * CSTACKEX-29 Resolve Precommits Issues * CSTACKEX-29 Added Method comments and change the ontap response class name --------- Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
* CSTACKEX-31 NAS and Job Feign Client and POJOs * CSTACKEX-31 Fixed Checks Issues * CSTACKEX-31 Resolve Review Comments * CSTACKEX-31 Resolve Review Comments * CSTACKEX-31 Resolve Review Comments * CSTACKEX-31 Added Aggr and size to volume model * CSTACKEX-31 Change the export policy endpoint path * CSTACKEX-31 Fixed check styles --------- Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
* CSTACKEX-30 SAN Feign Client * CSTACKEX-30 Fixed check style issues * CSTACKEX-30 Fixed review comments --------- Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
* CSTACKEX-7: ONTAP Primary storage pool --------- Co-authored-by: Locharla, Sandeep <Sandeep.Locharla@netapp.com>
CSTACKEX-34: Upgrade to framework classes design
* CSTACKEX-35 Create Async * CSTACKEX-35 Added Null and empty check * CSTACKEX-35 Resolved review comments * CSTACKEX-35 Removed Type Casting for logger --------- Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
* CSTACKEX-1: Feign changes and fixes for getting storage pool creation to work * CSTACKEX-01: Create Primary Storage pool changes with working code * CSTACKEX-01: Addressed all review comments and updated some code * CSTACKEX-01: Made some changes to fix some errors seen during testing * CSTACKEX-01: Addressed additional comments --------- Co-authored-by: Locharla, Sandeep <Sandeep.Locharla@netapp.com>
…ISCSI protocol is not enabled at the SVM(#61) ### Description This PR... Primary storage-pool is not failing even if NFS3/ISCSI protocol is not enabled at the storage VM <!-- For new features, provide link to FS, dev ML discussion etc. --> <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. --> <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged --> <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" --> <!-- Fixes: # --> <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] 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 - [x] Minor - [ ] Trivial ### Screenshots (if appropriate): <img width="1707" height="848" alt="Screenshot 2026-05-24 at 8 45 02 PM" src="https://github.com/user-attachments/assets/9dbc8386-29b4-4ce2-b863-6933c1c0a8d6" /> <img width="1697" height="937" alt="Screenshot 2026-05-24 at 8 44 46 PM" src="https://github.com/user-attachments/assets/5c8abce4-0343-453e-a980-bd7124034083" /> ### How Has This Been Tested? <!-- Please describe in detail how you tested your changes. --> <!-- Include details of your testing environment, and the tests you ran to --> #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document --> --------- Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
### Description Storage Pool Creation is failing "message": "Unexpected argument \"svm.iscsiEnabled\" <!-- For new features, provide link to FS, dev ML discussion etc. --> <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. --> <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged --> <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" --> <!-- Fixes: # --> <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] 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 - [x] Major - [ ] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [ ] Major - [ ] Minor - [ ] Trivial ### Screenshots (if appropriate): <img width="1471" height="145" alt="image" src="https://github.com/user-attachments/assets/f894ec2d-0769-4f6d-8ad3-13b3e6b2b5cc" /> <img width="2292" height="1672" alt="image" src="https://github.com/user-attachments/assets/403bce30-c9a2-44ec-a783-cf65c1fa2bd9" /> ### How Has This Been Tested? 1- Disable the NFS and ISCSI protocol on svm and checked the storage pool creation: It threw the appropriate error on UI, below is the screenshot 2- Enable the NFS and ISCSI protocol on svm: Storage pool creation went fine #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document --> Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
CSTACKEX-191: logical access methods can have return type as primitive
…etApp local changes, not meant for upstream merge.
CSTACKEX-175: adding initial changes for CI related workflows
#65) ### Description This PR... CSTACKEX-176: Storage Pool Backend by ONTAP FlexVolume is not Thin Provisioned CSTACKEX-172: VM create failure due to aggrs not available and UI says "Unable to orchestrate the start of VM instance" CSTACKEX-199: Error message when cluster password is wrong while creating primary storage pool is not user intuitive CSTACKEX-181: Multiple host with same hostname will not follow per host igroup design ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] 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): <img width="1125" height="861" alt="Screenshot 2026-06-18 at 2 33 52 PM" src="https://github.com/user-attachments/assets/e2f5bc20-0109-4b5a-a720-dbd3d39ee254" /> <img width="679" height="280" alt="image" src="https://github.com/user-attachments/assets/b7dac4f1-1a6b-4234-bcd5-47c5efe8f798" /> ### How Has This Been Tested? 1- Create a storage pool with invalid ontap cred - throw the error 2- Create a storage pool with valid ontap cred - success 3- Verify Ontap Volume - thin provisioned 4- VM instance VM1 creation - suceess and igroup created with cs_svmName_hostUUID 5- VM instance VM2 creation - reused the same igroup 6- VM instance VM1 and VM2 delete - igroup also got deleted #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document --> --------- Co-authored-by: Gupta, Surya <Surya.Gupta@netapp.com>
…loudstack and ontap supported scenarios
CSTACKEX-209: updating allowed snapshot name size to 255 as per the c…
CSTACKEX-209: addressing copilot comments
### Description This PR... <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. --> <!-- For new features, provide link to FS, dev ML discussion etc. --> <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. --> <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged --> <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" --> <!-- Fixes: # --> <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] 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): <img width="1215" height="117" alt="image" src="https://github.com/user-attachments/assets/0a677980-140d-4904-b196-5a732586a222" /> <img width="1228" height="393" alt="image" src="https://github.com/user-attachments/assets/d5715e87-88d0-4e6e-85e9-2c4b913dfa5f" /> ### How Has This Been Tested? <!-- Please describe in detail how you tested your changes. --> <!-- Include details of your testing environment, and the tests you ran to --> #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document --> --------- Co-authored-by: Srivastava, Piyush <Piyush.Srivastava@netapp.com>
This reverts commit 1e9de6c.
…ross multiple flexvolumes (#74) ### Description This PR... <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. --> <!-- For new features, provide link to FS, dev ML discussion etc. --> <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. --> <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged --> <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" --> <!-- Fixes: # --> <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### 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) - [x] 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 - [x] Major - [ ] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [x] Major - [ ] Minor - [ ] Trivial ### Screenshots (if appropriate): ### How Has This Been Tested? <!-- Please describe in detail how you tested your changes. --> <!-- Include details of your testing environment, and the tests you ran to --> #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request extends Apache CloudStack’s NetApp ONTAP integration across UI and storage subsystems, while also adding downstream-only CI tooling under private-cicd/. It aims to reduce ONTAP igroup collision risk (by changing igroup naming) and improves ONTAP-managed snapshot/attach flows and KVM iSCSI handling.
Changes:
- Add/extend “NetApp ONTAP” primary storage support in the UI (zone wizard + add primary storage form) and i18n labels.
- Update server-side snapshot / attach-volume logic for ONTAP managed storage (strategy selection, archiving behavior, refreshed volume metadata).
- Add ONTAP plugin tests/models/clients and introduce downstream Jenkins tooling under
private-cicd/.
Reviewed changes
Copilot reviewed 77 out of 77 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/views/infra/zone/ZoneWizardLaunchZone.vue | Adds NetApp ONTAP primary storage parameters during zone wizard storage pool creation. |
| ui/src/views/infra/zone/ZoneWizardAddResources.vue | Adds ONTAP-specific fields and protocol options to the zone wizard resource step. |
| ui/src/views/infra/AddPrimaryStorage.vue | Adds ONTAP provider UI fields and request parameter mapping for primary storage creation. |
| ui/public/locales/en.json | Adds ONTAP label/tooltip translations. |
| test/integration/plugins/ontap/README.md | Placeholder README for future Marvin integration tests for ONTAP plugin. |
| server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java | Adds ONTAP-specific error when memory snapshots are requested but unsupported. |
| server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java | Reloads volume after grantAccess to avoid stale managed-volume metadata (ONTAP). |
| server/src/main/java/com/cloud/storage/StorageManagerImpl.java | Skips capacity byte updates for ONTAP provider in specific pool update flow. |
| server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java | Avoids secondary archiving for ONTAP managed PRIMARY snapshots; adds helper + logging. |
| private-cicd/scripts/validate-local.sh | Adds local validation script for downstream CI tree (bash + YAML checks). |
| private-cicd/scripts/setup-ipmitool-wrapper.sh | Adds optional ipmitool wrapper installer for downstream CI agents. |
| private-cicd/scripts/mvn-ontap-fast.sh | Adds ONTAP plugin-only fast Maven build/test script for downstream CI. |
| private-cicd/scripts/mvn-full.sh | Adds downstream full Maven build script. |
| private-cicd/scripts/marvin-run.sh | Adds stub for future Marvin execution wiring. |
| private-cicd/scripts/install-build-deps-ubuntu.sh | Adds downstream Ubuntu/Debian dependency installer for CI agents. |
| private-cicd/README.md | Documents downstream-only CI/CD usage and phases. |
| private-cicd/marvin/zones/README.md | Documents downstream zone config templates layout. |
| private-cicd/marvin/zones/ontap-simulator.cfg.example | Adds example ONTAP Marvin zone config template. |
| private-cicd/marvin/README.md | Documents downstream Marvin assets. |
| private-cicd/marvin/bundles.txt | Defines downstream Marvin bundle lists for ONTAP tests. |
| private-cicd/Jenkinsfile | Adds downstream Jenkins pipeline with phased build modes. |
| private-cicd/docs/IMPLEMENTATION-PHASES.md | Documents phased CI rollout plan for downstream Jenkins pipeline. |
| private-cicd/docs/FOLDER-LAYOUT.md | Documents downstream CI folder structure. |
| private-cicd/docs/BRANCH-STRATEGY.md | Documents branching strategy to keep private-cicd/ out of upstream PRs. |
| private-cicd/docker/Dockerfile.agent | Adds downstream build-agent Dockerfile. |
| private-cicd/config/qa.yaml.example | Adds downstream config template for fork/branch overrides. |
| private-cicd/config/marvin.yaml | Adds downstream Marvin phase configuration file. |
| private-cicd/config/defaults.yaml | Adds downstream default config (CloudStack URL/branch + profiles). |
| private-cicd/config/build-fast.yaml | Adds downstream ONTAP fast-build configuration and change-detection stubs. |
| private-cicd/.gitignore | Ignores downstream secret-bearing overrides (qa.yaml, *.cfg). |
| plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java | Adds tests for new igroup naming and truncation behavior. |
| plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java | Adds unit tests for UnifiedNASStrategy behavior with mocked ONTAP clients. |
| plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProviderTest.java | Updates provider-name assertion to use DataStoreProvider constant. |
| plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java | Adds broad unit test coverage for ONTAP primary datastore driver flows. |
| plugins/storage/volume/ontap/src/main/resources/META-INF/cloudstack/storage-volume-ontap/spring-storage-volume-ontap-context.xml | Registers ONTAP VM snapshot strategy bean. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java | Refactors helpers (igroup naming, snapshot naming, strategy creation, volume request creation). |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java | Aligns constants with DataStoreProvider, adds new ONTAP keys and limits. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java | Implements unified NFS strategy create/delete/get + export policy handling adjustments. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/SANStrategy.java | Adds helper to validate initiator presence and new abstract mapping method. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/CloudStackVolume.java | Expands model fields used in snapshot/volume workflows. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java | Replaces PrimaryDataStoreInfo reference with storagePoolId. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/StorageProviderFactory.java | Decodes ONTAP password (base64) and wires unified strategies. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java | Uses canonical ONTAP provider-name constant. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java | Adds KVM-only guard and passes pool details to ModifyStoragePoolCommand. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java | Refactors initialize/validation; removes URL-parsing and tightens required ONTAP detail keys. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeConcise.java | Adds concise volume reference model used by snapshot/CG responses. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Volume.java | Adds guarantee/provisioning model fields. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Svm.java | Adjusts JSON mapping for protocol enabled flags via nested setters. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/SnapshotFileRestoreRequest.java | Adds model for file restore request. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/OntapStorage.java | Renames managementLIF→storageIP and simplifies constructor fields. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/LunRestoreRequest.java | Adds model for LUN restore API. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Igroup.java | Improves enum JSON serialization/deserialization. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FlexVolSnapshot.java | Adds model for FlexVol snapshot operations. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileInfo.java | Removes OffsetDateTime fields from model output/printing. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ConsistencyGroupVolumeProvisioningOptions.java | Adds model for CG member provisioning options. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ConsistencyGroupVolume.java | Adds model for CG volume members. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ConsistencyGroupSnapshot.java | Adds model for CG snapshot lifecycle. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/ConsistencyGroup.java | Adds model for consistency groups. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/CliSnapshotRestoreRequest.java | Adds model for CLI passthrough restore-file request. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java | Adds feign client for FlexVol + CG snapshot operations and CLI restore API. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java | Adds restoreLun API. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java | Adjusts file GET to request metadata. |
| plugins/storage/volume/ontap/pom.xml | Adds test dependency byte-buddy-agent and adds snapshot module dependency. |
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java | Makes iSCSI login/node-create idempotent and adds rescan + safer disconnect behavior. |
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java | Adds cleanup helper for leftover deltas and improved exception handling. |
| engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java | Refreshes volume and sets managed store target after grantAccess (ONTAP). |
| engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java | Excludes ONTAP managed pools from generic KVM file-based VM snapshot strategy; improves failure details. |
| engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreProvider.java | Introduces canonical ONTAP_PLUGIN_NAME constant. |
| .github/CODEOWNERS | Updates CODEOWNERS for ONTAP plugin path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+151
to
160
| public static String getIgroupName(String svmName, String hostUuid) { | ||
| //Igroup name format: cs_hostUuid_svmName | ||
| String sanitizedHostUuid = hostUuid.replaceAll("[^a-zA-Z0-9_-]", "_"); | ||
| String igroupName = OntapStorageConstants.CS + OntapStorageConstants.UNDERSCORE + sanitizedHostUuid + OntapStorageConstants.UNDERSCORE + svmName; | ||
| // ONTAP igroup names are limited to 96 characters; truncate if longer. | ||
| if (igroupName.length() > OntapStorageConstants.IGROUP_NAME_MAX_LENGTH) { | ||
| igroupName = igroupName.substring(0, OntapStorageConstants.IGROUP_NAME_MAX_LENGTH); | ||
| } | ||
| return igroupName; | ||
| } |
Comment on lines
39
to
+49
| public static StorageStrategy getStrategy(OntapStorage ontapStorage) { | ||
| ProtocolType protocol = ontapStorage.getProtocol(); | ||
| logger.info("Initializing StorageProviderFactory with protocol: " + protocol); | ||
| String decodedPassword = new String(java.util.Base64.getDecoder().decode(ontapStorage.getPassword()), StandardCharsets.UTF_8); | ||
| ontapStorage = new OntapStorage( | ||
| ontapStorage.getUsername(), | ||
| decodedPassword, | ||
| ontapStorage.getStorageIP(), | ||
| ontapStorage.getSvmName(), | ||
| ontapStorage.getSize(), | ||
| protocol); |
Comment on lines
+1608
to
+1612
| params['details[0].storageIP'] = this.prefillContent.ontapIP | ||
| params['details[0].username'] = this.prefillContent.ontapUsername | ||
| params['details[0].password'] = btoa(this.prefillContent.ontapPassword) | ||
| params['details[0].svmName'] = this.prefillContent.ontapSvmName | ||
| params['details[0].protocol'] = this.prefillContent.primaryStorageProtocol |
Comment on lines
+938
to
946
| } else if (values.provider === 'NetApp ONTAP') { | ||
| params['details[0].storageIP'] = values.ontapIP | ||
| params['details[0].username'] = values.ontapUsername | ||
| params['details[0].password'] = btoa(values.ontapPassword) | ||
| params['details[0].svmName'] = values.ontapSvmName | ||
| params['details[0].protocol'] = values.protocol | ||
| values.managed = true | ||
| url = this.ontapURL(values.ontapIP) | ||
| } |
Comment on lines
+255
to
264
| // Validate existing entries (reject unexpected keys, empty values) | ||
| for (Map.Entry<String, String> e : details.entrySet()) { | ||
| String key = e.getKey(); | ||
| String val = e.getValue(); | ||
| if (!allowedKeys.contains(key)) { | ||
| if (!requiredKeys.contains(key)) { | ||
| throw new CloudRuntimeException("Unexpected ONTAP detail key in URL: " + key); | ||
| } | ||
| if (StringUtils.isBlank(val)) { | ||
| throw new CloudRuntimeException("ONTAP primary storage creation failed, empty detail: " + key); | ||
| } |
Comment on lines
+86
to
+90
| return cloudstackVolume; | ||
| }catch (Exception e) { | ||
| logger.error("createCloudStackVolume: error occured " + e); | ||
| throw new CloudRuntimeException(e); | ||
| } |
Comment on lines
+104
to
+112
| if (answer == null || !answer.getResult()) { | ||
| String errMsg = answer != null ? answer.getDetails() : "Failed to delete qcow2 on KVM host"; | ||
| logger.error("deleteCloudStackVolume: " + errMsg); | ||
| throw new CloudRuntimeException(errMsg); | ||
| } | ||
| } catch (Exception e) { | ||
| logger.error("deleteCloudStackVolume: error occured " + e); | ||
| throw new CloudRuntimeException(e); | ||
| } |
Comment on lines
+171
to
+175
| String msg = result.toLowerCase(); | ||
| if (msg.contains("already exists") || msg.contains("database exists") || msg.contains("exists")) { | ||
| logger.debug("iSCSI node already exists for target {}, proceeding", volumeUuid); | ||
| return true; | ||
| } |
Comment on lines
+32
to
+36
| /** | ||
| * Primary storage provider name for NetApp ONTAP ({@code OntapPrimaryDatastoreProvider#getName}). | ||
| * Single canonical value; use this instead of duplicating the string across modules. | ||
| */ | ||
| String ONTAP_PLUGIN_NAME = "NetApp ONTAP"; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
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- Delete All VMs - Igroup got deleted
How did you try to break this feature and the system with this change?