fix(helm): correct logging CM, add webhook replicas, add extraObjects#3661
Open
aqeelat wants to merge 7 commits into
Open
fix(helm): correct logging CM, add webhook replicas, add extraObjects#3661aqeelat wants to merge 7 commits into
aqeelat wants to merge 7 commits into
Conversation
tekton-robot
added
the
release-note
tekton-robot
added
the
size/M
Member
|
hi @aqeelat , Thank you for your PR, can you go through the easyCLA please. |
Author
|
@jkhelil done |
jkhelil
reviewed
Jul 8, 2026
The logging ConfigMap used inert loglevel.controller / loglevel.webhook keys, copy-pasted from knative-serving. The Tekton operator's actual logger names are its unique-process-names (tekton-operator-lifecycle, tekton-operator-cluster-operations) and its webhook service name (<fullname>-webhook), so the old keys matched nothing and the effective level fell back to the global zap-logger-config, which was hardcoded to debug/development in production. Replace the inert keys with the operator's real logger names and drive the global zap-logger-config from operator.logLevel (default info, development false). The webhook key uses fullname (not operator-name) because the webhook derives its logger name from WEBHOOK_SERVICE_NAME, which is fullname-based. Also set CONFIG_LOGGING_NAME on the operator containers so they consume this ConfigMap; previously they fell back to knative's default name (config-logging), which this chart does not create, leaving all logging config inert for the operator containers. Signed-off-by: Abdullah Alaqeel <abdullah.t.aqeel@gmail.com>
The webhook Deployment hardcoded replicas: 1. Expose it through webhook.replicas (default 1 in values.yaml). The operator Deployment is left at a fixed replica count of 1 because the operator does not run leader election, so scaling its reconcilers horizontally would cause duplicate writes and status flapping. Signed-off-by: Abdullah Alaqeel <abdullah.t.aqeel@gmail.com>
Render arbitrary user-supplied manifests from values.extraObjects, matching the standard pattern used by charts like argoproj/argo-cd. Lets consumers deploy resources the chart deliberately does not template, most notably a TektonConfig CR. Defaults to an empty list, which renders nothing. Signed-off-by: Abdullah Alaqeel <abdullah.t.aqeel@gmail.com>
Quote the three loglevel.* ConfigMap values so a non-string value (false, null, a bare number) renders as a YAML string rather than being type-coerced. Matches the already-quoted `level` field in zap-logger-config. Addresses review feedback. Signed-off-by: Abdullah Alaqeel <abdullah.t.aqeel@gmail.com>
aqeelat
force-pushed
the
fix/chart-logging-replicas-extraobjects
branch
from
8bbe1e0 to
7010203
Compare
A webhook scaled to 0 pods has no endpoints, so admission calls time out and fail. Under failurePolicy: Fail this wedges operations on the matched CRs. Guard with `max 1` so an explicit 0 renders 1 instead of silently breaking admission. Addresses review feedback. Signed-off-by: Abdullah Alaqeel <abdullah.t.aqeel@gmail.com>
The openshift CRDs template opens {{- if ... -}} at line 1 but never
closes it, so helm reports "unexpected EOF" when parsing the chart.
Helm parses every template file at load time regardless of the if
condition, so this broke helm template/install/lint for all flavors,
not just openshift. Add the missing {{- end -}}, matching the
structure of kubernetes-crds.yaml.
Signed-off-by: Abdullah Alaqeel <abdullah.t.aqeel@gmail.com>
The operator Deployment hardcoded replicas: 1. Expose it through operator.replicas (default 1). Unlike the webhook, scaling up is safe here: the operator runs on knative sharedmain, which enables leader election by default, so >=2 replicas run active/standby (no duplicate writes). A config-leader-election ConfigMap is optional; knative falls back to defaults when it is absent. Signed-off-by: Abdullah Alaqeel <abdullah.t.aqeel@gmail.com>
aqeelat
changed the title
aqeelat
force-pushed
the
fix/chart-logging-replicas-extraobjects
branch
from
7ce2443 to
8265de9
Compare
tekton-robot
added
size/S
4 tasks
tekton-robot
added
the
do-not-merge/work-in-progress
aqeelat
force-pushed
the
fix/chart-logging-replicas-extraobjects
branch
from
8265de9 to
7ce2443
Compare
tekton-robot
added
size/M
aqeelat
changed the title
tekton-robot
removed
the
do-not-merge/work-in-progress
Author
|
@jkhelil sorry for the mess. The PR is ready now. |
Member
|
/approve |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jkhelil The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
Member
|
@infernus01 can you have a look |
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
3 participants
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.
Changes
Community Helm chart (
charts/tekton-operator/).Features
webhook.replicas— webhook Deployment replica count is configurable (default 1, floored at 1).operator.replicas— operator Deployment replica count is configurable (default 1; leader election is active, so >=2 runs active/standby).extraObjects— render arbitrary user-supplied manifests alongside the operator (e.g. a TektonConfig CR).Fixes
loglevel.controller/loglevel.webhookmatched nothing; replaced with the operator's real logger names (tekton-operator-lifecycle,tekton-operator-cluster-operations,<fullname>-webhook). Default level is nowinfoviaoperator.logLevel(was hardcodeddebug/development). Operator containers now setCONFIG_LOGGING_NAMEso they consume this ConfigMap.loglevel.*values are quoted to avoid YAML type coercion.{{- end -}}on an{{- if }}block causedunexpected EOFfor all flavors, since helm parses every file at load. Added the closing tag.Submitter Checklist
make test lintbefore submitting a PR(chart-only change;
make lint/make testcover Go packages and do not exercise the chart. Verified withhelm lintandhelm templateacross default, custom-logLevel, custom-operatorName, divergent release-name, replicas (operator + webhook), extraObjects, and both k8s/openshift flavors.)(No chart test framework exists in this repo. Validation is via
helm lint/helm templaterendering.)(values.yaml comments document all new/changed values, including the
autoInstallComponentswarning onextraObjectsand the leader-election note onoperator.replicas.)Release Notes