Refractor codebase (internal,cmd package only)

[Madhu-1]

trafficstars

currently, it's difficult to write a Unit testing for the current code base, if we move all cephfs and rbd and rados function to a new interface and methods it will allow us to mock the things are helping us to write the unit testing for the current codebase

the functions which can be moved to the different interfaces and to seperate files also example

GetOMapValue()
getImageInfo()
createImage()
deleteImage()
etc...

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically closed due to inactivity. Please re-open if this still requires investigation.

[Madhu-1]

• Move files to a specific folder based on the operations
• Break larger functions in the smaller functions
• Implement interfaces for all the functions/methods which does go-ceph/CLI calls
  • This helps in adding more unit testings
• Change error implementations
  • Internal functions should not return GRPC error codes/messages

[Madhu-1]

Reopening as planning to work on this one.

[Madhu-1]

@humblec @nixpanic @pkalever @Yuggupta27 @yati1998 @Rakshith-R please add your suggestions here so that we can consider for refactoring

[nixpanic]

I would really like to see separate packages for:

• k8s usage: helper functions that do not return k8s objects directly (1st step in moving to a k8s sidecar)
• provisioner and node-plugin: separated from internal/rbd and internal/cephfs API
• (future) cmd/tool: using internal/rbd and internal/cephfs API

[nixpanic]

And of course, the KMS API should move to its own internal/kms package too. With each provider placed in internal/kms/<provider>.

[humblec]

@Madhu-1 as I brought up in earlier meetings and had some discussions on, a couple of items I have been working on this regard are : [1] Refactoring operational framework and [2] e2e refactoring:

The first should help us to address transactional chain for the operation and help us to improve each transaction we have in place..etc, the design https://hackmd.io/TvrswhUUQDGwVk1vFLY7Pw The second should help us to have more flexible e2e tests.. #2178

so apart from the points raised above, I would like to do these 2 things which should help us to improve the overall code base :+1:

[Madhu-1]

Yes, agree am not tracking E2E on this one. so won't makes any changes to E2E as part of this issue. Updated the Issue title

[humblec]

Yes, agree am not tracking E2E on this one. so won't makes any changes to E2E as part of this issue. Updated the Issue title

Ok. If thats the case operational framwork improvement is something which I have been working on which could be part of this larger issue.

[humblec]

Yes, agree am not tracking E2E on this one. so won't makes any changes to E2E as part of this issue. Updated the Issue title

Ok. If thats the case operational framwork improvement is something which I have been working on which could be part of this larger issue.

@Madhu-1 may be I can open a seperate issue for that and add this as tracker ?

[Madhu-1]

Yes, agree am not tracking E2E on this one. so won't makes any changes to E2E as part of this issue. Updated the Issue title

Ok. If thats the case operational framwork improvement is something which I have been working on which could be part of this larger issue.

Do you have any draft doc or idea on which area you are working on. Please post-draft PR or update this issue with more details so that we can avoid duplication of work.

[Madhu-1]

Yes, agree am not tracking E2E on this one. so won't makes any changes to E2E as part of this issue. Updated the Issue title

Ok. If thats the case operational framwork improvement is something which I have been working on which could be part of this larger issue.

@Madhu-1 may be I can open a seperate issue for that and add this as tracker ?

sounds good to open separate issue for E2E

[humblec]

Yes, agree am not tracking E2E on this one. so won't makes any changes to E2E as part of this issue. Updated the Issue title

Ok. If thats the case operational framwork improvement is something which I have been working on which could be part of this larger issue.

Do you have any draft doc or idea on which area you are working on. Please post-draft PR or update this issue with more details so that we can avoid duplication of work.

sure , this was the draft design I started sometime back https://hackmd.io/TvrswhUUQDGwVk1vFLY7Pw . I will open an issue and add more details.

[pkalever]

I would really like to see separate packages for:

• k8s usage: helper functions that do not return k8s objects directly (1st step in moving to a k8s sidecar)

Right! We need to carve out utility functions that operate based on the container orchestration engine and return common ceph-csi understandable data formats.

We need a mechanism to identify CO, is this something that we want our users to specify as a flag?

Also, it would be nice to start listing some of the functions that need to get benefited out of this effort, example:

• NewK8sClient()
• k8sGetNodeLabels()
• getSecret()
• getSecrets()
• runVolumeHealer()
• genVolFromVolID()
• getKMSConfigMap()
• fetchEncryptionPassphrase()
• getServiceAccount()
• getToken()
• getCertificate()
• parseTenantConfig() and
• so on

[humblec]

One other thought I have here is, moving the utility functions to a common library and take it outside the internal package hierarchy. #2437

[Yuggupta27]

@humblec @nixpanic @pkalever @Yuggupta27 @yati1998 @Rakshith-R please add your suggestions here so that we can consider for refactoring

With refactoring, we will also be able to revisit returning correct errors based on the function call level and where it lies in the granular request chain (Some gRPC level errors are returned even in some util functions currently)

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.