Skip to content

Reduce resource consumption when generating Kubernetes certificates#52109

Merged
rosstimothy merged 1 commit intomasterfrom
tross/kube_cert_perf
Feb 13, 2025
Merged

Reduce resource consumption when generating Kubernetes certificates#52109
rosstimothy merged 1 commit intomasterfrom
tross/kube_cert_perf

Conversation

@rosstimothy
Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy commented Feb 13, 2025

Closes #52073.

The requested Kubernetes cluster is now cross referenced with the KubeServers in the unified resource cache. This results in a reduction in CPU, memory, and cert generation latency. This also cleans up some of the helper functions in lib/kube/utils that were no longer needed, and suboptimal.

The client side changes here shouldn't have any impact, as the server is performing the same check, and returning the equivalent error the client side code used to. This will also cut the time of tctl auth sign in half as both the client and server were performing the same expensive CheckKubeCluster operation.

Changelog: Improve latency and reduce resource consumption of generating Kubernetes certificates via tctl auth sign and tsh kube login.

@rosstimothy rosstimothy added the no-changelog Indicates that a PR does not require a changelog entry label Feb 13, 2025
@rosstimothy rosstimothy force-pushed the tross/kube_cert_perf branch 4 times, most recently from 2f4c6f3 to 3af4b9b Compare February 13, 2025 01:20
@rosstimothy rosstimothy removed the no-changelog Indicates that a PR does not require a changelog entry label Feb 13, 2025
Closes #52073.

The requested Kubernetes cluster is now cross referenced with the
KubeServers in the unified resource cache. This results in a
reduction in CPU, memory, and cert generation latency. This also
cleans up some of the helper functions in lib/kube/utils that
were no longer needed, and suboptimal.

The client side changes here shouldn't have any impact, as the
server is performing the same check, and returning the equivalent
error the client side code used to. This will also cut the time
of `tctl auth sign` in half as both the client and server were
performing the same expensive CheckKubeCluster operation.
@rosstimothy rosstimothy marked this pull request as ready for review February 13, 2025 02:12
@rosstimothy rosstimothy requested a review from tigrato February 13, 2025 02:12
@github-actions github-actions Bot added kubernetes-access size/sm tctl tctl - Teleport admin tool tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels Feb 13, 2025
@github-actions github-actions Bot requested review from avatus and creack February 13, 2025 02:12
@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from creack February 13, 2025 17:57
@rosstimothy rosstimothy added this pull request to the merge queue Feb 13, 2025
Merged via the queue into master with commit 8a5107c Feb 13, 2025
@rosstimothy rosstimothy deleted the tross/kube_cert_perf branch February 13, 2025 19:46
@public-teleport-github-review-bot
Copy link
Copy Markdown

@rosstimothy See the table below for backport results.

Branch Result
branch/v15 Failed
branch/v16 Failed
branch/v17 Failed

rosstimothy added a commit that referenced this pull request Feb 13, 2025
…52109)

Closes #52073.

The requested Kubernetes cluster is now cross referenced with the
KubeServers in the unified resource cache. This results in a
reduction in CPU, memory, and cert generation latency. This also
cleans up some of the helper functions in lib/kube/utils that
were no longer needed, and suboptimal.

The client side changes here shouldn't have any impact, as the
server is performing the same check, and returning the equivalent
error the client side code used to. This will also cut the time
of `tctl auth sign` in half as both the client and server were
performing the same expensive CheckKubeCluster operation.
rosstimothy added a commit that referenced this pull request Feb 13, 2025
…52109)

Closes #52073.

The requested Kubernetes cluster is now cross referenced with the
KubeServers in the unified resource cache. This results in a
reduction in CPU, memory, and cert generation latency. This also
cleans up some of the helper functions in lib/kube/utils that
were no longer needed, and suboptimal.

The client side changes here shouldn't have any impact, as the
server is performing the same check, and returning the equivalent
error the client side code used to. This will also cut the time
of `tctl auth sign` in half as both the client and server were
performing the same expensive CheckKubeCluster operation.
rosstimothy added a commit that referenced this pull request Feb 13, 2025
…52109)

Closes #52073.

