search

ceph / ceph-csi

CSI driver for Ceph
Apache License 2.0
1.29k stars 551 forks source link

Refractor codebase (internal,cmd package only) #852

Open Madhu-1 opened 4 years ago

Madhu-1 commented 4 years ago

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] commented 4 years ago

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] commented 4 years ago

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

Madhu-1 commented 3 years ago
Madhu-1 commented 3 years ago

Reopening as planning to work on this one.

Madhu-1 commented 3 years ago

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

nixpanic commented 3 years ago

I would really like to see separate packages for:

nixpanic commented 3 years ago

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

humblec commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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:

humblec commented 3 years ago

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 commented 3 years ago

@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] commented 3 years ago

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.