Skip to content

Parallelise CRD registration to improve bootstrap time #12719

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 30, 2020
Merged

Parallelise CRD registration to improve bootstrap time #12719

merged 2 commits into from
Jul 30, 2020

Conversation

tgraf
Copy link
Member

@tgraf tgraf commented Jul 30, 2020 •
edited
Loading

Unscientific measurement:

Before:

cilium_agent_bootstrap_seconds                        outcome="success" scope="k8sInit"                                                                 2.103586

After:

cilium_agent_bootstrap_seconds                        scope="k8sInit" outcome="success"                                                                 0.581899
cilium_agent_bootstrap_seconds                        outcome="success" scope="kvstore"                                                                 0.001010

@tgraf tgraf added area/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. kind/performance There is a performance impact of this. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. needs-backport/1.7 labels Jul 30, 2020
@tgraf tgraf requested a review from a team July 30, 2020 07:39
@tgraf tgraf requested review from a team as code owners July 30, 2020 07:39
tklauser
tklauser approved these changes Jul 30, 2020
View reviewed changes
Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor nit, feel free to ignore.

@coveralls
Copy link

coveralls commented Jul 30, 2020 •
edited
Loading

Coverage Status

Coverage decreased (-0.01%) to 36.66% when pulling b7d775a on pr/tgraf/parallel-k8s-init into 86476e2 on master.

tgraf added 2 commits July 30, 2020 11:13
@tgraf
k8s: Register CRDs in parallel
affc4f8 
Individual CRD registrations do not depend on each other, the registration can
be done in parallel.

Signed-off-by: Thomas Graf <thomas@cilium.io>
@tgraf
agent: Fix bootstrap metric for kvstore
b7d775a 
Do not account kvstore initialization as k8s bootstrap time.

Signed-off-by: Thomas Graf <thomas@cilium.io>
@tgraf tgraf force-pushed the pr/tgraf/parallel-k8s-init branch from 9ff7d63 to b7d775a Compare July 30, 2020 09:13
@tgraf
Copy link
Member Author

tgraf commented Jul 30, 2020

test-me-please

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 30, 2020
gandro
gandro reviewed Jul 30, 2020
View reviewed changes
pkg/k8s/apis/cilium.io/v2/client/register.go
@@ -18,6 +18,7 @@ import (
"context"
goerrors "errors"
"fmt"
"golang.org/x/sync/errgroup"
Copy link
Member

@gandro gandro Jul 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Is this import in the correct section?

@tgraf tgraf merged commit 87d68ea into master Jul 30, 2020
@tgraf tgraf deleted the pr/tgraf/parallel-k8s-init branch July 30, 2020 13:57
@christarazi christarazi mentioned this pull request Jul 31, 2020
Merged
@qmonnet qmonnet mentioned this pull request Aug 3, 2020
Merged
marseel added a commit to marseel/cilium that referenced this pull request Jul 2, 2025
@marseel
k8s: update CRDs in series instead of in parallel
3c803a9 
In the past, we were creating and updating CRDs in Agent and because of
that we were doing that in parallel to speed up bootstrap. Related to cilium#12719

However, since we started creating and updating CRDs in operator, this
is no longer necessary and can actually cause problems in large
clusters: cilium#39267

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
@marseel marseel mentioned this pull request Jul 2, 2025
Merged
marseel added a commit to marseel/cilium that referenced this pull request Jul 2, 2025
@marseel
operator: update CRDs in series instead of in parallel
7034d08 
In the past, we were creating and updating CRDs in Agent and because of
that we were doing that in parallel to speed up bootstrap. Related to cilium#12719

However, since we started creating and updating CRDs in operator, this
is no longer necessary and can actually cause problems in large
clusters: cilium#39267

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
github-merge-queue bot pushed a commit that referenced this pull request Jul 11, 2025
@marseel @tklauser
operator: update CRDs in series instead of in parallel
3356222 
In the past, we were creating and updating CRDs in Agent and because of
that we were doing that in parallel to speed up bootstrap. Related to #12719

However, since we started creating and updating CRDs in operator, this
is no longer necessary and can actually cause problems in large
clusters: #39267

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
rastislavs pushed a commit that referenced this pull request Jul 31, 2025
@marseel @rastislavs
operator: update CRDs in series instead of in parallel
5fefb33 
[ upstream commit 3356222 ]

In the past, we were creating and updating CRDs in Agent and because of
that we were doing that in parallel to speed up bootstrap. Related to #12719

However, since we started creating and updating CRDs in operator, this
is no longer necessary and can actually cause problems in large
clusters: #39267

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
github-merge-queue bot pushed a commit that referenced this pull request Aug 4, 2025
@marseel @aanm
operator: update CRDs in series instead of in parallel
ce69cfb 
[ upstream commit 3356222 ]

In the past, we were creating and updating CRDs in Agent and because of
that we were doing that in parallel to speed up bootstrap. Related to #12719

However, since we started creating and updating CRDs in operator, this
is no longer necessary and can actually cause problems in large
clusters: #39267

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
rastislavs pushed a commit that referenced this pull request Aug 4, 2025
@marseel @rastislavs
operator: update CRDs in series instead of in parallel
55d59e4 
[ upstream commit 3356222 ]

In the past, we were creating and updating CRDs in Agent and because of
that we were doing that in parallel to speed up bootstrap. Related to #12719

However, since we started creating and updating CRDs in operator, this
is no longer necessary and can actually cause problems in large
clusters: #39267

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
github-merge-queue bot pushed a commit that referenced this pull request Aug 4, 2025
@marseel @joestringer
operator: update CRDs in series instead of in parallel
a241eb2 
[ upstream commit 3356222 ]

In the past, we were creating and updating CRDs in Agent and because of
that we were doing that in parallel to speed up bootstrap. Related to #12719

However, since we started creating and updating CRDs in operator, this
is no longer necessary and can actually cause problems in large
clusters: #39267

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
rabelmervin pushed a commit to rabelmervin/cilium that referenced this pull request Aug 18, 2025
@marseel @rabelmervin
operator: update CRDs in series instead of in parallel
0e424ff 
In the past, we were creating and updating CRDs in Agent and because of
that we were doing that in parallel to speed up bootstrap. Related to cilium#12719

However, since we started creating and updating CRDs in operator, this
is no longer necessary and can actually cause problems in large
clusters: cilium#39267

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gandro gandro gandro left review comments

@tklauser tklauser tklauser approved these changes

@aanm aanm aanm approved these changes

Assignees
No one assigned
Labels
area/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. kind/performance There is a performance impact of this. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@tgraf @coveralls @gandro @tklauser @aanm @christarazi @qmonnet