The requested Kubernetes cluster is now cross referenced with the
KubeServers in the unified resource cache. This results in a
reduction in CPU, memory, and cert generation latency. This also
cleans up some of the helper functions in lib/kube/utils that
were no longer needed, and suboptimal.

The client side changes here shouldn't have any impact, as the
server is performing the same check, and returning the equivalent
error the client side code used to. This will also cut the time
of `tctl auth sign` in half as both the client and server were
performing the same expensive CheckKubeCluster operation.
rosstimothy added a commit that referenced this pull request Feb 13, 2025
…52109)

Closes #52073.

The requested Kubernetes cluster is now cross referenced with the
KubeServers in the unified resource cache. This results in a
reduction in CPU, memory, and cert generation latency. This also
cleans up some of the helper functions in lib/kube/utils that
were no longer needed, and suboptimal.

The client side changes here shouldn't have any impact, as the
server is performing the same check, and returning the equivalent
error the client side code used to. This will also cut the time
of `tctl auth sign` in half as both the client and server were
performing the same expensive CheckKubeCluster operation.
rosstimothy added a commit that referenced this pull request Feb 13, 2025
…52109)

Closes #52073.

The requested Kubernetes cluster is now cross referenced with the
KubeServers in the unified resource cache. This results in a
reduction in CPU, memory, and cert generation latency. This also
cleans up some of the helper functions in lib/kube/utils that
were no longer needed, and suboptimal.

The client side changes here shouldn't have any impact, as the
server is performing the same check, and returning the equivalent
error the client side code used to. This will also cut the time
of `tctl auth sign` in half as both the client and server were
performing the same expensive CheckKubeCluster operation.
carloscastrojumo pushed a commit to carloscastrojumo/teleport that referenced this pull request Feb 19, 2025
…ravitational#52109)

Closes gravitational#52073.

The requested Kubernetes cluster is now cross referenced with the
KubeServers in the unified resource cache. This results in a
reduction in CPU, memory, and cert generation latency. This also
cleans up some of the helper functions in lib/kube/utils that
were no longer needed, and suboptimal.

The client side changes here shouldn't have any impact, as the
server is performing the same check, and returning the equivalent
error the client side code used to. This will also cut the time
of `tctl auth sign` in half as both the client and server were
performing the same expensive CheckKubeCluster operation.
rosstimothy added a commit that referenced this pull request Feb 20, 2025
…52109)

Closes #52073.

The requested Kubernetes cluster is now cross referenced with the
KubeServers in the unified resource cache. This results in a
reduction in CPU, memory, and cert generation latency. This also
cleans up some of the helper functions in lib/kube/utils that
were no longer needed, and suboptimal.

The client side changes here shouldn't have any impact, as the
server is performing the same check, and returning the equivalent
error the client side code used to. This will also cut the time
of `tctl auth sign` in half as both the client and server were
performing the same expensive CheckKubeCluster operation.
rosstimothy added a commit that referenced this pull request Feb 20, 2025
…52109)

Closes #52073.

The requested Kubernetes cluster is now cross referenced with the
KubeServers in the unified resource cache. This results in a
reduction in CPU, memory, and cert generation latency. This also
cleans up some of the helper functions in lib/kube/utils that
were no longer needed, and suboptimal.

The client side changes here shouldn't have any impact, as the
server is performing the same check, and returning the equivalent
error the client side code used to. This will also cut the time
of `tctl auth sign` in half as both the client and server were
performing the same expensive CheckKubeCluster operation.
github-merge-queue Bot pushed a commit that referenced this pull request Feb 20, 2025
…52109) (#52146)

Closes #52073.

The requested Kubernetes cluster is now cross referenced with the
KubeServers in the unified resource cache. This results in a
reduction in CPU, memory, and cert generation latency. This also
cleans up some of the helper functions in lib/kube/utils that
were no longer needed, and suboptimal.

The client side changes here shouldn't have any impact, as the
server is performing the same check, and returning the equivalent
error the client side code used to. This will also cut the time
of `tctl auth sign` in half as both the client and server were
performing the same expensive CheckKubeCluster operation.
@codingllama
Copy link
Copy Markdown
Contributor

FYI, It looks like this broke local runs of lib/auth.TestAuthenticateSSHUser:

$ go test ./lib/auth -run=TestAuthenticateSSHUser -failfast -count=1

--- FAIL: TestAuthenticateSSHUser (0.24s)
    auth_test.go:750:
                Error Trace:    /Users/alan/code/teleport/lib/auth/auth_test.go:750
                Error:          Received unexpected error:
                                Kubernetes cluster "root-kube-cluster" is not registered in this Teleport cluster; you can list registered Kubernetes clusters using 'tsh kube ls'
                Test:           TestAuthenticateSSHUser
FAIL
FAIL    github.com/gravitational/teleport/lib/auth      3.094s
FAIL

Chatting with Tim about it.

rosstimothy added a commit that referenced this pull request Feb 24, 2025
#52109 added a dependency on the unified resource cache to user
cert generation to reduce resource consumption. A number of tests
that exercise generating Kubernetes user certs were either not
waiting for the Kubernetes resources to exist prior to authentication
and getting lucky, or checking that the resources were in the auth
cache, but not the unified resource cache.

This attempts to cover any tests which generate Kubernetes user
certificates to verify that the unified resource cache contains
the expected cluster before proceeding.

Fixes #52157.
Fixes #52441.
rosstimothy added a commit that referenced this pull request Feb 25, 2025
#52109 added a dependency on the unified resource cache to user
cert generation to reduce resource consumption. A number of tests
that exercise generating Kubernetes user certs were either not
waiting for the Kubernetes resources to exist prior to authentication
and getting lucky, or checking that the resources were in the auth
cache, but not the unified resource cache.

This attempts to cover any tests which generate Kubernetes user
certificates to verify that the unified resource cache contains
the expected cluster before proceeding.

Fixes #52157.
Fixes #52441.
github-merge-queue Bot pushed a commit that referenced this pull request Feb 26, 2025
#52109 added a dependency on the unified resource cache to user
cert generation to reduce resource consumption. A number of tests
that exercise generating Kubernetes user certs were either not
waiting for the Kubernetes resources to exist prior to authentication
and getting lucky, or checking that the resources were in the auth
cache, but not the unified resource cache.

This attempts to cover any tests which generate Kubernetes user
certificates to verify that the unified resource cache contains
the expected cluster before proceeding.

Fixes #52157.
Fixes #52441.
github-actions Bot pushed a commit that referenced this pull request Feb 26, 2025
#52109 added a dependency on the unified resource cache to user
cert generation to reduce resource consumption. A number of tests
that exercise generating Kubernetes user certs were either not
waiting for the Kubernetes resources to exist prior to authentication
and getting lucky, or checking that the resources were in the auth
cache, but not the unified resource cache.

This attempts to cover any tests which generate Kubernetes user
certificates to verify that the unified resource cache contains
the expected cluster before proceeding.

Fixes #52157.
Fixes #52441.
github-merge-queue Bot pushed a commit that referenced this pull request Feb 27, 2025
#52109 added a dependency on the unified resource cache to user
cert generation to reduce resource consumption. A number of tests
that exercise generating Kubernetes user certs were either not
waiting for the Kubernetes resources to exist prior to authentication
and getting lucky, or checking that the resources were in the auth
cache, but not the unified resource cache.

This attempts to cover any tests which generate Kubernetes user
certificates to verify that the unified resource cache contains
the expected cluster before proceeding.

Fixes #52157.
Fixes #52441.
rosstimothy added a commit that referenced this pull request Mar 20, 2025
…52109)

Closes #52073.

The requested Kubernetes cluster is now cross referenced with the
KubeServers in the unified resource cache. This results in a
reduction in CPU, memory, and cert generation latency. This also
cleans up some of the helper functions in lib/kube/utils that
were no longer needed, and suboptimal.

The client side changes here shouldn't have any impact, as the
server is performing the same check, and returning the equivalent
error the client side code used to. This will also cut the time
of `tctl auth sign` in half as both the client and server were
performing the same expensive CheckKubeCluster operation.
rosstimothy added a commit that referenced this pull request Mar 20, 2025
#52109 added a dependency on the unified resource cache to user
cert generation to reduce resource consumption. A number of tests
that exercise generating Kubernetes user certs were either not
waiting for the Kubernetes resources to exist prior to authentication
and getting lucky, or checking that the resources were in the auth
cache, but not the unified resource cache.

This attempts to cover any tests which generate Kubernetes user
certificates to verify that the unified resource cache contains
the expected cluster before proceeding.

Fixes #52157.
Fixes #52441.
rosstimothy added a commit that referenced this pull request Mar 20, 2025
…52109)

Closes #52073.

The requested Kubernetes cluster is now cross referenced with the
KubeServers in the unified resource cache. This results in a
reduction in CPU, memory, and cert generation latency. This also
cleans up some of the helper functions in lib/kube/utils that
were no longer needed, and suboptimal.

The client side changes here shouldn't have any impact, as the
server is performing the same check, and returning the equivalent
error the client side code used to. This will also cut the time
of `tctl auth sign` in half as both the client and server were
performing the same expensive CheckKubeCluster operation.
rosstimothy added a commit that referenced this pull request Mar 20, 2025
#52109 added a dependency on the unified resource cache to user
cert generation to reduce resource consumption. A number of tests
that exercise generating Kubernetes user certs were either not
waiting for the Kubernetes resources to exist prior to authentication
and getting lucky, or checking that the resources were in the auth
cache, but not the unified resource cache.

This attempts to cover any tests which generate Kubernetes user
certificates to verify that the unified resource cache contains
the expected cluster before proceeding.

Fixes #52157.
Fixes #52441.
github-merge-queue Bot pushed a commit that referenced this pull request Mar 20, 2025
…ates (#52148)

* Reduce resource consumption when generating Kubernetes certificates (#52109)

Closes #52073.

The requested Kubernetes cluster is now cross referenced with the
KubeServers in the unified resource cache. This results in a
reduction in CPU, memory, and cert generation latency. This also
cleans up some of the helper functions in lib/kube/utils that
were no longer needed, and suboptimal.

The client side changes here shouldn't have any impact, as the
server is performing the same check, and returning the equivalent
error the client side code used to. This will also cut the time
of `tctl auth sign` in half as both the client and server were
performing the same expensive CheckKubeCluster operation.

* Fix test flakes related to Kubernetes user cert generation (#52442)

#52109 added a dependency on the unified resource cache to user
cert generation to reduce resource consumption. A number of tests
that exercise generating Kubernetes user certs were either not
waiting for the Kubernetes resources to exist prior to authentication
and getting lucky, or checking that the resources were in the auth
cache, but not the unified resource cache.

This attempts to cover any tests which generate Kubernetes user
certificates to verify that the unified resource cache contains
the expected cluster before proceeding.

Fixes #52157.
Fixes #52441.
github-merge-queue Bot pushed a commit that referenced this pull request Mar 20, 2025
…ates (#52147)

* Reduce resource consumption when generating Kubernetes certificates (#52109)

Closes #52073.

The requested Kubernetes cluster is now cross referenced with the
KubeServers in the unified resource cache. This results in a
reduction in CPU, memory, and cert generation latency. This also
cleans up some of the helper functions in lib/kube/utils that
were no longer needed, and suboptimal.

The client side changes here shouldn't have any impact, as the
server is performing the same check, and returning the equivalent
error the client side code used to. This will also cut the time
of `tctl auth sign` in half as both the client and server were
performing the same expensive CheckKubeCluster operation.

* Fix test flakes related to Kubernetes user cert generation (#52442)

#52109 added a dependency on the unified resource cache to user
cert generation to reduce resource consumption. A number of tests
that exercise generating Kubernetes user certs were either not
waiting for the Kubernetes resources to exist prior to authentication
and getting lucky, or checking that the resources were in the auth
cache, but not the unified resource cache.

This attempts to cover any tests which generate Kubernetes user
certificates to verify that the unified resource cache contains
the expected cluster before proceeding.

Fixes #52157.
Fixes #52441.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/branch/v17 kubernetes-access size/sm tctl tctl - Teleport admin tool tsh tsh - Teleport's command line tool for logging into nodes running Teleport.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve performance of generating kubernetes user certificates

4 participants