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

[stress] Sync namespace federated credentials periodically and on startup #9063

Merged
merged 4 commits into from
Oct 3, 2024

Conversation

benbp
Copy link
Member

@benbp benbp commented Sep 26, 2024

The original implementation was skipping lookups to see if credentials already existed, on the assumption that deletion was always being handled. This resulted in multiple credentials being created for the same namespace.

Instead we can simply poll to make sure deleted namespaces don't continue to take up a federated identity credential slot as a failsafe. We also sync credential state on startup and keep an in-memory cache, so that namespace deletions/creations re-use existing credentials. There could be some edge cases here with the cache, but I think it's a better trade-off than always querying the managed identity pool every time a namespace is created, as the operation is slow and can increase stress deploy times (we need to wait on completion before deploying otherwise there is a race condition with the credential being created and the deploy container running).

EDIT: my original changes assumed DELETE events weren't being caught, but it's actually because I had added the below code. Now that is removed and the delete event handler method has been added back.

if (ns.Status?.Phase == "Terminating")
{
    return;
}

@benbp benbp added Central-EngSys This issue is owned by the Engineering System team. Stress This issue is related to stress testing, part of our reliability pillar. labels Sep 26, 2024
@benbp benbp self-assigned this Sep 26, 2024
@benbp benbp force-pushed the benbp/watcher-fed-cred branch from b9f3016 to 411d4ff Compare September 26, 2024 22:28
@benbp benbp requested a review from weshaggard September 26, 2024 22:29
@weshaggard
Copy link
Member

@microsoft-github-policy-service rerun

@benbp
Copy link
Member Author

benbp commented Sep 27, 2024

@weshaggard see my comment edit. I added back the delete handler method (facepalm). Otherwise everything is the same.

@benbp benbp merged commit c32711b into Azure:main Oct 3, 2024
14 checks passed
@benbp benbp deleted the benbp/watcher-fed-cred branch October 3, 2024 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Central-EngSys This issue is owned by the Engineering System team. Stress This issue is related to stress testing, part of our reliability pillar.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants