fix(e2e): add SinceSeconds support to pod log helpers#2515
fix(e2e): add SinceSeconds support to pod log helpers#2515chmouel merged 1 commit intotektoncd:mainfrom
Conversation
🔍 PR Lint Feedback
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant improvement to log retrieval mechanisms within the testing framework by adding support for the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for SinceSeconds in pod log helper functions to address issues with log rotation in E2E tests. The changes are well-implemented, adding a sinceSeconds parameter to relevant functions and updating call sites. The core logic correctly constructs PodLogOptions based on the new parameter. The one test that was failing due to log rotation is updated to use sinceSeconds, while other tests are updated to pass nil, preserving their existing behavior. The changes are clean and effectively solve the described problem.
|
/retest |
theakshaypant
left a comment
theakshaypant
left a comment
There was a problem hiding this comment.
Overall like the idea. Got a couple comments:
- Nit: Recommend changing the commit to
test(e2e): add SinceSeconds support to pod log helpers] - I see
sinceSecondsbeing set to nil for all tests, what is the plan to actually use it?
at the moment sinceSeconds is used only for |
I would rather prefer |
Fairs! That's why had added it as a nit, I personally prefer, all test related changes as |
The GetPodLog, GetControllerLogByName, RegexpMatchingInControllerLog, RegexpMatchingInPodLog, and GoldenPodLog functions now accept an optional sinceSeconds parameter, which maps to the Kubernetes PodLogOptions.SinceSeconds field. When kubelet rotates CRI container logs (e.g. containerd splitting 0.log into 0.log.<timestamp> and a new 0.log), the TailLines option may only read from the current log file, causing regexp assertions to miss matching lines that landed in the rotated segment. This was observed in TestGithubGHEPullRequestGitCloneTask where the "fetched git-clone task" message was written at 06:30:09, CRI rotation occurred at 06:30:13, and the test checked at 06:30:21 finding only 2 lines in the new 0.log. SinceSeconds uses timestamps rather than byte offsets, so it reads across rotated CRI log files reliably. Callers that do not need time-based filtering pass nil to preserve existing behaviour. Signed-off-by: Zaki Shaikh <zashaikh@redhat.com> Assisted-by: Claude Opus 4.6 (via Claude Code)
zakisk
force-pushed
the
fix-e2e-logs-absence
branch
from
04bb80b to
4fec6d0
Compare
everyone has his/her own interpretation 😄 |
3 participants
The GetPodLog, GetControllerLogByName, RegexpMatchingInControllerLog, RegexpMatchingInPodLog, and GoldenPodLog functions now accept an optional sinceSeconds parameter, which maps to the Kubernetes PodLogOptions.SinceSeconds field.
When kubelet rotates CRI container logs (e.g. containerd splitting 0.log into 0.log. and a new 0.log), the TailLines option may only read from the current log file, causing regexp assertions to miss matching lines that landed in the rotated segment. This was observed in TestGithubGHEPullRequestGitCloneTask where the "fetched git-clone task" message was written at 06:30:09, CRI rotation occurred at 06:30:13, and the test checked at 06:30:21 finding only 2 lines in the new 0.log.
SinceSeconds uses timestamps rather than byte offsets, so it reads across rotated CRI log files reliably. Callers that do not need time-based filtering pass nil to preserve existing behaviour.
Assisted-by: Claude Opus 4.6 (via Claude Code)
📝 Description of the Change
👨🏻 Linked Jira
🔗 Linked GitHub Issue
Fixes #
🚀 Type of Change
fix:)feat:)feat!:,fix!:)docs:)chore:)refactor:)enhance:)deps:)🧪 Testing Strategy
🤖 AI Assistance
If you have used AI assistance, please provide the following details:
Which LLM was used?
Extent of AI Assistance:
Important
If the majority of the code in this PR was generated by an AI, please add a
Co-authored-bytrailer to your commit message.For example:
Co-authored-by: Gemini gemini@google.com
Co-authored-by: ChatGPT noreply@chatgpt.com
Co-authored-by: Claude noreply@anthropic.com
Co-authored-by: Cursor noreply@cursor.com
Co-authored-by: Copilot Copilot@users.noreply.github.com
**💡You can use the script
./hack/add-llm-coauthor.shto automatically addthese co-author trailers to your commits.
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.