Skip to content

Add default anti affinity #810

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

Merged
merged 12 commits into from
Jul 18, 2025
Merged

Add default anti affinity #810

merged 12 commits into from
Jul 18, 2025

Conversation

jiangpengcheng
Copy link
Member

@jiangpengcheng jiangpengcheng commented Jul 2, 2025

(If this PR fixes a github issue, please add Fixes #<xyz>.)

Fixes #

(or if this PR is one task of a github issue, please add Master Issue: #<xyz> to link to the master issue.)

Master Issue: #

Motivation

Explain here the context, and why you're making that change. What is the problem you're trying to solve.

Modifications

Describe the modifications you've done.

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Documentation

Check the box below.

Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

1. prefer to be in different zones (soft rule)
2. must not be scheduled on the same node
Copy link

github-actions bot commented Jul 2, 2025

@jiangpengcheng:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@github-actions github-actions bot added doc-info-missing This pr needs to mark a document option in description and removed doc-info-missing This pr needs to mark a document option in description labels Jul 2, 2025
Copy link

github-actions bot commented Jul 2, 2025

@jiangpengcheng:Thanks for providing doc info!

@github-actions github-actions bot added the no-need-doc This pr does not need any document label Jul 2, 2025
@freeznet freeznet requested a review from Copilot July 8, 2025 14:47
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a feature flag to enable default pod anti-affinity for operator-managed pods, ensuring replicas avoid the same node and prefer distinct zones. Key changes:

  • Add AddDefaultAffinity config, CLI flag, and Helm value
  • Implement GenerateAffinity to inject default anti-affinity rules
  • Update integration tests and sample manifests to opt out of the new behavior

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
utils/configs.go Add AddDefaultAffinity global config var
main.go Introduce --add-default-affinity flag and wire through utils
controllers/spec/common.go Implement GenerateAffinity and apply to pod templates
charts/function-mesh-operator/values.yaml Add addDefaultAffinity value under controllerManager
charts/function-mesh-operator/templates/controller-manager-deployment.yaml Pass --add-default-affinity to the controller manager args
.ci/tests/integration/e2e_with_tls.yaml Set addDefaultAffinity=false in Helm install for TLS E2E
.ci/tests/integration/cases/python-function/manifests.yaml Increase replicas for sample to exercise anti-affinity logic
.ci/tests/kind.yaml Add extra worker nodes for zone-based scheduling tests
Comments suppressed due to low confidence (1)

controllers/spec/common.go:487

  • Add unit tests for GenerateAffinity to verify that both required and preferred anti-affinity rules are correctly added when AddDefaultAffinity is true and that no rules are added or mutated when false.
func GenerateAffinity(affinity *corev1.Affinity, labels map[string]string) *corev1.Affinity {

@@ -22,3 +22,4 @@ var EnableInitContainers = false
var GlobalBackendConfig = ""
var GlobalBackendConfigNamespace = "default"
var NamespacedBackendConfig = ""
var AddDefaultAffinity = true
Copy link
Member

Choose a reason for hiding this comment

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

Will a global flag be too broad? How can we stop adding the default affinity for selected instances or selected namespaces?

Copy link
Member Author

Choose a reason for hiding this comment

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

we can add a flag in CRD to disable it for each instance, but it's hard to set the flag per namespace

@jiangpengcheng jiangpengcheng merged commit 78e3688 into master Jul 18, 2025
13 checks passed
@jiangpengcheng jiangpengcheng deleted the add_default_anti_affinity branch July 18, 2025 07:04
freeznet pushed a commit that referenced this pull request Jul 24, 2025
* [feat] Add default affinities to ensure pods:
1. prefer to be in different zones (soft rule)
2. must not be scheduled on the same node

* add test

* fix npe

* fix vulnerabilities

* fix unittest

* fix unittest

* fix ci

* increase kind nodes in ci

* fix ci

* fix ci

* Add a `DisableDefaultAffinity` field to PodPolicy

* Update doc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-need-doc This pr does not need any document
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants