Skip to content

feat: optionally distribute replicas across architectures #153

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

Open
bschwedler wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: optionally distribute replicas across architectures #153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
315a5d0
feat: add topologySpreadConstraints to Connect, Workbench, PM
bschwedler Jul 23, 2026
50efae9
fix: default topologySpreadConstraints labelSelector
bschwedler Jul 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions api/core/v1beta1/connect_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ type ConnectSpec struct {

NodeSelector map[string]string `json:"nodeSelector,omitempty"`

// TopologySpreadConstraints controls how server pod replicas are spread across topology
// domains (e.g. node architecture, zone, hostname). Composes with the operator's built-in
// hostname-keyed soft anti-affinity.
TopologySpreadConstraints []corev1.TopologySpreadConstraint `json:"topologySpreadConstraints,omitempty"`

// AddEnv adds arbitrary environment variables to the container env
//
// Deprecated: use envVars instead. If the same variable name is set in both
Expand Down
5 changes: 5 additions & 0 deletions api/core/v1beta1/packagemanager_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ type PackageManagerSpec struct {
// If unset, product defaults are applied. Setting this replaces the defaults entirely.
Resources *v1.ResourceRequirements `json:"resources,omitempty"`

// TopologySpreadConstraints controls how server pod replicas are spread across topology
// domains (e.g. node architecture, zone, hostname). Composes with the operator's built-in
// hostname-keyed soft anti-affinity.
TopologySpreadConstraints []v1.TopologySpreadConstraint `json:"topologySpreadConstraints,omitempty"`

// AddEnv adds arbitrary environment variables to the container env
//
// Deprecated: use envVars instead. If the same variable name is set in both
Expand Down
66 changes: 46 additions & 20 deletions api/core/v1beta1/shared.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,29 @@ type SecretConfig struct {
Type product.SiteSecretType `json:"type,omitempty"`
}

// componentSelector is the label-identity selector for a component's own pods —
// shared by anti-affinity and topology spread so both target the same pod set.
func componentSelector(p product.KubernetesLabelser) *metav1.LabelSelector {
return &metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: KubernetesInstanceLabelKey,
Operator: metav1.LabelSelectorOpIn,
Values: []string{
p.KubernetesLabels()[KubernetesInstanceLabelKey],
},
},
{
Key: SiteLabelKey,
Operator: metav1.LabelSelectorOpIn,
Values: []string{
p.KubernetesLabels()[SiteLabelKey],
},
},
},
}
}

// ComponentSpecPodAntiAffinity generates a *corev1.PodAntiAffinity suitable for use in a
// given component's deployment template spec to inform kubernetes to place pod replicas
// on separate nodes when possible.
Expand All @@ -77,28 +100,31 @@ func ComponentSpecPodAntiAffinity(p product.KubernetesLabelser, namespace string
{
Weight: 1,
PodAffinityTerm: corev1.PodAffinityTerm{
TopologyKey: "kubernetes.io/hostname",
Namespaces: []string{namespace},
LabelSelector: &metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: KubernetesInstanceLabelKey,
Operator: metav1.LabelSelectorOpIn,
Values: []string{
p.KubernetesLabels()[KubernetesInstanceLabelKey],
},
},
{
Key: SiteLabelKey,
Operator: metav1.LabelSelectorOpIn,
Values: []string{
p.KubernetesLabels()[SiteLabelKey],
},
},
},
},
TopologyKey: "kubernetes.io/hostname",
Namespaces: []string{namespace},
LabelSelector: componentSelector(p),
},
},
},
}
}

// ComponentSpecTopologySpreadConstraints defaults any constraint's LabelSelector to
// componentSelector when the caller left it nil. A nil LabelSelector matches zero pods
// (Kubernetes semantics: metav1.LabelSelectorAsSelector(nil) returns labels.Nothing()),
// silently turning maxSkew/whenUnsatisfiable into a no-op. This exists so the common case
// (spread this component's own replicas) works without the caller re-deriving labels the
// operator already owns; an explicit caller-supplied LabelSelector is always left untouched.
func ComponentSpecTopologySpreadConstraints(
constraints []corev1.TopologySpreadConstraint,
p product.KubernetesLabelser,
) []corev1.TopologySpreadConstraint {
out := make([]corev1.TopologySpreadConstraint, len(constraints))

@claude claude Bot Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The componentSelector uses MatchExpressions (specifically In) rather than MatchLabels. Both produce equivalent Kubernetes behavior, but the anti-affinity helper directly above uses MatchExpressions for the same reason — so this is consistent.

One thing to note: the selector is built from KubernetesLabels() keys app.kubernetes.io/instance and posit.team/site. These are the selector labels, so they're stable and correct for pod matching. Looks good.

for i, c := range constraints {
if c.LabelSelector == nil {
c.LabelSelector = componentSelector(p)
}
out[i] = c
}
return out

@claude claude Bot Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: When constraints is nil, this returns a non-nil empty slice (make([]corev1.TopologySpreadConstraint, 0)). Kubernetes treats both the same, but if any downstream code does a == nil check on the result, it would surprise. Consider early-returning nil when the input is empty:

Suggested change
return out
func ComponentSpecTopologySpreadConstraints(
constraints []corev1.TopologySpreadConstraint,
p product.KubernetesLabelser,
) []corev1.TopologySpreadConstraint {
if len(constraints) == 0 {
return nil
}
out := make([]corev1.TopologySpreadConstraint, len(constraints))
for i, c := range constraints {
if c.LabelSelector == nil {
c.LabelSelector = componentSelector(p)
}
out[i] = c
}
return out
}

@claude claude Bot Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Important: When constraints is nil (no constraints configured), this returns a non-nil empty slice (make([]corev1.TopologySpreadConstraint, 0)). Kubernetes treats an explicit empty slice the same as nil here, so this is harmless — but it's worth knowing that callers passing nil get back []TopologySpreadConstraint{} rather than nil. If downstream code ever does a == nil check on the result, it would behave unexpectedly.

Consider returning nil when the input is empty:

Suggested change
return out
func ComponentSpecTopologySpreadConstraints(
constraints []corev1.TopologySpreadConstraint,
p product.KubernetesLabelser,
) []corev1.TopologySpreadConstraint {
if len(constraints) == 0 {
return nil
}
out := make([]corev1.TopologySpreadConstraint, len(constraints))
for i, c := range constraints {
if c.LabelSelector == nil {
c.LabelSelector = componentSelector(p)
}
out[i] = c
}
return out
}

}
15 changes: 15 additions & 0 deletions api/core/v1beta1/site_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ type InternalPackageManagerSpec struct {
// If unset, product defaults are applied. Setting this replaces the defaults entirely.
Resources *corev1.ResourceRequirements `json:"resources,omitempty"`

// TopologySpreadConstraints controls how server pod replicas are spread across topology
// domains (e.g. node architecture, zone, hostname). Composes with the operator's built-in
// hostname-keyed soft anti-affinity.
TopologySpreadConstraints []corev1.TopologySpreadConstraint `json:"topologySpreadConstraints,omitempty"`

// Deprecated: use envVars instead. If the same variable name is set in both
// addEnv and envVars, envVars takes precedence: it is rendered after addEnv,
// and Kubernetes resolves a duplicate env var name to the last occurrence.
Expand Down Expand Up @@ -288,6 +293,11 @@ type InternalConnectSpec struct {
// If unset, product defaults are applied. Setting this replaces the defaults entirely.
Resources *corev1.ResourceRequirements `json:"resources,omitempty"`

// TopologySpreadConstraints controls how server pod replicas are spread across topology
// domains (e.g. node architecture, zone, hostname). Composes with the operator's built-in
// hostname-keyed soft anti-affinity.
TopologySpreadConstraints []corev1.TopologySpreadConstraint `json:"topologySpreadConstraints,omitempty"`

@claude claude Bot Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The InternalConnectSpec.TopologySpreadConstraints field is missing a // +optional kubebuilder marker. All optional slice fields in this file use it (e.g. AdditionalRuntimeImages at line 350). Without it, the CRD schema technically marks the field as required in some generators.

Same applies to the TopologySpreadConstraints fields added to InternalPackageManagerSpec (line 223) and InternalWorkbenchSpec (line 434).

Fix this →


Auth AuthSpec `json:"auth,omitempty"`

// RegisterOnFirstLogin controls whether new users are automatically registered
Expand Down Expand Up @@ -418,6 +428,11 @@ type InternalWorkbenchSpec struct {
// If unset, product defaults are applied. Setting this replaces the defaults entirely.
Resources *corev1.ResourceRequirements `json:"resources,omitempty"`

// TopologySpreadConstraints controls how server pod replicas are spread across topology
// domains (e.g. node architecture, zone, hostname). Composes with the operator's built-in
// hostname-keyed soft anti-affinity. Applies only to the server Deployment, not sessions.
TopologySpreadConstraints []corev1.TopologySpreadConstraint `json:"topologySpreadConstraints,omitempty"`

// Tolerations that are applied universally to server and sessions
Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

Expand Down
5 changes: 5 additions & 0 deletions api/core/v1beta1/workbench_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ type WorkbenchSpec struct {
// If unset, product defaults are applied. Setting this replaces the defaults entirely.
Resources *corev1.ResourceRequirements `json:"resources,omitempty"`

// TopologySpreadConstraints controls how server pod replicas are spread across topology
// domains (e.g. node architecture, zone, hostname). Composes with the operator's built-in
// hostname-keyed soft anti-affinity. Applies only to the server Deployment, not sessions.
TopologySpreadConstraints []corev1.TopologySpreadConstraint `json:"topologySpreadConstraints,omitempty"`

Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

// AddEnv adds arbitrary environment variables to the container env
Expand Down
42 changes: 42 additions & 0 deletions api/core/v1beta1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions client-go/applyconfiguration/core/v1beta1/connectspec.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

61 changes: 36 additions & 25 deletions client-go/applyconfiguration/core/v1beta1/internalconnectspec.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading