loft-sh / vcluster

vCluster - Create fully functional virtual Kubernetes clusters - Each vcluster runs inside a namespace of the underlying k8s cluster. It's cheaper than creating separate full-blown clusters and it offers better multi-tenancy and isolation than regular namespaces.
https://www.vcluster.com
Apache License 2.0
6.92k stars 426 forks source link

reduce concurrent reconciles of the same virtual resource #2246

Closed lizardruss closed 2 weeks ago

lizardruss commented 3 weeks ago

What issue type does this pull request address? (keep at least one, remove the others) /kind bugfix

What does this pull request do? Which issues does it resolve? (use resolves #<issue_number> if possible) resolves ENG-4924

Please provide a short message that should be published in the vcluster release notes Fixed an issue where vcluster incorrectly re-created pods during garbage collection

What else do we need to know? The previous method of passing through the syncEventType would potentially enqueue the same object 4 different ways:

Usually this would not be a concern, but the following scenario occurs frequently enough to cause a conformance test to fail:

  1. virtual object is deleted
  2. a update event for the virtual object is received that adds the deletionTimestamp
  3. a delete event for the virtual object is received
  4. these events would enqueue the object as objects with the same name in different namespaces from the controller runtime's perspective
  5. the two objects are reconciled concurrently by the controller-runtime
  6. while the Reconcile function is processing the virtual object update the virtual object is deleted, this leads to the SyncToVirtual implementation incorrectly re-creating the virtual object

This PR attempts to solve this by:

netlify[bot] commented 3 weeks ago

Deploy Preview for vcluster-docs canceled.

Built without sensitive environment variables

Name Link
Latest commit 6960aa800e4dde62859d261601fb5894b42a4d4c
Latest deploy log https://app.netlify.com/sites/vcluster-docs/deploys/6720fe351a72300008d0262d
lizardruss commented 3 weeks ago

Currently looking into a solution that:

FabianKramm commented 2 weeks ago

Closing this since we went for another solution