Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Improve performance of fetching cluster resources when deploying flux to sync in all namespaces #2520

Merged
merged 2 commits into from
Oct 17, 2019
Merged

Improve performance of fetching cluster resources when deploying flux to sync in all namespaces #2520

merged 2 commits into from
Oct 17, 2019

Conversation

waseem-h
Copy link
Contributor

what

Improve logic for fetching resources from the cluster

why

Flux fetches resources from all namespaces on a per namespace basis even if we tell flux to deploy in all namespaces. This becomes an issue when there are a lot of namespaces and a lot of resource kinds.

use-case

We have 300+ namespaces and 25+ CRDs in our cluster and it takes about 13.5 minutes for only the resources to be fetched which is a problem for us since we want to sync changes every minute. This PR improves the performance significantly by fetching resources from all namespaces if that is enabled instead of a per namespace basis.

@waseem-h
Copy link
Contributor Author

I would also like to mention that this reduces the number of calls made to the API Server by a huge amount.
Previously lets say you had 30 namespaces and 20 different kinds of resources, then flux would have to make 600 requests to fetch the state. With this, flux only makes 20 requests to the API Server and retrieves the same state. This is assuming that you're allowing flux to watch in all namespaces. If flux is running on selected namespaces then this PR has no affect on the number of calls

@waseem-h
Copy link
Contributor Author

@hiddeco It would be great if you could review this today

stefanprodan
stefanprodan approved these changes Oct 17, 2019
View reviewed changes
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

@waseem-h can you please squash the commits? thanks

@waseem-h
Fetch resources directly if all namespaces are enabled
c8484d4 
Fix test cases

Change to make

Change back to string array

Fix test case

Change back to []string format

Fetch full ns object instead of creating one
@waseem-h
Copy link
Contributor Author

@stefanprodan squashed

hiddeco
hiddeco approved these changes Oct 17, 2019
View reviewed changes
Copy link
Member

@hiddeco hiddeco left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you @waseem-h, this is an obvious optimization we missed 💯

@stefanprodan stefanprodan merged commit f169854 into fluxcd:master Oct 17, 2019
@stefanprodan
Copy link
Member

@waseem-h you can deploy the fix on your cluster using this image fluxcd/flux-prerelease:master-f1698542

@waseem-h
Copy link
Contributor Author

Thanks @stefanprodan. Any idea when the new minor is going to be released?

@stefanprodan
Copy link
Member

Probably in two weeks from now.

@waseem-h
Copy link
Contributor Author

Awesome

@2opremio 2opremio added this to the 1.16.0 milestone Nov 21, 2019
@2opremio 2opremio mentioned this pull request Dec 2, 2019
Closed
@2opremio 2opremio mentioned this pull request Jan 8, 2020
Closed
2opremio added a commit to 2opremio/flux that referenced this pull request Jan 8, 2020
@2opremio
Correctly scope imagePullSecrets by their namespace
a802915 
PR fluxcd#2520 broke the scoping of imagePullSecrets when
allowing to get workloads from all namespaces at once.

The problem is that a secret cache (`seenCreds`) was indexed
by the secret name (making it namespace specific).

Processing all the namespaces at once without adjusting the
scoping opened the door to clashes between secrets from
different namespaces which are called the same.
@2opremio 2opremio mentioned this pull request Jan 8, 2020
Merged
2opremio added a commit to 2opremio/flux that referenced this pull request Jan 13, 2020
@2opremio
Correctly scope imagePullSecrets by their namespace
cf5a3e6 
PR fluxcd#2520 broke the scoping of imagePullSecrets when
allowing to get workloads from all namespaces at once.

The problem is that a secret cache (`seenCreds`) was indexed
by the secret name (making it namespace specific).

Processing all the namespaces at once without adjusting the
scoping opened the door to clashes between secrets from
different namespaces which are called the same.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hiddeco hiddeco hiddeco approved these changes

@stefanprodan stefanprodan stefanprodan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.16.0
Development

Successfully merging this pull request may close these issues.
4 participants
@waseem-h @stefanprodan @hiddeco @2opremio