Code Monkey home page Code Monkey logo

Comments (29)

alex-berger avatar alex-berger commented on August 30, 2024 27

Any news on this, this is really annoying as there are so many opportunities where nodes are deleted:

  • by the cluster autoscaler
  • during cluster upgrade
  • ...
    And then we always have to manually fix dozens of pods which cannot start as they cannot attach their volumes.

from external-attacher.

gkaskonas avatar gkaskonas commented on August 30, 2024 11

Facing the same issue with the latest EKS and ebs csi controller

from external-attacher.

alex-berger avatar alex-berger commented on August 30, 2024 5

@msau42 this happens every now and then in our clusters and 6 minutes is a long delay in a world with 99.95% and more uptime SLAs. Also note, that we already drain the nodes and still it might happen that a node terminates immediately without proper draining.
That's reality and not just theory, and we have to deal with it. That's why we need resilient and self healing controllers, which make sure the system recovers from such error automatically within reasonable time.
After all this is the reason why everybody is moving to kubernetes. If we still want to live on assumptions and fix everything manually whenever our overly optimistic assumptions fail, then we don't need to maintain complex kubernetes systems 😁.

from external-attacher.

mitchellmaler avatar mitchellmaler commented on August 30, 2024 4

Still ran into this running Kubernetes v1.21.3. The node was deleted and the VolumeAttachment was still around specifying the old node name.

from external-attacher.

spriya-m avatar spriya-m commented on August 30, 2024 4

Facing the issue with Kubernetes v1.24 as well.

from external-attacher.

karunsiri avatar karunsiri commented on August 30, 2024 2

Still hitting this with EKS v1.29.

from external-attacher.

fejta-bot avatar fejta-bot commented on August 30, 2024 1

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

from external-attacher.

galgross avatar galgross commented on August 30, 2024 1

1.21.

facing the same issue in k8s 1.21.5

from external-attacher.

avivgold098 avatar avivgold098 commented on August 30, 2024 1

@jsafrane what do you think about that issue? My organization can help with the implementation

from external-attacher.

msau42 avatar msau42 commented on August 30, 2024

cc @jsafrane

from external-attacher.

msau42 avatar msau42 commented on August 30, 2024

/kind bug

from external-attacher.

jsafrane avatar jsafrane commented on August 30, 2024

What should happen is:

  1. ListVolumes() shows that volume is not attached to the node anymore
  2. We actually mark VolumeAttachment.status.attached as detached

The volume might be already attaching. A/D controller does not know about that.

  1. In k/k AD controller, VerifyVolumesAttached() sees that VolumeAttachment is detached, updates asw

It's not possible to distinguish attaching from attached there.

  1. AD reconciler allows new Attach on new node to proceed.

IMO, A/D controller (or csi plugin) should see the Node does not exist and delete VolumeAttachment.

from external-attacher.

msau42 avatar msau42 commented on August 30, 2024

Do you see a problem if we mark VolumeAttachment as detached while it's still attaching? How is this different from the normal case when we are first attaching a volume and it's VolumeAttachment.status.attached = false?

from external-attacher.

jsafrane avatar jsafrane commented on August 30, 2024

From external provisioner POV, it should be safe. In the worst case when both regular sync and ListVolume sync race, it marks just attached volume as detached. This causes VolumeAttachment to be synced again, ControllerPublish will be called and it will fix the VolumeAttachment status.

Still, something in the A/D controller / CSI plugin there must check the destination node is gone and delete VolumeAttachment and update ASW.

from external-attacher.

msau42 avatar msau42 commented on August 30, 2024

When a node is deleted, the combination of pods getting deleted and AD.verifyvolumesattached causes the detach

from external-attacher.

msau42 avatar msau42 commented on August 30, 2024

I forgot to mention in the initial comment, the Pod does get deleted, which should trigger detach, however AD controller doesn't because it still sees the volume mounted in asw and says it's not safe to detach. This also starts the 6 minute force detach timer.

It depends on VerifyVolumesAttached to override asw.

from external-attacher.

jsafrane avatar jsafrane commented on August 30, 2024

It starts making sense now.

I'm thinking if it can break anything in A/D controller. If A/D VerifyVolumesAreAttached finds a volume that is detached and the pod does not exist, it will remove the pod from ASW. Who is going to delete the VolumeAttachment then? CSI plugin.Detach is not called... Should A/D controller call it?

from external-attacher.

msau42 avatar msau42 commented on August 30, 2024

