Skip to content

Conversation

@suryag1201
Copy link

@suryag1201 suryag1201 commented Oct 30, 2025

Description

CSTACKEX-36 Attach Cluster/Zone and Grant Access

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?

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

if(ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) {
Map<String, String> getCloudStackVolumeMap = new HashMap<>();
getCloudStackVolumeMap.put(Constants.NAME, volumeVO.getPath());
getCloudStackVolumeMap.put(Constants.SVM_DOT_NAME, svmName);
Copy link
Author

Choose a reason for hiding this comment

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

check for wildcard in values

Copilot AI review requested due to automatic review settings November 3, 2025 16:20
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements functionality to attach ONTAP storage pools to clusters/zones and grant access to volumes for iSCSI protocol. The implementation creates initiator groups (igroups) during cluster/zone attachment and enables LUN mapping when granting volume access to hosts.

Key changes:

  • Implements access group (igroup) creation and retrieval for SAN storage
  • Adds LUN mapping/unmapping operations for volume access control
  • Implements attachCluster and attachZone methods to create igroups with host initiators
  • Adds grantAccess and revokeAccess implementations for volume access management

Reviewed Changes

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

Show a summary per file
File Description
Utility.java Added helper methods for creating access groups, generating igroup names, LUN names, and OS type mapping
Constants.java Added new constants for API endpoints, query parameters, and naming conventions
UnifiedSANStrategy.java Implemented methods for igroup and LUN map operations (create, get, enable/disable logical access)
UnifiedNASStrategy.java Updated method signatures to match interface changes
StorageStrategy.java Modified abstract method signatures to accept Map parameters and added public visibility
OntapPrimaryDatastoreProvider.java Fixed logger import to use correct Log4j interface
OntapPrimaryDatastoreLifecycle.java Implemented cluster/zone attachment logic with igroup creation and host protocol validation
SANFeignClient.java Updated Feign client method signatures for igroup and LUN map operations
OntapPrimaryDatastoreDriver.java Implemented grantAccess and revokeAccess methods for volume access control

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

Choose a reason for hiding this comment

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

lets take latest from Sandeep PR


private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, Host host) {
Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId());
StorageStrategy storageStrategy = utils.getStrategyByStoragePoolDetails(details);

Choose a reason for hiding this comment

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

this method has moved from utility class

@suryag1201 suryag1201 closed this Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants