ceph-csi
ceph-csi copied to clipboard
ceph
Enable AnnotateFSResize featuregate
- [x] Helm deployment have been updated to have this featuregate
- [x] Deployment manifests are also updated to have this featuregate enabled ~~- E2E added for RBD for validation of the annotation~~
AnnotateFsResize store current size of pvc in pv's annotation, so as if pvc is deleted while expansion was pending on the node, the size of pvc can be restored to old value.
Signed-off-by: Humble Chirammal [email protected]
The commitlint still fails ?
fatal: unsafe repository ('/go/src/github.com/ceph/ceph-csi' is owned by someone else)
To add an exception for this directory, call:
git config --global --add safe.directory /go/src/github.com/ceph/ceph-csi
make: *** [Makefile:153: commitlint] Error 128
make: *** [Makefile:201: containerized-test] Error 2
Error: Process completed with exit code 2.
@Madhu-1 even after https://github.com/ceph/ceph-csi/pull/3102 ?
The commitlint still fails ?
fatal: unsafe repository ('/go/src/github.com/ceph/ceph-csi' is owned by someone else) To add an exception for this directory, call: git config --global --add safe.directory /go/src/github.com/ceph/ceph-csi make: *** [Makefile:153: commitlint] Error 128 make: *** [Makefile:201: containerized-test] Error 2 Error: Process completed with exit code 2.@Madhu-1 even after #3102 ?
@humblec i don't think you have rebased this PR on top of my changes. https://github.com/ceph/ceph-csi/runs/6350701526?check_suite_focus=true#step:3:35
![mergify[bot] avatar](https://avatars.githubusercontent.com/in/10562?v=4 "Published by a pub.dev verified publisher"){kind=small}
the commit messages, and maybe comments in the code should indicate what version of Kubernetes is needed for this. It would also be good to reference any documentation/KEP in the commit message(s).
I have added the above details to commits.. ptal. thanks @nixpanic
Is it needed that this gets tested for each CSI-driver? Should it not be part of the k8s-e2e-external-storage suite, or some other Kubernetes specific test?
Is it needed that this gets tested for each CSI-driver? Should it not be part of the k8s-e2e-external-storage suite, or some other Kubernetes specific test?
not necessarily for all CSI drivers. For ceph csi, we are enabling it with the sidecar , so adding a test as well to make sure, we get the expected string in the annotation. @nixpanic , ptal.. thanks.
/retest ci/centos/mini-e2e-helm/k8s-1.21 /retest ci/centos/mini-e2e-helm/k8s-1.23
Is it needed that this gets tested for each CSI-driver? Should it not be part of the k8s-e2e-external-storage suite, or some other Kubernetes specific test?
I Agree here. this is nothing to do with the CSI driver. any CSI driver which supports Resize will be tested for this one. IMO k8s-e2e-external-storage is the best place for this one.
Is it needed that this gets tested for each CSI-driver? Should it not be part of the k8s-e2e-external-storage suite, or some other Kubernetes specific test?
I Agree here. this is nothing to do with the CSI driver. any CSI driver which supports Resize will be tested for this one. IMO k8s-e2e-external-storage is the best place for this one.
@Madhu-1 @nixpanic is it that, just enable the featuregate and drop the sepcific e2e test in this PR ?
Is it needed that this gets tested for each CSI-driver? Should it not be part of the k8s-e2e-external-storage suite, or some other Kubernetes specific test?
I Agree here. this is nothing to do with the CSI driver. any CSI driver which supports Resize will be tested for this one. IMO k8s-e2e-external-storage is the best place for this one.
@Madhu-1 @nixpanic is it that, just enable the featuregate and drop the sepcific e2e test in this PR ?
Yes please, that should be enough for us
Is it needed that this gets tested for each CSI-driver? Should it not be part of the k8s-e2e-external-storage suite, or some other Kubernetes specific test?
I Agree here. this is nothing to do with the CSI driver. any CSI driver which supports Resize will be tested for this one. IMO k8s-e2e-external-storage is the best place for this one.
@Madhu-1 @nixpanic is it that, just enable the featuregate and drop the sepcific e2e test in this PR ?
Yes please, that should be enough for us
@Madhu-1 done.. ptal.. thanks
changes LGTM, please adjust the commit message to be the same in both commits. and update the PR description.
@Madhu-1 unifiied the commit header..
On Tue, May 17, 2022 at 12:30:10AM -0700, Humble Devassy Chirammal wrote:
Is it needed that this gets tested for each CSI-driver? Should it not be part of the k8s-e2e-external-storage suite, or some other Kubernetes specific test?
I Agree here. this is nothing to do with the CSI driver. any CSI driver which supports Resize will be tested for this one. IMO k8s-e2e-external-storage is the best place for this one.
@Madhu-1 @nixpanic is it that, just enable the featuregate and drop the sepcific e2e test in this PR ?
Yes, I guess so. And validate that the testing is included in the k8s-e2e-external-storage suite.
On Tue, May 17, 2022 at 12:30:10AM -0700, Humble Devassy Chirammal wrote: > > Is it needed that this gets tested for each CSI-driver? Should it not be part of the k8s-e2e-external-storage suite, or some other Kubernetes specific test? > > I Agree here. this is nothing to do with the CSI driver. any CSI driver which supports Resize will be tested for this one. IMO k8s-e2e-external-storage is the best place for this one. @Madhu-1 @nixpanic is it that, just enable the featuregate and drop the sepcific e2e test in this PR ? Yes, I guess so. And validate that the testing is included in the k8s-e2e-external-storage suite.
Sure.. will do.
This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
This pull request has been automatically closed due to inactivity. Please re-open if these changes are still required.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions.
This pull request has been automatically closed due to inactivity. Please re-open if these changes are still required.