[DND-1443] HPA autoscaling/v2 + helm-polish cleanup, LICENSE, and 0.1.0 release - #24
Merged
Merged
Conversation
hpa.yaml emitted apiVersion: autoscaling/v2beta1, removed in Kubernetes 1.25.
On any cluster >= 1.25, autoscaling.enabled=true produced a
HorizontalPodAutoscaler the API server rejects; kubeconform (strict) also
failed it ("could not find schema for HorizontalPodAutoscaler").
- hpa.yaml: apiVersion autoscaling/v2beta1 -> autoscaling/v2, and the metrics
shape migrated to the v2 form (spec.metrics[].resource.target.type=Utilization
+ averageUtilization, instead of targetAverageUtilization).
- Add test-values/autoscaling.yaml (HPA enabled, CPU + memory targets) and
register it in the lint-render matrix so kubeconform now validates the HPA.
Previously no gated scenario enabled autoscaling, so the HPA was never
rendered or checked. helm-diff globs test-values/*.yaml and picks it up too.
No Chart.yaml version bump (deferred, handled separately), so verify-version
fails by design for now.
Verified: renders autoscaling/v2 with the v2 metrics shape; kubeconform -strict
(no HPA skip, CI-equivalent) validates all rendered resources; helm lint +
actionlint clean.
This comment has been minimized.
This comment has been minimized.
Run helm-polish (helm lint wrapper + opinionated style rules) and clear all
errors (0xxx + 1xxx default rule set). Template-source style only; the rendered
manifests are unchanged except that each backend properties block loses a
leading blank line (and the resulting checksum/config annotation recomputes).
- HP1101: template comments use leading-dash trimming ({{- /* ) in _helpers.tpl.
- HP1105/HP1106: the per-backend `include ... | nindent 4` lines in configmap.yaml
use a leading dash and are indented 4 spaces to match the nindent.
- HP1110: control-flow directives (if/with/end) indented by control-flow nesting
depth in _helpers.tpl (labels), deployment.yaml (podLabels), secret.yaml
(azureblob credentials). All are {{- }} so trimming makes these no-ops in
output.
Remaining: HP0104 "icon is recommended" (info) is left as-is; it needs an icon
asset/URL, which is a branding decision rather than a lint fix.
Verified: helm-polish reports 0 errors (--strict exits 0); helm lint clean;
kubeconform -strict valid across all test-values; render diff vs base is only
the cosmetic blank-line/checksum change described above.
This comment has been minimized.
This comment has been minimized.
The repo had no top-level LICENSE. Copy the Apache License 2.0 (with the 'Copyright (c) Comet ML, Inc' header) verbatim from comet-ml/common-helm-chart to the repo root, matching the licensing of the other Comet helm charts. Repo-root only; not under charts/s3proxy/, so the packaged chart is unaffected.
This comment has been minimized.
This comment has been minimized.
Reconciles the chart changes that merged without a version bump (DND-1442 #21, azureblob endpoint + multi-backend routing #22) together with this PR's HPA autoscaling/v2 fix, the helm-polish template cleanup, and the added LICENSE. Clears the verify-version gate and cuts a 0.1.0 release on merge (minor bump: new azureblob regions value + bucketLocators, no breaking changes).
📝 README.md PreviewThe following changes to Click to expand diffdiff --git a/README.md b/README.md
index b95b0fe..79b854b 100644
--- a/README.md
+++ b/README.md
@@ -1,6 +1,6 @@
# s3proxy
-  
+  
A Helm chart for deploying S3Proxy - Access other storage backends via the S3 API
|
📊 Helm Render Diff SummaryChart
🔍 Detailed Changes📝 Changes in
|
CRThaze
changed the title
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
1 participant
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.
This PR started as the DND-1443 HPA fix and now also carries a chart lint cleanup, a LICENSE, and the version bump that reconciles the recently merged chart changes. Four self-contained commits:
1. DND-1443: migrate
hpa.yamltoautoscaling/v2hpa.yamlemittedapiVersion: autoscaling/v2beta1, removed in Kubernetes 1.25. On any cluster at or above 1.25,autoscaling.enabled=trueproduced an HPA the API server rejects, andkubeconform -strictfailed it. Surfaced by the lint-render check added in DND-1417.hpa.yaml:autoscaling/v2beta1toautoscaling/v2, with the metrics migrated to the v2 shape (spec.metrics[].resource.target.type: Utilization+averageUtilization, replacingtargetAverageUtilization). Values API unchanged.test-values/autoscaling.yaml(HPA enabled, CPU + memory targets), registered in thelint-rendermatrix, so kubeconform now validates the HPA on every PR. Previously no gated scenario enabled autoscaling, which is why the bug shipped.helm-diffglobstest-values/*.yamland picks it up too.2. helm-polish template style cleanup
Ran
helm-polish(ahelm lintwrapper plus opinionated style rules) and cleared all findings in the default rule set (0 errors;--strictexits 0).HP1101: template comments use leading-dash trimming ({{- /*) in_helpers.tpl.HP1105/HP1106: the per-backendinclude ... | nindent 4lines inconfigmap.yamluse a leading dash and are indented to match the nindent.HP1110: control-flow directives (if/with/end) indented by control-flow nesting depth in_helpers.tpl,deployment.yaml, andsecret.yaml.These are template-source style only. The rendered manifests are unchanged except that each backend properties block loses a leading blank line (from the nindent dash-trim), which recomputes the
checksum/configannotation. All reindents are{{- }}trims and produce no output change. The one remaining helm-polish item isHP0104"icon is recommended" (info), left as-is because it needs an icon asset rather than a code fix.3. Add LICENSE (Apache-2.0)
The repo had no top-level LICENSE. Copied the Apache License 2.0 (with the
Copyright (c) Comet ML, Incheader) verbatim fromcomet-ml/common-helm-chart, matching the other Comet helm charts. Repo-root only, not undercharts/s3proxy/, so the packaged chart is unaffected.4. Bump chart version 0.0.8 to 0.1.0
Reconciles the chart changes that merged without a version bump (DND-1442 in #21, azureblob endpoint + multi-backend routing in #22) together with this PR's changes. Clears the
verify-versiongate and cuts a0.1.0release on merge. Minor bump: newazureblob.regionsvalue and per-backendbucketLocators, no breaking changes.Verification
helm-polish: 0 errors (--strictexits 0);helm lintclean.kubeconform -strict(k8s 1.29): valid across alltest-values, including the new autoscaling scenario (HPA validated).verify-versionpass.Note on sequencing
PR #23 (azureblob
regions) also changes the chart without its own bump. Once this PR merges and releases0.1.0, #23 will need its own bump (for example0.1.1) or a rebase onto this branch before itsverify-versionpasses.