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.

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

Decrease CRD setup API calls when starting cilium-agent #10676

Merged
merged 2 commits into from Mar 24, 2020
Merged

Decrease CRD setup API calls when starting cilium-agent #10676

merged 2 commits into from Mar 24, 2020

Conversation

aanm
Copy link
Member

@aanm aanm commented Mar 24, 2020

This commit reduces the number of unnecessary API calls being executed when initializing Cilium.

Number of API calls made when starting before this PR (10 GETs and 2 PUTs):

GET /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumnetworkpolicies.cilium.io
GET /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumnetworkpolicies.cilium.io
GET /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumclusterwidenetworkpolicies.cilium.io
GET /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumclusterwidenetworkpolicies.cilium.io
GET /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumendpoints.cilium.io
GET /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumendpoints.cilium.io
PUT /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumendpoints.cilium.io
GET /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumendpoints.cilium.io
GET /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumnodes.cilium.io
GET /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumnodes.cilium.io
PUT /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumnodes.cilium.io
GET /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumnodes.cilium.io

Number of API calls made when starting after this PR (4 GETs):

GET /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumnetworkpolicies.cilium.io
GET /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumclusterwidenetworkpolicies.cilium.io
GET /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumendpoints.cilium.io
GET /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/ciliumnodes.cilium.io

@aanm
pkg/k8s: only update CRD if io.cilium.k8s.crd.schema.version is set
7f4914c 
Since only CNP and CCNP have io.cilium.k8s.crd.schema.version set, it
does not make sense to check if other CRDs, for example CiliumNode and
CiliumEndpoint, require an update of its CRD. This commit reduces the
number of unnecessary updates of the CRD by 2 requests each time
cilium-agent is created.

Signed-off-by: André Martins <andre@cilium.io>
@aanm
pkg/k8s: to CRD get after checking for CRD status condition
34e370e 
We already have a CRD object in memory so we don't need to re-fetch it
again, only in case the CRD Status condition is not Established that we
need to refetch the CRD object to verify its status.

Doing this reduced the number of unnecessary CRD GETs by 2 each time the
cilium-agent is restarted.

Signed-off-by: André Martins <andre@cilium.io>
@aanm aanm added pending-review sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Mar 24, 2020
@aanm aanm requested a review from a team as a code owner March 24, 2020 00:55
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Mar 24, 2020
@aanm
Copy link
Member Author

aanm commented Mar 24, 2020

test-me-please

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 45.599% when pulling 34e370e on pr/fix-crd-setup into 0f49fc6 on master.

tgraf
tgraf approved these changes Mar 24, 2020
View reviewed changes
Copy link
Member

@tgraf tgraf left a comment

Choose a reason for hiding this comment

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

Looks good. The polling of the CRD should probably use exponential backoff as well so we avoid hammering the API server, restart, hammer the API server, ...

@borkmann borkmann merged commit 65f2fe2 into master Mar 24, 2020
1.8.0 automation moved this from In progress to Merged Mar 24, 2020
@borkmann borkmann deleted the pr/fix-crd-setup branch March 24, 2020 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgraf tgraf tgraf approved these changes

Assignees
No one assigned
Labels
release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers.
Projects
No open projects
1.8.0
  
Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@aanm @coveralls @tgraf @borkmann