Skip to content

Filter while paging in GetByVMID#49968

Merged
rosstimothy merged 1 commit intomasterfrom
tross/azure_getvm
Dec 10, 2024
Merged

Filter while paging in GetByVMID#49968
rosstimothy merged 1 commit intomasterfrom
tross/azure_getvm

Conversation

@rosstimothy
Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy commented Dec 9, 2024

This updates GetByVMID, used by azure joining, to filter by id for each page of vms instead of filtering after accumulating all vms in memory first. While this does reduce memory consumption, the listing process is still suboptimal in that all instances in a subscription are retrieved in response to every join request.

Changelog: Reduce Auth memory consumption when agents join using the azure join method.

@rosstimothy rosstimothy added the no-changelog Indicates that a PR does not require a changelog entry label Dec 9, 2024
@aws-amplify-us-west-2
Copy link
Copy Markdown

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-49968.d3pp5qlev8mo18.amplifyapp.com

This updates GetByVMID, used by azure joining, to filter by id for
each page of vms instead of filtering after accumulating _all_ vms
in memory first. While this does reduce memory consumption, the listing
process is still suboptimal in that _all_ instances in a subscription
are retrieved in response to _every_ join request.
@rosstimothy rosstimothy marked this pull request as ready for review December 9, 2024 21:58
@github-actions github-actions Bot requested review from gzdunek and hugoShaka December 9, 2024 21:58
@hugoShaka
Copy link
Copy Markdown
Contributor

Related to https://github.com/gravitational/teleport.e/issues/2164

We can stop listing the VMs entirely and parse the token.

Comment thread lib/auth/join_azure_test.go
@rosstimothy
Copy link
Copy Markdown
Contributor Author

Related to gravitational/teleport.e#2164

We can stop listing the VMs entirely and parse the token.

I agree that azure joining is flawed, however, this seemed like a relatively low fruit to address until we make a decision on how to proceed improving azure joining.

@rosstimothy rosstimothy added backport/branch/v15 backport/branch/v17 and removed no-changelog Indicates that a PR does not require a changelog entry labels Dec 9, 2024
@rosstimothy rosstimothy enabled auto-merge December 9, 2024 22:30
@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from gzdunek December 10, 2024 00:52
@rosstimothy rosstimothy added this pull request to the merge queue Dec 10, 2024
Merged via the queue into master with commit 6c51c4e Dec 10, 2024
@rosstimothy rosstimothy deleted the tross/azure_getvm branch December 10, 2024 01:12
@public-teleport-github-review-bot
Copy link
Copy Markdown

@rosstimothy See the table below for backport results.

Branch Result
branch/v15 Create PR
branch/v16 Create PR
branch/v17 Create PR

carloscastrojumo pushed a commit to carloscastrojumo/teleport that referenced this pull request Feb 19, 2025
This updates GetByVMID, used by azure joining, to filter by id for
each page of vms instead of filtering after accumulating _all_ vms
in memory first. While this does reduce memory consumption, the listing
process is still suboptimal in that _all_ instances in a subscription
are retrieved in response to _every_ join request.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants