Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CLI] Update CLI with Log command #453

Merged
merged 4 commits into from
Sep 4, 2023
Merged

[CLI] Update CLI with Log command #453

merged 4 commits into from
Sep 4, 2023

Conversation

khrm
Copy link
Contributor

@khrm khrm commented May 5, 2023

Changes

Added Log command and Get Record command.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you review them:

  • Has Docs included if any changes are user facing
  • Has Tests included if any functionality added or changed
  • Tested your changes locally (if this is a code change)
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including functionality, content, code)
  • Has a kind label. You can add a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
  • Release notes block below has been updated with any user-facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
  • Release notes contain the string "action required" if the change requires additional action from users switching to the new release

Release Notes

Add List and Get command for Logs. Also added Get Record command. 

@tekton-robot tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 5, 2023
@tekton-robot tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 5, 2023
Copy link
Contributor Author

@khrm khrm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/kind feature

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label May 8, 2023
@khrm khrm changed the title Update CLI with Log command [CLI] Update CLI with Log command May 8, 2023
Copy link
Contributor

@adambkaplan adambkaplan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed that the docs and commands appear to be out of sync.

It looks like our CI testing does not cover this submodule - should we consider moving tkn-results "in-tree", so to speak?

tools/tkn-results/cmd/records/get_record.go Outdated Show resolved Hide resolved
tools/tkn-results/cmd/logs/logs.go Outdated Show resolved Hide resolved
tools/tkn-results/docs/tkn-results_logs_get.md Outdated Show resolved Hide resolved
tools/tkn-results/docs/tkn-results_logs_list.md Outdated Show resolved Hide resolved
tools/tkn-results/docs/tkn-results_records_get.md Outdated Show resolved Hide resolved
@xinnjie
Copy link
Contributor

xinnjie commented Jul 13, 2023

Nice feature to have. We should revise code and merge it quicker. It had been one month since pr come out.

@sayan-biswas sayan-biswas mentioned this pull request Jul 26, 2023
5 tasks
@tekton-robot tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 16, 2023
@khrm khrm added this to the v0.8.0 milestone Aug 24, 2023
@tekton-robot tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 24, 2023
Copy link
Contributor Author

@khrm khrm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@enarha @avinal @sayan-biswas @adambkaplan Let's merge this also.
Without this, results isn't that much usable through cli.
It has been open for a long time.

@khrm khrm force-pushed the cli branch 5 times, most recently from 85808ff to 158c567 Compare August 30, 2023 07:03
Copy link
Member

@avinal avinal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expand this to see the test results

Tested

  • logs available in sub-commands
  • logs get and list sub-commands available in help
  • logs list working
$ ./tkn-results logs list default/results/- 
Name                                                                                            Type                                    Start                                   Update
default/results/0dfc883d-722a-4489-9ab8-3cccc74ca4f6/logs/db6a5d59-2170-3367-9eb5-83f3d264ec62  results.tekton.dev/v1alpha2.Log         2023-08-29 16:50:33 +0530 IST           2023-08-29 16:50:34 +0530 IST
default/results/fa9c1955-1ebc-4b1b-87c9-16d0bb69d5bf/logs/b7eaa4f1-8e5b-31a7-b545-706cacc9f850  results.tekton.dev/v1alpha2.Log         2023-08-29 16:54:40 +0530 IST           2023-08-29 16:54:42 +0530 IST
default/results/fa9c1955-1ebc-4b1b-87c9-16d0bb69d5bf/logs/7f277e6f-beb4-3a93-b34e-314ad29f97fb  results.tekton.dev/v1alpha2.Log         2023-08-29 16:54:42 +0530 IST           2023-08-29 16:54:42 +0530 IST
default/results/1af526f7-6b44-4df9-9222-62cce2afa7cc/logs/2795be56-0b7f-309d-91a0-7f16dae1746f  results.tekton.dev/v1alpha2.Log         2023-08-29 16:54:35 +0530 IST           2023-08-29 16:54:36 +0530 IST
default/results/4000d220-5274-46a0-b0cb-d628c8e41262/logs/1ff9ee1e-3368-3e2d-a22c-ddf1875cd8a3  results.tekton.dev/v1alpha2.Log         2023-08-29 17:01:05 +0530 IST           2023-08-29 17:01:06 +0530 IST
default/results/fc0796d1-78c9-4098-9459-99013dc06854/logs/88c5f96a-74c3-338f-b182-ae8573ee193b  results.tekton.dev/v1alpha2.Log         2023-08-29 17:01:08 +0530 IST           2023-08-29 17:01:17 +0530 IST
default/results/fc0796d1-78c9-4098-9459-99013dc06854/logs/ae938c49-9e00-3dd3-8562-1344bea25d41  results.tekton.dev/v1alpha2.Log         2023-08-29 17:01:09 +0530 IST           2023-08-29 17:01:09 +0530 IST
default/results/4313afd1-bf60-4df3-83d4-bd2c0c1649c6/logs/980bd0a7-2d70-3d19-bcf2-e61d49ae6637  results.tekton.dev/v1alpha2.Log         2023-08-29 17:03:49 +0530 IST           2023-08-29 17:03:51 +0530 IST
default/results/bc54da9a-9951-4afa-a5ed-a7e51bb6f958/logs/68ae2e1b-dcd9-33fc-96bb-47db7bbd312e  results.tekton.dev/v1alpha2.Log         2023-08-29 17:03:54 +0530 IST           2023-08-29 17:04:02 +0530 IST
default/results/bc54da9a-9951-4afa-a5ed-a7e51bb6f958/logs/a1db7bda-178e-318f-b084-838019671b2f  results.tekton.dev/v1alpha2.Log         2023-08-29 17:03:55 +0530 IST           2023-08-29 17:03:55 +0530 IST
default/results/93046b50-ff51-45bc-bb4c-de21c33f8b0f/logs/0820c9eb-a58d-3747-9709-87b113adfc5e  results.tekton.dev/v1alpha2.Log         2023-08-29 17:07:57 +0530 IST           2023-08-29 17:08:10 +0530 IST
default/results/93046b50-ff51-45bc-bb4c-de21c33f8b0f/logs/76d0c470-b3ff-3804-8f7d-b5276757eaf1  results.tekton.dev/v1alpha2.Log         2023-08-29 17:07:55 +0530 IST           2023-08-29 17:08:09 +0530 IST
  • filtering logs works