Looks like VolumesAreAttached calls DeleteVolumeNode, which completely removes it from asw, so you're right, Detach won't get called and VolumeAttachment will be leaked. One possibility is to not completely remove it from asw, but mark some special state.

Another possibility is we add VolumeAttachment GC to the AD: kubernetes/kubernetes#77324

from external-attacher.

msau42 avatar msau42 commented on August 30, 2024

Another thought, should VolumeAttachment have ownerref to Node object so when the Node object gets deleted, we automatically trigger Detach?

from external-attacher.

jsafrane avatar jsafrane commented on August 30, 2024

I'd prefer to have a proper fix instead of piling up hacks in A/D controller & external-attacher.

Node object is deleted. PodGCController deletes pods from the node. Should it delete VolumeAttachments too? That looks too CSI specific... Or should we add loop to A/D controller to force-detach from really deleted nodes sooner that in 6 minutes? This will work for all volume plugins and we don't need ListVolumes / VerifyVolumesAreAttached at all.

How probable is it that the deleted node comes back and how quickly? If someone accidentally deletes a Node object (keeping kubelet running), how quickly is it re-created? PodGCController has 40s "quarantine".

from external-attacher.

msau42 avatar msau42 commented on August 30, 2024

VerifyVolumesAreAttached is still needed for other scenarios where node still exists but volume got detached out of band, or node gets recreated with the same name.

The other thing I realized is that this problem only occurs if the Node is ungracefully deleted (without a drain).

If someone deleted the Node object, kubelet only recreates it if it restarts.

I think we should still let Pod GC invoke detach. Right now it is guarded by the existence of node, which is guarded by this check. I'm not sure what problems may happen if we delete the node from cache while we think pods are still attached to it. The other problem is that nodeDelete is only called via informer. If it fails that one time, then we never retry removing the node from cache again.

from external-attacher.

msau42 avatar msau42 commented on August 30, 2024

I discussed a bit with @saad-ali and he has concerns about using the volumeattachment.status.attached field to indicate that a volume is detached because it could also mean attach or detach could be in progress. If we want to fix this, we may need to think about having a new field that can actually indicate something is detached with nothing in progress.

Also, we probably should revisit the logic in csi.VolumesAreAttached that uses the status.attached field, and also revisit other cases where we return attached = false. That causes the volume to be removed from asw. Could that be problematic if it's actually still attached?

from external-attacher.

jsafrane avatar jsafrane commented on August 30, 2024

concerns about using the volumeattachment.status.attached field to indicate that a volume is detached because it could also mean attach or detach could be in progress

The only signal that a volume is fully detached is that VolumeAttachment is deleted / missing and IMO only this should be used by A/D controller to check that the volume is fully detached. When VolumeAttachment exists, the volume may be attaching / detaching / fully attached.

We could add "detached and can't attach because node is gone" to VolumeAttachment, but that is safe and race free only until the node appears again.

from external-attacher.

msau42 avatar msau42 commented on August 30, 2024

/lifecycle frozen

from external-attacher.

ialidzhikov avatar ialidzhikov commented on August 30, 2024

@msau42 , isn't this issue resolved by kubernetes/kubernetes#96617 (which is the fix for kubernetes/kubernetes#77324)?

from external-attacher.

msau42 avatar msau42 commented on August 30, 2024

nodes are deleted:

  • by the cluster autoscaler
  • during cluster upgrade

Those scenarios are graceful scenarios, where we highly recommend drain is done first. Draining first before deleting the node should solve at least the graceful case. Although there is a slight race that the drain process also needs to fully wait for volumes to be completely unmounted.

For the ungraceful case, the 6 minute force detach should kick in

from external-attacher.

irasnyd avatar irasnyd commented on August 30, 2024

Still hitting this with EKS v1.26

from external-attacher.

nooperpudd avatar nooperpudd commented on August 30, 2024

Need to wait about 6mins to attach volume

Warning  FailedAttachVolume      8m12s  attachdetach-controller  Multi-Attach error for volume "pvc-bc4b37b5-1726-414b-aac8-ec1a01209041" Volume is already exclusively attached to one node and can't be attached to another  
                 
Normal   SuccessfulAttachVolume  2m8s   attachdetach-controller  AttachVolume.Attach succeeded for volume "pvc-bc4b37b5-1726-414b-aac8-ec1a01209041"

from external-attacher.

fungaren avatar fungaren commented on August 30, 2024

Maybe we can learn from this:

https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/release-1.29/cmd/hooks/prestop.go

from external-attacher.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.