Don't run API server if not needed#4428
Open
caseydavenport wants to merge 3 commits intotigera:masterfrom
Open
Don't run API server if not needed#4428caseydavenport wants to merge 3 commits intotigera:masterfrom
caseydavenport wants to merge 3 commits intotigera:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces support for running the API server pod without the aggregation API server container when using v3 CRDs instead of aggregated API resources. The main purpose is to avoid running unnecessary API server components in Calico OSS when not needed, while maintaining the ability to run other containers (like query server) in Enterprise deployments.
Changes:
- Added
RequiresAggregationServerconfiguration flag to conditionally enable/disable the aggregation API server - Modified deployment logic to conditionally include containers and resources based on whether aggregation server is needed
- Updated test infrastructure to use
ptr.To()utility and improved error messages with type information
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/render/apiserver.go | Core logic changes to conditionally render aggregation server resources and containers |
| pkg/render/apiserver_test.go | Added test coverage for non-aggregation mode and updated test setup to use ptr.To() |
| pkg/controller/apiserver/apiserver_controller.go | Added early exit for Calico OSS with v3 CRDs and wired up RequiresAggregationServer flag |
| pkg/controller/apiserver/apiserver_controller_test.go | Refactored to use options.ControllerOptions struct instead of individual fields |
| pkg/render/common/test/testing.go | Enhanced error messages to include resource type information |
Comments suppressed due to low confidence (1)
pkg/render/apiserver_test.go:1
- While removing blank lines, a duplicate assertion was exposed at lines 1862 and 1863: both check
d.Spec.Selector.MatchLabelsfor the same key-value pair. This is redundant and should be removed.
// Copyright (c) 2019-2026 Tigera, Inc. All rights reserved.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
For Calico OSS, if we aren't using an API server just degrade and return.
For enterprise, we need to run the Pod but disable the aggregation API server container and associated resources.
Release Note
For PR author
make gen-filesmake gen-versionsFor PR reviewers
A note for code reviewers - all pull requests must have the following:
kind/bugif this is a bugfix.kind/enhancementif this is a a new feature.enterpriseif this PR applies to Calico Enterprise only.