feat(cli): tkn-result would port-forward to service/tekton-results-api-service automatically

[xinnjie]

Changes

If Results API server address not specified, tkn-result would port-forward to service/tekton-results-api-service automatically. It helps a lot when service/tekton-results-api-service is not exported.

Test case:

• Results API server address specified, no functionality is changed.
  • If address is invalid, cli will timeout.
  • If address is valid, cli will query successfully.
• Results API server address not specified, cli queries Results API service successfully without Kubectl port-forward manually.

  /kind feature

  Submitter Checklist

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

• [x] Has Docs included if any changes are user facing
• [ ] Has Tests included if any functionality added or changed
• [x] Tested your changes locally (if this is a code change)
• [x] Follows the commit message standard
• [x] Meets the Tekton contributor standards (including functionality, content, code)
• [x] 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
• [x] 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

[CLI]No need to port-forward `service/tekton-results-api-service` manually. If Results API server address not specified and not disable explicitly, tkn-result will port-forward to service/tekton-results-api-service automatically.

[tekton-robot]

Hi @xinnjie. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

[xinnjie]

/assign @khrm

[enarha]

/ok-to-test

[xinnjie]

/assign @sayan-biswas @enarha

[xinnjie]

We can probably add this. There are a lot of CLI prs remaining. We need to merge these.

Could you give the pr a lgtm if it is ok to add the feature? Thanks a lot. @khrm

[sayan-biswas]

@xinnjie I'll check this out today.

[sayan-biswas]

We probably need this behind a flag not automatically port-forward. @sayan-biswas @avinal What do you think?

Agreed!!

Although there's a pre-condition to this automatic behaviour (server address not configured), but still I feel a flag would provide more explicit control to the user.

[avinal]

We probably need this behind a flag not automatically port-forward. @sayan-biswas @avinal What do you think?

I have similar point as Sayan, but I am more in favor of automatic since a condition is already defined. That said, a flag would be useful from a user's control perspective.

@xinnjie The changes look good to me, but I am unable to test due to a problem with my setup. I will put my test results if I fix my setup.

[xinnjie]

Sorry for late response.

@avinal @sayan-biswas @khrm how about adding a flag -portforward to enable port-forwarding while address is empty, and default values is true. That said, when address is empty, port-forwarding is the default behavior, users can choose to disable it explicitly.

I prefer port-forward automatically (when address is empty), because there is a use case that tekton-results-api-service is not exported outside the cluster, and people who do not export the tekton-results-api-service need auto port-forwarding more while people export the tekton-results-api-service will set address by default.

[xinnjie]

portforward flag added.

Test case:

• portforward not set in config file and command line, addr not set. Port forward run as expected.

• When portforward: false is added to config file.

  > ./tkn-results  list -                   
  Dial: context deadline exceeded
  Error: context deadline exceeded
  Usage:
  tkn-results list [flags] <parent>
  ...

• When portforward not set in config file and --portforward=false set in command line:

  
  > ./tkn-results --portforward=false  list -                   
  Dial: context deadline exceeded
  Error: context deadline exceeded
  Usage:
  tkn-results list [flags] <parent>
  
  <parent>: Parent name to query. This is typically corresponds to a namespace, but may vary depending on the API Server. "-" may be used to query all parents.

[xinnjie]

Please any one review this PR. It has been 2 weeks now.

[avinal]

cc @enarha @sayan-biswas as discussed in the WG call, I have tested it, please put a final review, it is ready to be merged.

[enarha]

@xinnjie Apologies for the delay on this one. Can you please rebase and fix the conflicts (I want to avoid doing that manually) and I'll make sure we address the PR in the next couple of days. Thank you again.

[xinnjie]

Rebased.

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: avinal, sayan-biswas

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: - ~~[OWNERS](https://github.com/tektoncd/results/blob/main/OWNERS)~~ [sayan-biswas] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

[enarha]

/lgtm