Skip to content

[Do not merge]Move apiserver component into calico-system ns for enterprise #3960

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.

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

vara2504
Copy link
Contributor

@vara2504 vara2504 commented May 30, 2025

Need to be merged along with the calico-private changes.

Changes to move the API server component into the calico-system namespace for the enterprise variant

Update default deny policy in calico-system to exclude API server pods
Create a separate CA bundle for the API server in calico-system
Clean up the legacy tigera-system namespace
Update all references from tigera-system to calico-system

Description

For PR author

  • Tests for change.
  • If changing pkg/apis/, run make gen-files
  • If changing versions, run make gen-versions

For PR reviewers

A note for code reviewers - all pull requests must have the following:

  • Milestone set according to targeted release.
  • Appropriate labels:
    • kind/bug if this is a bugfix.
    • kind/enhancement if this is a a new feature.
    • enterprise if this PR applies to Calico Enterprise only.

@marvin-tigera marvin-tigera added this to the v1.39.0 milestone May 30, 2025
@vara2504 vara2504 force-pushed the mfp_ns_apiserver branch from fb938ce to ac6d107 Compare June 5, 2025 23:07
@vara2504 vara2504 changed the title Move apiserver component into calico-system ns for enterprise [Do not merge]Move apiserver component into calico-system ns for enterprise Jun 5, 2025
@vara2504 vara2504 marked this pull request as ready for review June 5, 2025 23:46
@vara2504 vara2504 requested a review from a team as a code owner June 5, 2025 23:46
// deployment becomes unhealthy and reconciliation of non-NetworkPolicy resources in the apiserver controller
// would resolve it, we render the network policies of components last to prevent a chicken-and-egg scenario.
if includeV3NetworkPolicy {
components = append(components, render.APIServerPolicy(&apiServerCfg))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved includeV3NetworkPolicy from later in the render sequence (line 455) to the beginning. This ensures the cnx-apiserver-access NetworkPolicy is created early during upgrades, while API server pods are still running in the tigera-system namespace. This prevents default deny issues in calico-system when new API server pods start there.

Even though the default deny policy in calico-system is updated to exclude API server pods, applying that update still requires a healthy API server — hence the need to ensure access first

Copy link
Member

Choose a reason for hiding this comment

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

Can we rename this from cnx-access as part of this move? cnx is confusing and has no meaning.

Copy link
Member

Choose a reason for hiding this comment

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

OK if not, but if it's easy that would be nice.

@@ -408,6 +418,10 @@ func (r *ReconcileAPIServer) Reconcile(ctx context.Context, request reconcile.Re
return reconcile.Result{}, err
}

// Check if the legacy namespace 'tigera-system' exists and can be cleaned up
canCleanupOlderResources := false
Copy link
Member

Choose a reason for hiding this comment

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

Looks like line 422 can be removed. Optionally, 423 can be removed and called in-line @446.

@@ -2063,3 +2063,20 @@ func crdPoolsToOperator(crds []crdv1.IPPool) []operator.IPPool {
}
return pools
}

func allowTigeraDefaultDenyForCalicoSystem(namespace string) *v3.NetworkPolicy {
Copy link
Member

Choose a reason for hiding this comment

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

No need for passing a namespace.

@@ -1039,3 +1039,15 @@ func MaintainInstallationFinalizer(
// Update the installation with any finalizer changes.
return c.Patch(ctx, installation, patchFrom)
}

func IsTigeraStatusReady(ts *operatorv1.TigeraStatus, l logr.Logger) bool {
Copy link
Member

@rene-dekker rene-dekker Jun 6, 2025

Choose a reason for hiding this comment

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

Should we perhaps make an available() method on the object itself? Logging can be removed or handled outside of the func.

vara2504 added 2 commits June 10, 2025 15:41
Update default deny policy in calico-system to exclude API server pods
Create a separate CA bundle for the API server in calico-system
Clean up the legacy tigera-system namespace
Update all references from tigera-system to calico-system
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants