feat(chart): isolate runtime and provider-aware chart hardening#73
Open
anchapin wants to merge 2 commits into
Open
feat(chart): isolate runtime and provider-aware chart hardening#73anchapin wants to merge 2 commits into
anchapin wants to merge 2 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nd values - Switch db and redis Deployments from RollingUpdate to Recreate strategy to prevent Multi-Attach errors on RWO PVCs during helm upgrade - Upgrade web-hpa from deprecated autoscaling/v1 to autoscaling/v2 with behavior.scaleDown.stabilizationWindowSeconds block - Clear hardcoded global.provider.name: openstack from values.yaml (now empty); fail guard enforces explicit provider selection on every deployment - Rename nfs_pvc.storage_class -> nfs_pvc.storageClass for camelCase consistency; template supports both keys for backward compatibility - Add inline comments documenting web_hpa.maxReplicas: 1 file-locking requirement - Add inline comments documenting that worker_hpa.maxReplicas drives both HPA scaling and web MAX_REQUESTS (x1.05) - Add PVC lookup guards to preserve immutable fields on upgrade - Update app-secrets.yaml template Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR restructures the Helm chart’s configuration and templates to enforce provider-aware scheduling defaults, tighten runtime hardening (secrets, lifecycle, hooks), and add OpenStack-specific support paths (autoscaler, storage class defaults).
Changes:
- Introduces
global.*configuration (provider, images, node group scheduling, storage class defaults) and updates baseline/template values accordingly. - Refactors workload templates (web/worker/web-background/rserve/db/redis) to use shared helper functions, provider-aware affinity, priority classes, and secret-backed env vars.
- Adds new operational components: autoscaler templating/guards, pre-delete cleanup hook RBAC+Job hardening, and secret validation/creation logic.
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| openstudio-server/values.yaml | New baseline values layout (global/provider, secrets, autoscaler, sizing, hooks). |
| openstudio-server/values_small.templateyaml | Updates small template to match new global.* + secrets/hooks conventions. |
| openstudio-server/values_large.templateyaml | Updates large template to match new global.* + secrets/hooks conventions. |
| openstudio-server/values_production.templateyaml | Adds a production-oriented example values file aligned to the new schema. |
| openstudio-server/templates/_scheduling.tpl | Adds shared helper templates for provider, naming, affinity, images, storage defaults, and secret keys. |
| openstudio-server/templates/autoscaler/cluster-autoscaler-autodiscover.yaml | Provider-aware autoscaler configuration with OpenStack-specific guards and mounts. |
| openstudio-server/templates/db/db-deploy.yaml | Switches to helper-based names/labels, priority classes, affinity, and secret-backed credentials. |
| openstudio-server/templates/db/db-pvc.yaml | Makes PVC more upgrade-safe by preserving existing accessModes/storageClass where possible. |
| openstudio-server/templates/db/db-svc.yaml | Uses helper naming and safer port defaults. |
| openstudio-server/templates/hooks/pre-delete-hook.yaml | Reworks pre-delete cleanup into namespace-scoped RBAC + hardened kubectl Job. |
| openstudio-server/templates/loadbalancer/loadbalancer.yaml | Provider-aware annotations/defaults plus support for extra annotations/source ranges. |
| openstudio-server/templates/nfs/nfs-pvc.yaml | Improves PVC defaults and backward compatibility for storageClass keys. |
| openstudio-server/templates/priority-class/priority_high.yaml | Makes priority class creation configurable (name/create), used by workloads. |
| openstudio-server/templates/priority-class/priority_low.yaml | Makes priority class creation configurable (name/create), used by workloads. |
| openstudio-server/templates/redis/redis-deploy.yaml | Uses recreate strategy for RWO PVCs; moves password into secret-backed env var. |
| openstudio-server/templates/redis/redis-pvc.yaml | Preserves existing PVC settings and adds safer defaults/quoting. |
| openstudio-server/templates/redis/redis-svc.yaml | Uses helper naming and safer defaults for port. |
| openstudio-server/templates/rserve/rserve-deploy.yaml | Adds affinity/priority class usage and secret-backed Redis/Mongo/Web secrets. |
| openstudio-server/templates/rserve/rserve-svc.yaml | Uses helper naming and adjusts selector/port defaults. |
| openstudio-server/templates/secrets/app-secrets.yaml | Adds secret validation and optional secret creation path with required key checks. |
| openstudio-server/templates/storageclass/storageclass.yaml | Creates ssd storage class only when needed, with provider-aware provisioners. |
| openstudio-server/templates/web/web-deploy.yaml | Uses helpers, provider-aware affinity, priority classes, secret-backed env vars, and MAX_REQUESTS sizing. |
| openstudio-server/templates/web/web-hpa.yaml | Migrates web HPA to autoscaling/v2 with behavior + metrics config. |
| openstudio-server/templates/web/web-svc.yaml | Switches to helper naming and safer default port/selector logic. |
| openstudio-server/templates/web-background/web-background-deploy.yaml | Adds affinity/priority class usage, secret-backed env vars, and startup retry loop. |
| openstudio-server/templates/worker/worker-deploy.yaml | Adds affinity/priority class usage, secret-backed env vars, startup retry loop, and configurable preStop. |
| openstudio-server/templates/worker/worker-hpa.yaml | Uses helper naming and adds keep-policy annotation. |
| openstudio-server/charts/nfs-server-provisioner/values.yaml | Removes hard-coded affinity and defers to parent/provider-aware defaults. |
| openstudio-server/charts/nfs-server-provisioner/templates/deployment.yaml | Applies provider-aware affinity when subchart affinity is not set. |
| openstudio-server/charts/nfs-server-provisioner/templates/pvc.yaml | Adds provider-aware default storage class selection and preserves existing accessModes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| size: 110Gi | ||
| # Backend storage for the in-cluster NFS server (nfs-pvc-data). | ||
| # Keep nfs_pvc.storage below this value to avoid NFS provisioning failures. | ||
| size: 1.5Ti # Increased from 5Gi for production |
| size: 100Gi | ||
| accessModes: | ||
| - "ReadWriteOnce" | ||
| size: 300Gi # Increased from 2Gi for production data |
| storageClass: "nfs" | ||
| # Frontend shared RWX claim consumed by web/rserve/background pods. | ||
| # Keep this below nfs-server-provisioner.persistence.size (recommended 85-95%). | ||
| storage: "300Gi" # Increased from 2Gi for production shared storage |
| label: "redis" | ||
| port: 6379 | ||
| url: "redis://:openstudio@queue:6379" | ||
| size: 300Gi # Increased from 1Gi for production queues |
|
|
||
|
|
||
|
|
||
| minReplicas: 1 # Increased from 2 for baseline production capacity |
Comment on lines
+1
to
+4
| {{- $priorityClasses := default (dict) .Values.priorityClasses -}} | ||
| {{- if (default true (get $priorityClasses "create")) -}} | ||
| {{- $highName := default "high-priority" (get $priorityClasses "highName") -}} | ||
| {{- if not (lookup "scheduling.k8s.io/v1" "PriorityClass" "" $highName) }} |
Comment on lines
+1
to
+4
| {{- $priorityClasses := default (dict) .Values.priorityClasses -}} | ||
| {{- if (default true (get $priorityClasses "create")) -}} | ||
| {{- $lowName := default "low-priority" (get $priorityClasses "lowName") -}} | ||
| {{- if not (lookup "scheduling.k8s.io/v1" "PriorityClass" "" $lowName) }} |
Comment on lines
9
to
11
| selector: | ||
| app: {{ .Values.web.name }} | ||
| app: {{ default (default "web" (get $web "label")) (get $webSvc "label") }} | ||
| release: {{ .Release.Name }} |
Comment on lines
6
to
8
| selector: | ||
| app: {{ .Values.rserve.label }} | ||
| app: {{ default (default "rserve" .Values.rserve.label) .Values.rserve_svc.label }} | ||
| release: {{ .Release.Name }} |
Comment on lines
+5
to
+6
| annotations: | ||
| helm.sh/resource-policy: keep |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR isolates Helm chart runtime hardening and provider-aware template changes.
Scope
openstudio-server/templates/**Why split
Allows focused review of chart behavior, scheduling/affinity, provider handling, and runtime safety changes without mixing OpenStack onboarding/scripts/docs.