fix(tektonresult): default watcher logs_api from spec - #3884
Open
pujitha24 wants to merge 1 commit into
Open
Conversation
tekton-robot
added
the
release-note
tekton-robot
added
the
size/L
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3884 +/- ##
=======================================
Coverage 26.33% 26.34%
=======================================
Files 465 465
Lines 24951 24953 +2
=======================================
+ Hits 6572 6574 +2
Misses 17661 17661
Partials 718 718
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
/retest |
2 similar comments
Contributor
Author
|
/retest |
Member
|
/retest |
Member
|
/lgtm |
Member
tekton-robot
added
lgtm
Contributor
|
New changes are detected. LGTM label has been removed. |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Motivation: setting spec.logs_api: true on a TektonResult (or via
TektonConfig) enables the LOGS_API env var on the tekton-results-api
deployment, but the tekton-results-watcher deployment has its own,
separate -logs_api command-line flag that defaults to false. That
flag is what actually makes the Watcher forward TaskRun/PipelineRun
logs to the API server; nothing defaulted it from the top-level
logs_api setting, so users following the documented example (setting
only the top-level logs_api) got an API server with logging enabled
but a Watcher that never sends logs, and logs were never stored.
Approach: default Result.Watcher.LogsAPI from the top-level
ResultsAPIProperties.LogsAPI in Result.setDefaults() whenever the
user has not explicitly set spec.watcher.logs_api, mirroring the
adjacent RouteEnabled/RouteTLSTermination defaulting already in that
function. An explicit spec.watcher.logs_api value is never
overridden.
This defaulting runs wherever Result.setDefaults() runs today, i.e.
when a TektonResult is created/updated via TektonConfig (the
documented install path). TektonResult.SetDefaults does not call
Result.setDefaults() at all, so a CR applied directly as a
standalone `kind: TektonResult` (bypassing TektonConfig) does not
get this or the pre-existing RouteEnabled default either; that gap
predates this change and is not addressed here.
Validation: added TestResult_SetDefaultsWatcherLogsAPI, a table test
covering propagate-true, propagate-false, no-override-when-explicit,
and no-op-when-top-level-unset. Confirmed by temporarily reverting
the fix that the two propagation subtests fail without it and pass
with it:
go test ./pkg/apis/operator/v1alpha1/... \
-run TestResult_SetDefaultsWatcherLogsAPI -v
Also ran, all passing:
go build ./...
go test ./pkg/apis/operator/v1alpha1/... \
./pkg/reconciler/kubernetes/tektonresult/... \
./pkg/reconciler/kubernetes/tektonconfig/...
gofmt -l on both changed files (clean)
golangci-lint could not be run in this sandbox (its download step
fails a checksum check with no network access); go vet ./pkg/apis/
operator/v1alpha1/... was run instead and reported nothing.
Report: tektoncd#2600
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
pujitha24
force-pushed
the
auto/issue-2600
branch
from
August 14, 2026 08:01
29dd4dd to
d7f756f
Compare
Contributor
Author
|
Tide was flagging this as unmergeable because the branch had a merge commit from syncing upstream/main, which a clean rebase can't replay past a later commit ( |
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
Fixes a bug where setting
spec.logs_api: trueon aTektonResult(or viaTektonConfig) enabled theLOGS_APIenv var on thetekton-results-apideployment, but did not propagate to thetekton-results-watcherdeployment's separate-logs_apiflag (which defaults tofalse). As a result, users following the documented example of only setting the top-levellogs_apiended up with an API server that had logging enabled but a Watcher that never forwarded logs, so logs were never stored.Result.setDefaults()now defaultsResult.Watcher.LogsAPIfrom the top-levelResultsAPIProperties.LogsAPIwhenever the user has not explicitly setspec.watcher.logs_api, mirroring the existingRouteEnabled/RouteTLSTerminationdefaulting in the same function. An explicitspec.watcher.logs_apivalue is never overridden.Note: this defaulting runs wherever
Result.setDefaults()runs today (i.e. when aTektonResultis created/updated viaTektonConfig, the documented install path).TektonResult.SetDefaultsdoes not callResult.setDefaults(), so a CR applied directly as a standalonekind: TektonResultdoes not get this (or the pre-existingRouteEnabled) default — that gap predates this change and is not addressed here.Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
make test lintbefore submitting a PRSee the contribution guide for more details.
Release Notes
AI assistance: this change was drafted with Claude Code.
Fixes #2600