Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(attach): avoid creation of volumeattachment object #85

Merged
merged 1 commit into from
Apr 27, 2020

Conversation

pawanpraka1
Copy link
Contributor

@pawanpraka1 pawanpraka1 commented Apr 17, 2020

fixes :- #79

k8s is very slow in attaching the volumes when dealing with the
large number of volume attachment object.

(k8s issue kubernetes/kubernetes#84169)

The volumeattachment is not required for ZFSPV, so avoid creation
of attachment object, also removed the csi-attacher container as
this is also not needed as it acts on volumeattachment object.

k8s is very slow in attaching the volumes when dealing with the
large number of volume attachment object :

k8s issue kubernetes/kubernetes#84169).

Volumeattachment is a CR created just to tell the watcher of it
which is csi-attacher, that it has to call the Controller Publish/Unpublish grpc.
Which does all the tasks to attach the volumes to a node for example call to the
DigitalOcean Block Storage API service to attach a created volume to a specified node.
Since for ZFSPV, volume is already present locally, nothing needs to done in Controller
Publish/Unpublish, so it is good to remove them.

so avoiding creation of attachment object in this change, also removed the csi-attacher
container as this is also not needed as it acts on volumeattachment object.

Removed csi-cluster-driver-registrar container also as it is deprecated and not needed anymore.

We are using csidriver beta CRDs so minimum k8s version required is 1.14+.

Signed-off-by: Pawan pawan@mayadata.io

@pawanpraka1 pawanpraka1 added the bug Something isn't working. label Apr 17, 2020
@pawanpraka1 pawanpraka1 added this to the v0.6.1 milestone Apr 17, 2020
@pawanpraka1 pawanpraka1 requested a review from kmova April 17, 2020 10:07
@pawanpraka1 pawanpraka1 added the pr/hold-review hold the review. label Apr 17, 2020
@pawanpraka1 pawanpraka1 force-pushed the attach branch 4 times, most recently from 9a0b9a0 to dbb8b4f Compare April 20, 2020 11:23
@codecov-io
Copy link

codecov-io commented Apr 20, 2020

Codecov Report

Merging #85 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #85   +/-   ##
=======================================
  Coverage   23.57%   23.57%           
=======================================
  Files          14       14           
  Lines         475      475           
=======================================
  Hits          112      112           
  Misses        362      362           
  Partials        1        1           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d57976e...24e5de6. Read the comment docs.

upgrade/cleanup.sh Outdated Show resolved Hide resolved
k8s is very slow in attaching the volumes when dealing with the
large number of volume attachment object.

(k8s issue kubernetes/kubernetes#84169)

The volumeattachment is not required for ZFSPV, so avoid creation
of attachment object, also removed the csi-attacher container as
this is also not needed as it acts on volumeattachment object.

k8s is very slow in attaching the volumes when dealing with the
large number of volume attachment object :

k8s issue kubernetes/kubernetes#84169).

Volumeattachment is a CR created just to tell the watcher of it
which is csi-attacher, that it has to call the Controller Publish/Unpublish grpc.
Which does all the tasks to attach the volumes to a node for example call to the
DigitalOcean Block Storage API service to attach a created volume to a specified node.
Since for ZFSPV, volume is already present locally, nothing needs to done in Controller
Publish/Unpublish, so it is good to remove them.

so avoiding creation of attachment object in this change, also removed the csi-attacher
container as this is also not needed as it acts on volumeattachment object.

Removed csi-cluster-driver-registrar container also as it is deprecated and not needed anymore.

We are using csidriver beta CRDs so minimum k8s version required is 1.14+.

Signed-off-by: Pawan <pawan@mayadata.io>
Copy link
Contributor

@w3aman w3aman left a comment

Choose a reason for hiding this comment

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

/ changes are good and tested as well. good to merge

@pawanpraka1 pawanpraka1 added the pr/release-note PR has to be added in the release note. label Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working. pr/release-note PR has to be added in the release note.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants