added repo
This commit is contained in:
Your Name
@@ -0,0 +1,167 @@
|
||||
# Coding Guidelines — Helm values authoring
|
||||
|
||||
> The hard stops live in [../SANCTITY_RULES.md](../SANCTITY_RULES.md); the layer/scope rules in [../AGENT_BOUNDARIES.md](../AGENT_BOUNDARIES.md). This file is "how to write the values YAML well" — style, conventions, and the recurring footguns the linter doesn't catch.
|
||||
|
||||
---
|
||||
|
||||
## File-level conventions
|
||||
|
||||
### Where files go
|
||||
|
||||
| Path | Meaning |
|
||||
|------|---------|
|
||||
| `helm-overrides/<cluster>/<app>/custom-values.yaml` | The primary Helm values file Argo's `valueFiles` references. **Default name. Do not invent alternatives.** |
|
||||
| `helm-overrides/<cluster>/<app>/<extra>.yaml` | Sidecar raw manifests applied alongside the Helm release. Common shapes: `computeclass/<x>-cc.yaml`, `external-dns-services/<svc>.yaml`, `elastic-cluster/argo-launch.yaml`, `mimir-distributed/alertmanager_config.yaml`. |
|
||||
| `helm-templates/<chart>/Chart.yaml` | Chart manifest; usually a thin wrapper declaring an upstream dep. |
|
||||
| `helm-templates/<chart>/values.yaml` | The chart's own defaults — rarely edited; treat as upstream. |
|
||||
| `helm-templates/<chart>/templates/` | Manifest templates — vanilla upstream unless intentionally forked. **Don't edit casually.** |
|
||||
|
||||
### Cluster directory naming
|
||||
|
||||
The cluster directory's name is a contract — it must match the Kubernetes cluster name registered in Argo CD. Conventions:
|
||||
|
||||
| Pattern | Use |
|
||||
|---------|-----|
|
||||
| `k8s-<bu>-prd-ase1` | Standard GKE prod cluster, BU-owned (AWS-style naming retained). |
|
||||
| `k8s-<bu>-prd-ase1c` | GCP zone-c twin. |
|
||||
| `k8s-shared-int-ase1` | Shared int (pre-prod) cluster. The only non-prod cluster. |
|
||||
| `k8s-aurva-prd-ase1` | Aurva integration. |
|
||||
| `db-<numeric-id>-...` | Auto-named dataplane / data-tier clusters. Minimal override sets (typically `kube-state-metrics` + `victoria-metrics-agent`). |
|
||||
| `k8s-supply-dev-ase1` | The lone dev/sandbox cluster. |
|
||||
|
||||
### App directory naming
|
||||
|
||||
Inside `helm-overrides/<cluster>/`, each subdirectory is one Argo Application = one Helm release.
|
||||
|
||||
- One Helm release per directory.
|
||||
- Multiple Contour instances per cluster is the norm — `contour-external`, `contour-internal-0`, `contour-internal-1`, `contour-internal-intra-{0,1}`. Each maps to a different node pool / dedicated taint or compute class. **They are separate releases — don't merge them.**
|
||||
- Versioned siblings (`argo-cd` ↔ `argo-cd-green`, `keda` ↔ `keda-2.17.1`) live as parallel directories under `helm-templates/`, but their per-cluster overrides typically live under one directory until the migration cuts over. See [blue-green-chart-migration](../../platform/procedures/blue-green-chart-migration.md).
|
||||
|
||||
---
|
||||
|
||||
## Field-level conventions (`custom-values.yaml`)
|
||||
|
||||
### Image references
|
||||
|
||||
- **Always pin images to Meesho's Artifact Registry mirror** for production:
|
||||
```yaml
|
||||
image:
|
||||
registry: asia-southeast1-docker.pkg.dev
|
||||
repository: meesho-devops-admin-0622/admin/sre/<image>
|
||||
tag: <semver-or-sha>
|
||||
```
|
||||
- **Never use `:latest` or unpinned tags** in production overrides.
|
||||
- **Never reference Docker Hub, Quay, GCR upstream, or ECR directly** in a production override. Mirror it via the platform team's image-pull workflow first.
|
||||
|
||||
### `fullnameOverride`
|
||||
|
||||
- **Set it once, never change it.** Service DNS, PVC binding, ConfigMap references, and downstream Argo Application names depend on stability. ([SANCTITY_RULES R9](../SANCTITY_RULES.md))
|
||||
- For dataplane (`db-*`) clusters, the convention is `fullnameOverride: <kind>-dbc-<bu>-prd` (e.g. `kube-state-metrics-dbc-dsci-prd`).
|
||||
- For BU clusters, omit `fullnameOverride` unless the chart's default name collides with another release in the same namespace.
|
||||
|
||||
### `nameOverride`
|
||||
|
||||
Almost never needed. Helm's `<release>-<chart>` naming is usually fine.
|
||||
|
||||
### Replica counts and HPA bounds
|
||||
|
||||
- Read the chart's defaults before specifying replicas. Some charts have HPA-managed replicas that conflict with `replicaCount`.
|
||||
- For HPA-managed releases, set `minReplicas` and `maxReplicas` *and* leave `replicaCount` unset (or set to `null`).
|
||||
- Don't lower `minReplicas` to zero unless the workload genuinely scales-from-zero.
|
||||
|
||||
### Resource requests and limits
|
||||
|
||||
- **Always set `resources.requests`** for production releases. Without requests, the scheduler treats the pod as best-effort.
|
||||
- **Set `resources.limits`** unless the chart documentation explicitly recommends omitting them (some sidecars deliberately go limit-less).
|
||||
- **Don't copy resources from another cluster.** Workload sizing is per-traffic-tier; the supply prd cluster's Contour requests are not the demand prd cluster's.
|
||||
|
||||
### `nodeSelector`, `tolerations`, `affinity`, `topologySpreadConstraints`
|
||||
|
||||
The single biggest source of silent mis-deploys. Per [SANCTITY_RULES R5](../SANCTITY_RULES.md):
|
||||
|
||||
| Cluster type | Key style | Example |
|
||||
|--------------|-----------|---------|
|
||||
| GKE Autopilot (`k8s-central-prd-ase1`, `k8s-dsgpu-prd-ase1`, `k8s-shared-int-ase1`) | `cloud.google.com/compute-class` | `nodeSelector: {cloud.google.com/compute-class: contour-internal-0-cc}` |
|
||||
| Standard GKE | `dedicated:` | `nodeSelector: {dedicated: contour-internal-0}` |
|
||||
| Most others | `dedicated:` | same |
|
||||
|
||||
Cross-reference [`contour-nodeselector-tolerations-summary.md`](../../../contour-nodeselector-tolerations-summary.md) for the per-cluster Contour matrix. For non-Contour apps, copy from a sibling app on the *same* cluster, not from the same app on a *different* cluster.
|
||||
|
||||
### Probes
|
||||
|
||||
- Always set `livenessProbe` and `readinessProbe` for any long-running container.
|
||||
- For workloads that take >30 s to warm (Jenkins, JFrog, ClickHouse), bump `initialDelaySeconds` accordingly — not the timeout, not the period.
|
||||
- A `startupProbe` is the right tool for slow-warm containers; don't fight it with a 600s `initialDelaySeconds` on `livenessProbe`.
|
||||
|
||||
### Persistence
|
||||
|
||||
- StorageClass references go through the cluster-wide singletons in `manifests/storageclass/`: `pd-standard-retain-dr`, `sc-filestore-standard`, `sc-pd-ssd`, `sc-pd-standard`. **Never reference a StorageClass that doesn't exist in `manifests/storageclass/`.**
|
||||
- For stateful releases, set `persistence.size` explicitly. Default sizes are rarely right.
|
||||
- Never enable `persistence.enabled: true` without confirming the StorageClass and its retention/reclaim policy.
|
||||
|
||||
### Secrets
|
||||
|
||||
- **Never inline secret values** in `custom-values.yaml`. ([SANCTITY_RULES R4](../SANCTITY_RULES.md))
|
||||
- Reference secrets by name: `existingSecret: <secret-name>`, where the secret is materialised by the per-cluster `external-secrets` app from GCP Secret Manager / Vault.
|
||||
- Most clusters have an `external-secrets/` override directory; if your release needs a secret, the corresponding `ExternalSecret` lives there.
|
||||
|
||||
### Annotations and labels
|
||||
|
||||
- **Add labels conservatively.** Most charts already emit sensible label sets (`app.kubernetes.io/name`, etc.).
|
||||
- For ingress (`Ingress`, `HTTPProxy`, Contour `Service`), `external-dns` annotations and AWS/GCP load-balancer annotations are normal — copy from a sibling on the same cluster.
|
||||
- **Don't invent label keys.** If you find yourself adding `meesho.com/<something>`, double-check whether the project already has a convention for it.
|
||||
|
||||
---
|
||||
|
||||
## Field-level conventions (raw sidecar manifests)
|
||||
|
||||
For `helm-overrides/<cluster>/<app>/<extra>.yaml` files (no Helm templating, applied as-is):
|
||||
|
||||
- One Kubernetes resource per file unless they are tightly coupled.
|
||||
- Use `apiVersion: v1` etc. — pin the API version explicitly.
|
||||
- Set `metadata.namespace` (don't rely on the Argo Application's `destination.namespace` for these).
|
||||
- For `ComputeClass` / `NodeClass` / `BackendConfig` / GKE-specific resources, sample a sibling cluster's existing file before authoring.
|
||||
- For `external-dns-services/*.yaml`, the `Service` resource carries `external-dns.alpha.kubernetes.io/hostname` annotations — match the cluster's existing DNS pattern.
|
||||
|
||||
---
|
||||
|
||||
## YAML style
|
||||
|
||||
- **2-space indent. No tabs.**
|
||||
- **Use single quotes for `'*'`** and other glob-like strings; bare strings elsewhere where unambiguous.
|
||||
- **Trailing newline at EOF.**
|
||||
- **No `---` document separators** unless you genuinely need multi-document YAML (rare in this repo).
|
||||
- **Don't comment out fields; remove them.** The repo doesn't use commented-out scaffolding.
|
||||
- **Preserve key order from siblings.** A reordered file is a noisy diff that drowns the real change.
|
||||
- **Don't reformat unrelated YAML in passing.** ([SANCTITY_RULES R14](../SANCTITY_RULES.md))
|
||||
|
||||
---
|
||||
|
||||
## Diff hygiene
|
||||
|
||||
When opening a PR:
|
||||
|
||||
- **One change-type per PR.** Adding a service should not also "normalise labels on three other apps."
|
||||
- **Keep diffs minimal.** Don't reformat surrounding YAML.
|
||||
- **Cite the procedure followed** (link to one of `docs/platform/procedures/*.md`) in the PR description.
|
||||
- **Show the validation you ran** — `helm template`, `yamllint`, sibling-file diff, the kubectl context you ran a `helm diff` against.
|
||||
- **Pair the sister-repo PR** (`devops-infra-argo-config`) when adding a new app or cluster — link both.
|
||||
|
||||
---
|
||||
|
||||
## Common mistakes the hooks do **not** catch
|
||||
|
||||
These are the recurring footguns that pre-commit hooks won't flag:
|
||||
|
||||
1. **`nodeSelector` / `tolerations` / `computeClass` copied from the wrong cluster.** Pods stay `Pending`, or schedule on the wrong node pool.
|
||||
2. **`fullnameOverride` modified.** Downstream Service DNS resolves to nothing.
|
||||
3. **`spec.source.path` in the sister repo's `Application` not updated** to point at the new chart sibling after a blue-green migration.
|
||||
4. **Image tag pinned to Docker Hub** or Quay instead of the GAR mirror.
|
||||
5. **`replicaCount` set on an HPA-managed release.** HPA fights the static count.
|
||||
6. **`persistence.storageClass` referencing a class that doesn't exist** on this cluster — PVC stays `Pending` forever.
|
||||
7. **`existingSecret` referencing a secret the per-cluster `external-secrets` app doesn't create.** Pods crashloop on missing env.
|
||||
8. **`Chart.yaml` `dependencies[].version` bumped without `helm dependency update`.** Argo CD will use the lockfile and silently render the old version.
|
||||
9. **Edits inside `helm-templates/<chart>/templates/`** — silently fork the chart; clobbered on next upstream sync.
|
||||
10. **Sidecar raw-manifest namespace mismatch** with the Helm release's namespace — orphaned resources.
|
||||
|
||||
The agent's job is to be the second pair of eyes on every one of these.
|
||||
Reference in New Issue
Block a user