$ ./tkn-results logs list default/results/- --limit 5 --output json --filter="data.spec.resource.kind=='PipelineRun'"
{
  "records":  [
    {
      "name":  "default/results/fa9c1955-1ebc-4b1b-87c9-16d0bb69d5bf/logs/7f277e6f-beb4-3a93-b34e-314ad29f97fb",
      "id":  "e38f8413-798e-41f7-a4b9-8ace9aedcbf5",
      "uid":  "e38f8413-798e-41f7-a4b9-8ace9aedcbf5",
      "data":  {
        "type":  "results.tekton.dev/v1alpha2.Log",
        "value":  "eyJraW5kIjogIkxvZyIsICJzcGVjIjogeyJ0eXBlIjogIkZpbGUiLCAicmVzb3VyY2UiOiB7InVpZCI6ICJmYTljMTk1NS0xZWJjLTRiMWItODdjOS0xNmQwYmI2OWQ1YmYiLCAia2luZCI6ICJQaXBlbGluZVJ1biIsICJuYW1lIjogInJhbmRvbS1sb2dzIiwgIm5hbWVzcGFjZSI6ICJkZWZhdWx0In19LCAic3RhdHVzIjogeyJzaXplIjogMH0sICJtZXRhZGF0YSI6IHsidWlkIjogIjdmMjc3ZTZmLWJlYjQtM2E5My1iMzRlLTMxNGFkMjlmOTdmYiIsICJuYW1lIjogInJhbmRvbS1sb2dzLWxvZyIsICJuYW1lc3BhY2UiOiAiZGVmYXVsdCIsICJjcmVhdGlvblRpbWVzdGFtcCI6IG51bGx9LCAiYXBpVmVyc2lvbiI6ICJyZXN1bHRzLnRla3Rvbi5kZXYvdjFhbHBoYTIifQ=="
      },
      "etag":  "e38f8413-798e-41f7-a4b9-8ace9aedcbf5-1693308282980985027",
      "createdTime":  "2023-08-29T11:24:42.980985Z",
      "createTime":  "2023-08-29T11:24:42.980985Z",
      "updatedTime":  "2023-08-29T11:24:42.980985Z",
      "updateTime":  "2023-08-29T11:24:42.980985Z"
    }
  ]
}
$ ./tkn-results logs list default/results/- --filter="data.spec.resource.kind=='TaskRun'"          
Name                                                                                            Type                                    Start                                   Update
default/results/0dfc883d-722a-4489-9ab8-3cccc74ca4f6/logs/db6a5d59-2170-3367-9eb5-83f3d264ec62  results.tekton.dev/v1alpha2.Log         2023-08-29 16:50:33 +0530 IST           2023-08-29 16:50:34 +0530 IST
default/results/fa9c1955-1ebc-4b1b-87c9-16d0bb69d5bf/logs/b7eaa4f1-8e5b-31a7-b545-706cacc9f850  results.tekton.dev/v1alpha2.Log         2023-08-29 16:54:40 +0530 IST           2023-08-29 16:54:42 +0530 IST
default/results/1af526f7-6b44-4df9-9222-62cce2afa7cc/logs/2795be56-0b7f-309d-91a0-7f16dae1746f  results.tekton.dev/v1alpha2.Log         2023-08-29 16:54:35 +0530 IST           2023-08-29 16:54:36 +0530 IST
default/results/4000d220-5274-46a0-b0cb-d628c8e41262/logs/1ff9ee1e-3368-3e2d-a22c-ddf1875cd8a3  results.tekton.dev/v1alpha2.Log         2023-08-29 17:01:05 +0530 IST           2023-08-29 17:01:06 +0530 IST
default/results/fc0796d1-78c9-4098-9459-99013dc06854/logs/88c5f96a-74c3-338f-b182-ae8573ee193b  results.tekton.dev/v1alpha2.Log         2023-08-29 17:01:08 +0530 IST           2023-08-29 17:01:17 +0530 IST
default/results/4313afd1-bf60-4df3-83d4-bd2c0c1649c6/logs/980bd0a7-2d70-3d19-bcf2-e61d49ae6637  results.tekton.dev/v1alpha2.Log         2023-08-29 17:03:49 +0530 IST           2023-08-29 17:03:51 +0530 IST
default/results/bc54da9a-9951-4afa-a5ed-a7e51bb6f958/logs/68ae2e1b-dcd9-33fc-96bb-47db7bbd312e  results.tekton.dev/v1alpha2.Log         2023-08-29 17:03:54 +0530 IST           2023-08-29 17:04:02 +0530 IST
default/results/93046b50-ff51-45bc-bb4c-de21c33f8b0f/logs/76d0c470-b3ff-3804-8f7d-b5276757eaf1  results.tekton.dev/v1alpha2.Log         2023-08-29 17:07:55 +0530 IST           2023-08-29 17:08:09 +0530 IST

  • Get logs works
$ ./tkn-results logs get default/results/4000d220-5274-46a0-b0cb-d628c8e41262/logs/1ff9ee1e-3368-3e2d-a22c-ddf1875cd8a3
{
  "name":  "default/results/4000d220-5274-46a0-b0cb-d628c8e41262/logs/1ff9ee1e-3368-3e2d-a22c-ddf1875cd8a3",
  "data":  "W3ByZXBhcmVdIDIwMjMvMDgvMjkgMTE6MzA6NTcgRW50cnlwb2ludCBpbml0aWFsaXphdGlvbgoKW2hlbGxvXSBoZWxsbwoKJSFzKDxuaWw+KQo="
}
  • Getting logs larger than LOGS_BUFFER_SIZE fails to display all the chunks, also the format of the response is not user-friendly
  • Records get sub-command is available
  • Records get command works
$ ./tkn-results records get default/results/60ddbc6d-b1f6-401d-8016-b101c15d1b27/records/7a6193bf-54e3-4337-a3ea-7d79583ebdb8
{
  "name":  "default/results/60ddbc6d-b1f6-401d-8016-b101c15d1b27/records/7a6193bf-54e3-4337-a3ea-7d79583ebdb8",
  "id":  "f5e7954f-c1f6-46fb-b491-ce9e7a132f82",
  "uid":  "f5e7954f-c1f6-46fb-b491-ce9e7a132f82",
  "data":  {
    "type":  "tekton.dev/v1beta1.TaskRun",
    "value":  "data-ommited=="
  },
  "etag":  "f5e7954f-c1f6-46fb-b491-ce9e7a132f82-1693311373223986106",
  "createdTime":  "2023-08-29T11:16:12.430007Z",
  "createTime":  "2023-08-29T11:16:12.430007Z",
  "updatedTime":  "2023-08-29T12:16:13.223986Z",
  "updateTime":  "2023-08-29T12:16:13.223986Z"
}

The functionality works as expected but the format of getting Logs and Record is not user friendly. The reason anyone wants to use a CLI is for quick access and human readability. Our CLI dispalyed the desired data in encoded form. For example tkn CLI displayed most responses in human readable form with proper decoration and there are options for YAML/JSON response. We should follow the same suite. This can be followup PR/feature. Will open an issue.

For this PR the GET log doesn't displays all the chunks, that needs to be fixed.

tools/tkn-results/cmd/logs/get_log.go Show resolved Hide resolved
@avinal
Copy link
Member

avinal commented Aug 30, 2023

Opened #587 for improving UX

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 30, 2023
@khrm khrm force-pushed the cli branch 2 times, most recently from 0f86ec4 to f60f74a Compare August 31, 2023 13:22
@khrm khrm force-pushed the cli branch 2 times, most recently from f81fd80 to f6231e8 Compare September 4, 2023 07:34
Copy link
Member

@avinal avinal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, works as expected
/approve
/lgtm

@tekton-robot
Copy link

@avinal: changing LGTM is restricted to collaborators

In response to this:

Tested, works as expected
/approve
/lgtm

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.

@tekton-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: avinal, vdemeester

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

The pull request process is described 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

@enarha
Copy link
Contributor

enarha commented Sep 4, 2023

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 4, 2023
@tekton-robot tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 4, 2023
@enarha
Copy link
Contributor

enarha commented Sep 4, 2023

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 4, 2023
@khrm khrm dismissed adambkaplan’s stale review September 4, 2023 08:30

Addressed the review comments.

@tekton-robot tekton-robot merged commit 6dd5018 into tektoncd:main Sep 4, 2023
@khrm khrm deleted the cli branch September 4, 2023 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants