From 456bc5e6c95df1f1029bb7f6b52624887319e8b5 Mon Sep 17 00:00:00 2001 From: German Arutyunov Date: Mon, 9 Nov 2026 11:04:46 +0100 Subject: [PATCH] fix: do not revert HPA-managed replicas of MCPServer deployments The MCPServer CRD defaulted spec.deployment.replicas to 1 and the controller fully replaced the Deployment on every reconcile. Because the controller also watches its Deployments, every scale event from a HorizontalPodAutoscaler triggered a reconcile that reset replicas to 1. Drop the CRD default and keep the live replica count when the MCPServer does not set one, matching how kagent handles Agent deployments. Co-Authored-By: Claude Opus 5.5 Signed-off-by: German Arutyunov --- api/v1alpha1/mcpserver_types.go | 5 +- config/crd/bases/kagent.dev_mcpservers.yaml | 5 +- helm/kmcp-crds/templates/mcpserver-crd.yaml | 5 +- pkg/controller/mcpserver_controller.go | 16 +++++ .../transportadapter_translator.go | 3 +- pkg/controller/upsert_output_test.go | 67 +++++++++++++++++++ 6 files changed, 94 insertions(+), 7 deletions(-) create mode 100644 pkg/controller/upsert_output_test.go diff --git a/api/v1alpha1/mcpserver_types.go b/api/v1alpha1/mcpserver_types.go index 95570e1..b04af5e 100644 --- a/api/v1alpha1/mcpserver_types.go +++ b/api/v1alpha1/mcpserver_types.go @@ -347,9 +347,10 @@ type MCPServerDeployment struct { NodeSelector map[string]string `json:"nodeSelector,omitempty"` // Replicas defines the number of desired pod replicas. - // Defaults to 1 if not specified. + // If not specified, the Deployment is created with 1 replica and its + // replica count is left untouched afterwards, so that it can be managed + // by a HorizontalPodAutoscaler. // +optional - // +kubebuilder:default=1 Replicas *int32 `json:"replicas,omitempty"` // ImagePullSecrets defines the list of secrets to use for pulling container images. diff --git a/config/crd/bases/kagent.dev_mcpservers.yaml b/config/crd/bases/kagent.dev_mcpservers.yaml index de1da0a..6b77293 100644 --- a/config/crd/bases/kagent.dev_mcpservers.yaml +++ b/config/crd/bases/kagent.dev_mcpservers.yaml @@ -1590,10 +1590,11 @@ spec: listen. type: integer replicas: - default: 1 description: |- Replicas defines the number of desired pod replicas. - Defaults to 1 if not specified. + If not specified, the Deployment is created with 1 replica and its + replica count is left untouched afterwards, so that it can be managed + by a HorizontalPodAutoscaler. format: int32 type: integer resources: diff --git a/helm/kmcp-crds/templates/mcpserver-crd.yaml b/helm/kmcp-crds/templates/mcpserver-crd.yaml index de1da0a..6b77293 100644 --- a/helm/kmcp-crds/templates/mcpserver-crd.yaml +++ b/helm/kmcp-crds/templates/mcpserver-crd.yaml @@ -1590,10 +1590,11 @@ spec: listen. type: integer replicas: - default: 1 description: |- Replicas defines the number of desired pod replicas. - Defaults to 1 if not specified. + If not specified, the Deployment is created with 1 replica and its + replica count is left untouched afterwards, so that it can be managed + by a HorizontalPodAutoscaler. format: int32 type: integer resources: diff --git a/pkg/controller/mcpserver_controller.go b/pkg/controller/mcpserver_controller.go index 12fd8a6..3d34e48 100644 --- a/pkg/controller/mcpserver_controller.go +++ b/pkg/controller/mcpserver_controller.go @@ -365,9 +365,25 @@ func upsertOutput(ctx context.Context, kube client.Client, output client.Object) } else { // If found, update it output.SetResourceVersion(existing.GetResourceVersion()) + preserveReplicas(existing, output) if err := kube.Update(ctx, output); err != nil { return err } } return nil } + +// preserveReplicas keeps the live replica count of a Deployment whose desired +// spec leaves replicas unset, so that a HorizontalPodAutoscaler scaling it is +// not reverted on every reconcile. +func preserveReplicas(existing, desired client.Object) { + existingDeployment, ok := existing.(*appsv1.Deployment) + if !ok { + return + } + desiredDeployment, ok := desired.(*appsv1.Deployment) + if !ok || desiredDeployment.Spec.Replicas != nil { + return + } + desiredDeployment.Spec.Replicas = existingDeployment.Spec.Replicas +} diff --git a/pkg/controller/transportadapter/transportadapter_translator.go b/pkg/controller/transportadapter/transportadapter_translator.go index c8e463f..9e96f42 100644 --- a/pkg/controller/transportadapter/transportadapter_translator.go +++ b/pkg/controller/transportadapter/transportadapter_translator.go @@ -303,7 +303,8 @@ func (t *transportAdapterTranslator) translateTransportAdapterDeployment( } } - // Determine replicas (default to 1) + // Leave replicas unset when not specified: a new Deployment defaults to 1, + // and upsertOutput keeps the live count on updates. var replicas *int32 if server.Spec.Deployment.Replicas != nil { replicas = server.Spec.Deployment.Replicas diff --git a/pkg/controller/upsert_output_test.go b/pkg/controller/upsert_output_test.go new file mode 100644 index 0000000..051d117 --- /dev/null +++ b/pkg/controller/upsert_output_test.go @@ -0,0 +1,67 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "testing" + + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestUpsertOutputReplicas(t *testing.T) { + tests := []struct { + name string + desired *int32 + want int32 + }{ + {name: "unset keeps the live count", desired: nil, want: 3}, + {name: "explicit value overrides the live count", desired: int32Ptr(2), want: 2}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + meta := metav1.ObjectMeta{Name: "mcp", Namespace: "default"} + kube := fake.NewClientBuilder().WithObjects(&appsv1.Deployment{ + ObjectMeta: meta, + Spec: appsv1.DeploymentSpec{Replicas: int32Ptr(3)}, + }).Build() + + desired := &appsv1.Deployment{ + ObjectMeta: meta, + Spec: appsv1.DeploymentSpec{Replicas: tt.desired}, + } + if err := upsertOutput(ctx, kube, desired); err != nil { + t.Fatalf("upsertOutput: %v", err) + } + + got := &appsv1.Deployment{} + if err := kube.Get(ctx, client.ObjectKeyFromObject(desired), got); err != nil { + t.Fatalf("get deployment: %v", err) + } + if got.Spec.Replicas == nil || *got.Spec.Replicas != tt.want { + t.Fatalf("replicas = %v, want %d", got.Spec.Replicas, tt.want) + } + }) + } +} + +func int32Ptr(i int32) *int32 { return &i }