You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "younsl (via GitHub)" <gi...@apache.org> on 2026/07/22 14:32:18 UTC

[PR] feat(helm): add revisionHistoryLimit value for operator Deployment [superset-kubernetes-operator]

younsl opened a new pull request, #207:
URL: https://github.com/apache/superset-kubernetes-operator/pull/207

   ## Summary
   
   The Helm chart never set `spec.revisionHistoryLimit` on the operator Deployment, so it silently inherited the Kubernetes default of 10 old ReplicaSets with no way to tune it. This adds a `revisionHistoryLimit` chart value (default `10`) wired to the manager Deployment, letting operators lower ReplicaSet retention in namespaces that redeploy the operator frequently.
   
   ## Details
   
   - Add `revisionHistoryLimit` to `values.yaml` (helm-docs annotated) and `values.schema.json`.
   - Set `spec.revisionHistoryLimit` from the value in `templates/deployment.yaml`.
   - Regenerate the chart `README.md` via `make helm-docs`.
   - Note the addition under Unreleased in `docs/reference/releases.md`.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(helm): add revisionHistoryLimit value for operator Deployment [superset-kubernetes-operator]

Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #207:
URL: https://github.com/apache/superset-kubernetes-operator/pull/207#discussion_r3631512813


##########
charts/superset-operator/templates/deployment.yaml:
##########
@@ -22,6 +22,7 @@ metadata:
     {{- include "superset-operator.labels" . | nindent 4 }}
 spec:
   replicas: {{ .Values.replicas }}
+  revisionHistoryLimit: {{ .Values.revisionHistoryLimit }}

Review Comment:
   Let's make this conditional on `revisionHistoryLimit` actually being set:
   ```suggestion
       {{- if not (kindIs "invalid" .Values.revisionHistoryLimit) }}
       revisionHistoryLimit: {{ .Values.revisionHistoryLimit }}
       {{- end }}
   ```



##########
charts/superset-operator/values.yaml:
##########
@@ -9,6 +9,9 @@ image:
 # -- Number of operator manager replicas.
 replicas: 1
 
+# -- Number of old ReplicaSets to retain for rollback (Kubernetes default 10).
+revisionHistoryLimit: 10

Review Comment:
   Let's default to the property being unset, and refrain from repeating the current default value (you never know if it will change some day). Admins will be well aware of the default (and annoying 😆) default value.
   ```suggestion
   # -- Number of old ReplicaSets to retain for rollback. Leave unset to use the Kubernetes default. Set to 0 to disable rollback history.
   revisionHistoryLimit: ~
   ```



##########
docs/reference/releases.md:
##########
@@ -25,6 +25,7 @@ This page tracks notable changes in Apache Superset Kubernetes Operator releases
 
 ### Added
 
+- **Helm `revisionHistoryLimit`.** The Helm chart now exposes a `revisionHistoryLimit` value (default `10`) that sets `spec.revisionHistoryLimit` on the operator Deployment, capping the number of old ReplicaSets retained for rollback ([@younsl](https://github.com/younsl)).

Review Comment:
   Let's remove the explicit callout of current defaults. Something like this:
   ```suggestion
   - **Helm `revisionHistoryLimit`.** The Helm chart now exposes a `revisionHistoryLimit` value that allows setting `spec.revisionHistoryLimit` on the operator Deployment, capping the number of old ReplicaSets retained for rollback ([@younsl](https://github.com/younsl)).
   ```



##########
charts/superset-operator/values.schema.json:
##########
@@ -23,6 +23,7 @@
       }
     },
     "replicas": { "type": "integer", "minimum": 0 },
+    "revisionHistoryLimit": { "type": "integer", "minimum": 0 },

Review Comment:
   Let's accept null
   ```suggestion
       "revisionHistoryLimit": { "type": ["integer", "null"], "minimum": 0 },
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(helm): add revisionHistoryLimit value for operator Deployment [superset-kubernetes-operator]

Posted by "younsl (via GitHub)" <gi...@apache.org>.
younsl commented on PR #207:
URL: https://github.com/apache/superset-kubernetes-operator/pull/207#issuecomment-5052468192

   Thanks for the review @villebro! I've addressed all the comments — `revisionHistoryLimit` is now optional (defaults to unset), the schema accepts null, and I dropped the echoed defaults from the values, release notes, and added a note to the developer guidelines. Ready for another look when you get a chance.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(helm): add revisionHistoryLimit value for operator Deployment [superset-kubernetes-operator]

Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro merged PR #207:
URL: https://github.com/apache/superset-kubernetes-operator/pull/207


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org