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

fix race condition in cloud endpoint controller #575

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Alice-Lilith
Copy link
Contributor

Fixes a race condition in the cloud endpoint controller. Currently we rely on the status.ID of a could endpoint to inform the controller about whether the cloud endpoint already exists and needs to be updated, or if this is a new custom resource and we need to update the cloud endpoint definition on the server side.

Relying on the status.ID within the controller leads to race conditions when a user (or in this case, the translation from ingresses to endpoints) updates/changes a CloudEndpoint resource after it was first created, but before the controller has assigned it a status.ID and updated it. When this happens, the incoming changed but not new CloudEndpoint still has no status.ID (which will eventually get properly set) and this causes the controller to create multiple different definitions for the cloud endpoint on the ngrok server side and then not being able to properly remove them once the CloudEndpoint resource is deleted.

Instead of doing that, this makes the controller keep an in-memory map of the known CloudEndpoint resources so that we can simply check the in-memory map to see if one of them has been created and assigned an ID or not, rather than waiting for the status to be updated and the resource to be re-reconciled again after that.

@github-actions github-actions bot added the area/controller Issues dealing with the controller label Jan 16, 2025
@Alice-Lilith Alice-Lilith marked this pull request as ready for review January 16, 2025 18:27
@Alice-Lilith Alice-Lilith requested a review from a team as a code owner January 16, 2025 18:27
Copy link

codecov bot commented Jan 16, 2025

Codecov Report

Attention: Patch coverage is 0% with 49 lines in your changes missing coverage. Please review.

Project coverage is 23.57%. Comparing base (0fac123) to head (ec36c80).

Files with missing lines Patch % Lines
...ernal/controller/ngrok/cloudendpoint_controller.go 0.00% 49 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #575      +/-   ##
==========================================
- Coverage   23.66%   23.57%   -0.09%     
==========================================
  Files          88       88              
  Lines       12446    12491      +45     
==========================================
  Hits         2945     2945              
- Misses       9314     9359      +45     
  Partials      187      187              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/controller Issues dealing with the controller
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant