cli icon indicating copy to clipboard operation
cli copied to clipboard

PodLogsFollower

Open otaviof opened this issue 3 years ago • 5 comments

Changes

  • Refactoring the PodFollower into PodLogsFollower, a component focused on the pod lifecycle, capable of both streaming live logs and read logs from an existing pod
  • shp buildrun logs waits a few seconds for the pod, instead of error out immediately
  • Refactoring BuildRun inspections into a single component
  • Introducing FakeParams to simplify unit-testing
  • Refactoring Params to avoid cyclic-dependencies
  • Minor documentation updates

Fixes #103

Submitter Checklist

  • [x] Includes tests if functionality changed/was added
  • [x] Includes docs if changes are user-facing
  • [x] Set a kind label on this PR
  • [x] Release notes block has been filled in, or marked NONE

Release Notes

Able to both stream and read all logs from a existing BuildRun, therefore the --follow/-F flag has been removed from "shp buildrun logs"

otaviof avatar Mar 25 '22 12:03 otaviof

/assign @gabemontero @HeavyWombat

otaviof avatar Mar 28 '22 12:03 otaviof

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: To complete the pull request process, please ask for approval from gabemontero after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci[bot] avatar Mar 31 '22 06:03 openshift-ci[bot]

Before I go down that path, a couple of high level questions that might avoid more churn on my part:

1. am I correct in thinking that `-F` was removed because you simply "follow by default" and the logic figures out if the pod is finished before the watch apparatus is fully set up.  If so, highlighting that in the release note and PR description would be very helpful.

I need to go back on this @gabemontero. Testing the behavior on main branch, it shows a different outcome during on using the --follow flag.

I traced back the GetPodLogs() usage, and consolidated it the new PodLogsFollower, with the same behavior we see today on main branch.

Please consider the latest changes.

otaviof avatar Apr 01 '22 12:04 otaviof

/hold

Making sure this does not go into v0.10 release.

SaschaSchwarze0 avatar Apr 04 '22 13:04 SaschaSchwarze0

@otaviof: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci[bot] avatar Apr 09 '22 22:04 openshift-ci[bot]