CSTACKEX-34: Upgrade to framework classes design by rajiv-jain-netapp · Pull Request #13 · NetApp/cloudstack

rajiv-jain-netapp · 2025-10-28T06:14:16Z

Description

Updated the class structure for the framwork classes
This PR...

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

Screenshots (if appropriate):

How Has This Been Tested?

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

piyush5netapp · 2025-10-28T07:07:01Z

...torage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

We can have model where we have nested hierarchy of the file and volume model combined instead of map. This will help us know what fields are required for what operation from readability perspective.

suryag1201 · 2025-10-28T09:26:07Z

...torage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

OntapStorage also has protocol, we can remove this

suryag1201 · 2025-10-28T09:26:30Z

...torage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

as per discussion, we should pass those new models as a param also instead of map in required methods

suryag1201 · 2025-10-28T09:27:41Z

...rage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java

please specify as private because we already have a getter with public

suryag1201 · 2025-10-28T09:27:55Z

...volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/CloudStackVolume.java

same here, define as private

suryag1201 · 2025-10-28T09:29:02Z

...rage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java

This should be ExportPolicy object

rajiv-jain-netapp

Incorporated review comments

rajiv-jain-netapp · 2025-10-28T10:12:47Z

...torage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

rajiv-jain-netapp · 2025-10-28T10:13:07Z

...torage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

rajiv-jain-netapp · 2025-10-28T10:13:45Z

...torage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

sandeeplocharla · 2025-10-28T13:08:54Z

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java

-        ISCSI
-    }

    public static final String NFS = "nfs";


These 2 constants can be deleted

sandeeplocharla

Please see if the constants can be removed as part of the next PR

CSTACKEX-34: Upgrade to framework classes design

973f5e2

rajiv-jain-netapp requested review from piyush5netapp, sandeeplocharla and suryag1201 October 28, 2025 06:15

piyush5netapp reviewed Oct 28, 2025

View reviewed changes

rajiv-jain-netapp added 3 commits October 28, 2025 14:33

CSTACKEX-34: incorporating the review comments

686a892

CSTACKEX-34: transient changes to the review comments

edfcdde

CSTACKEX-34: Unable to get checkstyle pass hence fixing this as well

73eb9f5

suryag1201 reviewed Oct 28, 2025

View reviewed changes

...volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/CloudStackVolume.java Outdated

Copy link

suryag1201 Oct 28, 2025

Choose a reason for hiding this comment

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

same here, define as private

suryag1201 reviewed Oct 28, 2025

View reviewed changes

...rage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java Outdated

Copy link

suryag1201 Oct 28, 2025

Choose a reason for hiding this comment

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

This should be ExportPolicy object

rajiv-jain-netapp added 3 commits October 28, 2025 15:18

CSTACKEX-34: further review comments incorporations

465fffe

CSTACKEX-34: addressing checkstyle issues

3d6bd91

CSTACKEX-34: fix checksyle issues

5815ebd

rajiv-jain-netapp commented Oct 28, 2025

View reviewed changes

sandeeplocharla reviewed Oct 28, 2025

View reviewed changes

sandeeplocharla approved these changes Oct 28, 2025

View reviewed changes

piyush5netapp approved these changes Oct 28, 2025

View reviewed changes

rajiv-jain-netapp merged commit 618f957 into main Oct 28, 2025
16 checks passed

Conversation

rajiv-jain-netapp commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajiv-jain-netapp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandeeplocharla left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

rajiv-jain-netapp commented Oct 28, 2025 •

edited

Loading