From 8e52f56f0f108d9e5f70ce5c6b07961215602be8 Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Fri, 19 Jun 2026 22:04:01 +0400 Subject: [PATCH] fix(k8s): custom-stack VPA/HPA name double-suffixed parentEnv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DeploySimpleContainer already applies the parentEnv suffix to Service/Deployment for custom stacks (parentEnv != stackEnv), e.g. myapp -> myapp-tenant-a. But NewSimpleContainer re-derived baseResourceName via generateDeploymentName on the already-suffixed name, producing myapp-tenant-a-tenant-a. baseResourceName names the VPA (resource name AND spec.targetRef) and the HPA, so for every custom stack the VPA targeted a non-existent deployment and never right-sized the pods (they stayed pinned at the base request). The Deployment itself was always correct (it uses sanitizedDeployment directly), which is why the mismatch went unnoticed. Fix: baseResourceName := sanitizedDeployment — the caller already made it parentEnv-aware. No-op for non-custom stacks; fixes the VPA targetRef and tidies the HPA name for custom stacks. Documented the caller pre-suffix contract on NewSimpleContainer so it can't regress. On the next deploy of a custom stack, Pulumi replaces the VPA and HPA (old double-suffixed logical names removed, correctly-named ones created); the HPA's scaleTargetRef already pointed at the real Deployment object, so scaling is unaffected. VPA/HPA carry no RetainOnDelete/aliases, so the orphans are cleanly deleted (not stranded). The configmap/secret/image-pull-secret names remain double-suffixed for custom stacks (self-consistent: the pod references the same generated names), so they keep working; renaming them is a separate migration with pod-rollout cost. Test: TestNewSimpleContainer_CustomStackVPATargetsDeployment replicates the production caller contract (pre-suffixed Service/Deployment for a custom stack) and asserts the rendered VPA's spec.targetRef.name + name are single-suffixed. Adds the VPA's GVK type token to the test mock so the CustomResource is captured. Confirmed it fails on the bug and passes with the fix; full package green. Signed-off-by: Dmitrii Creed --- .../kubernetes/custom_stack_vpa_test.go | 89 +++++++++++++++++++ .../pulumi/kubernetes/simple_container.go | 8 +- .../kubernetes/simple_container_mocks_test.go | 3 +- 3 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 pkg/clouds/pulumi/kubernetes/custom_stack_vpa_test.go diff --git a/pkg/clouds/pulumi/kubernetes/custom_stack_vpa_test.go b/pkg/clouds/pulumi/kubernetes/custom_stack_vpa_test.go new file mode 100644 index 00000000..4ff06d62 --- /dev/null +++ b/pkg/clouds/pulumi/kubernetes/custom_stack_vpa_test.go @@ -0,0 +1,89 @@ +package kubernetes + +import ( + "strings" + "testing" + + "github.com/samber/lo" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + corev1 "github.com/pulumi/pulumi-kubernetes/sdk/v4/go/kubernetes/core/v1" + sdk "github.com/pulumi/pulumi/sdk/v3/go/pulumi" + + "github.com/simple-container-com/api/pkg/api/logger" + "github.com/simple-container-com/api/pkg/clouds/k8s" +) + +// Regression: DeploySimpleContainer passes the already parentEnv-suffixed name as +// Service/Deployment (e.g. "myapp-tenant-a" for ScEnv=tenant-a, parentEnv=production). +// NewSimpleContainer must derive the VPA name AND its spec.targetRef from that name +// directly, not re-apply the suffix. Re-applying produced "myapp-tenant-a-tenant-a", +// so a custom-stack VPA targeted a non-existent deployment and never right-sized. +func TestNewSimpleContainer_CustomStackVPATargetsDeployment(t *testing.T) { + mocks := NewSimpleContainerMocks() + err := sdk.RunErr(func(ctx *sdk.Context) error { + args := &SimpleContainerArgs{ + Namespace: "myapp", + Service: "myapp-tenant-a", + Deployment: "myapp-tenant-a", + ScEnv: "tenant-a", + ParentEnv: lo.ToPtr("production"), + Domain: "tenant-a.example.com", + Prefix: "/", + Replicas: 2, + IngressContainer: &k8s.CloudRunContainer{ + Name: "myapp-tenant-a", + MainPort: lo.ToPtr(8080), + Ports: []int{8080}, + }, + Log: logger.New(), + Containers: []corev1.ContainerArgs{ + {Name: sdk.String("myapp-tenant-a"), Image: sdk.String("nginx:latest")}, + }, + Scale: &k8s.Scale{EnableHPA: true, MinReplicas: 2, MaxReplicas: 4, CPUTarget: lo.ToPtr(80)}, + VPA: &k8s.VPAConfig{ + Enabled: true, + UpdateMode: lo.ToPtr("Auto"), + MinAllowed: &k8s.VPAResourceRequirements{CPU: lo.ToPtr("250m")}, + }, + } + _, err := NewSimpleContainer(ctx, args) + return err + }, sdk.WithMocks("test", "test", mocks)) + require.NoError(t, err) + + mocks.mu.RLock() + defer mocks.mu.RUnlock() + + // The VPA must target the actual (single-suffixed) Deployment, not a double-suffixed name. + // Only VPA/HPA names derive from baseResourceName (the fix); the configmap/secret names + // are intentionally left as-is, so scope the double-suffix guard to VPA/HPA. + foundVPA := false + for id, props := range mocks.createdResources { + if strings.HasSuffix(id, "-vpa-cr") || strings.Contains(id, "-hpa") { + assert.NotContains(t, id, "tenant-a-tenant-a", "VPA/HPA name double-suffixed: %s", id) + } + + kind, ok := props["kind"] + if !ok || !kind.IsString() || kind.StringValue() != "VerticalPodAutoscaler" { + continue + } + foundVPA = true + + require.True(t, props["metadata"].IsObject(), "VPA metadata should be an object") + vpaName := props["metadata"].ObjectValue()["name"] + require.True(t, vpaName.IsString()) + assert.Equal(t, "myapp-tenant-a-vpa", vpaName.StringValue(), "VPA name double-suffixed") + + require.True(t, props["spec"].IsObject(), "VPA spec should be an object") + targetRef := props["spec"].ObjectValue()["targetRef"] + require.True(t, targetRef.IsObject(), "VPA spec.targetRef should be an object") + refName := targetRef.ObjectValue()["name"] + require.True(t, refName.IsString()) + assert.Equal(t, "myapp-tenant-a", refName.StringValue(), + "VPA must target the real Deployment, not a double-suffixed name") + assert.False(t, strings.Contains(refName.StringValue(), "tenant-a-tenant-a")) + } + require.True(t, foundVPA, "VPA should have been created and captured by the mock") +} diff --git a/pkg/clouds/pulumi/kubernetes/simple_container.go b/pkg/clouds/pulumi/kubernetes/simple_container.go index 2954d223..5c7d5ebf 100644 --- a/pkg/clouds/pulumi/kubernetes/simple_container.go +++ b/pkg/clouds/pulumi/kubernetes/simple_container.go @@ -169,6 +169,9 @@ type SimpleContainer struct { Deployment *v1.Deployment `pulumi:"deployment"` } +// NewSimpleContainer provisions a stack's Deployment, Service, VPA/HPA and related resources. +// Callers MUST pass args.Service/args.Deployment already parentEnv-suffixed (see generateDeploymentName); +// child resource names are derived from them as-is, so re-suffixing here would double-suffix custom stacks. func NewSimpleContainer(ctx *sdk.Context, args *SimpleContainerArgs, opts ...sdk.ResourceOption) (*SimpleContainer, error) { sc := &SimpleContainer{} @@ -325,8 +328,9 @@ func NewSimpleContainer(ctx *sdk.Context, args *SimpleContainerArgs, opts ...sdk secretVolumeToData[secretVolume.Name] = secretVolume.Content } + // args.Deployment is already parentEnv-suffixed by the caller; use it directly so custom-stack VPA/HPA names aren't double-suffixed. + baseResourceName := sanitizedDeployment // Generate resource names with parentEnv-aware logic - baseResourceName := generateDeploymentName(sanitizedService, args.ScEnv, parentEnv) volumesCfgName := generateConfigVolumesName(sanitizedService, args.ScEnv, parentEnv) envSecretName := generateSecretName(sanitizedService, args.ScEnv, parentEnv) volumesSecretName := generateSecretVolumesName(sanitizedService, args.ScEnv, parentEnv) @@ -986,7 +990,7 @@ ${proto}://${domain} { // Create HPA if enabled (validation already done in deployment.go) if args.Scale != nil && args.Scale.EnableHPA { hpaArgs := &HPAArgs{ - Name: baseResourceName, // Uses parentEnv-aware name + Name: baseResourceName, Deployment: deployment, MinReplicas: args.Scale.MinReplicas, MaxReplicas: args.Scale.MaxReplicas, diff --git a/pkg/clouds/pulumi/kubernetes/simple_container_mocks_test.go b/pkg/clouds/pulumi/kubernetes/simple_container_mocks_test.go index 559809c4..32aced4d 100644 --- a/pkg/clouds/pulumi/kubernetes/simple_container_mocks_test.go +++ b/pkg/clouds/pulumi/kubernetes/simple_container_mocks_test.go @@ -65,7 +65,8 @@ func (m *simpleContainerMocks) NewResource(args pulumi.MockResourceArgs) (string return m.mockPDB(args, outputs) case "kubernetes:autoscaling/v2:HorizontalPodAutoscaler": return m.mockHPA(args, outputs) - case "kubernetes:apiextensions.k8s.io/v1:CustomResource": + case "kubernetes:apiextensions.k8s.io/v1:CustomResource", + "kubernetes:autoscaling.k8s.io/v1:VerticalPodAutoscaler": return m.mockCustomResource(args, outputs) default: return resourceID, resource.NewPropertyMapFromMap(outputs), nil