Skip to content

Conversation

fe80
Copy link

@fe80 fe80 commented Apr 3, 2025

Summary

This feature add topologySpreadConstraints
support

Tests

cc:

@wangxiaoyou1993 wangxiaoyou1993 requested a review from Copilot May 9, 2025 04:35
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 adds support for Kubernetes topologySpreadConstraints to enable the scheduler and webServer deployments to better distribute pods across failure domains.

  • Added empty topologySpreadConstraints arrays with commented examples in values.yaml for both scheduler and webServer.
  • Updated scheduler.yaml and deployment.yaml templates to render topologySpreadConstraints with conditional logic to include label selectors.

Reviewed Changes

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

File Description
charts/mageai/values.yaml Adds topologySpreadConstraints arrays with sample commented configurations.
charts/mageai/templates/scheduler.yaml Implements topologySpreadConstraints rendering with conditional labelSelector inclusion for the scheduler.
charts/mageai/templates/deployment.yaml Implements topologySpreadConstraints rendering with conditional labelSelector inclusion for the webServer.
Comments suppressed due to low confidence (1)

charts/mageai/templates/scheduler.yaml:120

  • [nitpick] The helper function names differ between templates ('mageai.schedulerSelectorLabels' in scheduler.yaml versus 'mageai.selectorLabels' in deployment.yaml). If these perform similar tasks, consider standardizing their names for consistency.
{{- include "mageai.schedulerSelectorLabels" $ | nindent 14 }}

@fe80 fe80 force-pushed the feat/topology-spread branch from 49f09aa to 70f7c82 Compare August 27, 2025 07:48
@fe80 fe80 force-pushed the feat/topology-spread branch from 70f7c82 to 5ec1982 Compare August 27, 2025 07:48
@Poil
Copy link

Poil commented Sep 8, 2025

Up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants