triggers icon indicating copy to clipboard operation
triggers copied to clipboard

fix: Issue in eventlisteners e2e when kubernetes host has a path

Open danielfbm opened this issue 9 months ago • 18 comments

Changes

fix: Issue in eventlisteners e2e when kubernetes host has a path

Previously having a prefix into the server url would generate an error during testing due to url parsing of host with embeded path:

parse "https://k8s.example.com%2Fkubernetes%2Fpath/api/v1/namespaces/arrakis-t7ss8/pods/el-my-eventlistener-848756bd88-sg8fv/portforward": invalid URL escape "%2F" Previously having a prefix into the server url would generate an error during testing due to url parsing of host with embeded path

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • [ ] Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
  • [ ] Has Tests included if any functionality added or changed
  • [ ] Follows the commit message standard
  • [ ] Meets the Tekton contributor standards (including functionality, content, code)
  • [x] Has a kind label. You can add one by adding 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). See some examples of good release notes.
  • [ ] Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

danielfbm avatar Feb 07 '25 07:02 danielfbm

Hi @danielfbm. 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. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

tekton-robot avatar Feb 07 '25 07:02 tekton-robot

@dibyom @iancoffey hi~ could you guys help me reviewing this PR? thank you 🫡

danielfbm avatar Feb 10 '25 02:02 danielfbm

hi, anyone else can take a look? @savitaashture @khrm thanks 🙇

danielfbm avatar Feb 19 '25 00:02 danielfbm

/kind bug

danielfbm avatar Mar 01 '25 07:03 danielfbm

@khrm @iancoffey is there anything else I can help with to move this forward? thanks

danielfbm avatar Mar 06 '25 11:03 danielfbm

/retest

danielfbm avatar Mar 15 '25 13:03 danielfbm

@khrm @iancoffey hi~ sorry for being pushy, but can we move forward? is there anything else to be done? Please let me know if there is any. Thanks

danielfbm avatar Mar 24 '25 02:03 danielfbm

lgtm @khrm agree?

iancoffey avatar Mar 27 '25 16:03 iancoffey

Agreed. Sorry for the delay.

/lgtm

@savitaashture

khrm avatar Mar 27 '25 20:03 khrm

@danielfbm can you please rebase the PR so that we can merge this thank you

savitaashture avatar May 02 '25 10:05 savitaashture

@danielfbm can you please rebase the PR so that we can merge this thank you

@savitaashture I did rebase but the tests are failing now. I will retest locally and will do any changes, if necessary. Thanks

danielfbm avatar May 03 '25 16:05 danielfbm

@danielfbm can you please rebase the PR so that we can merge this thank you

@savitaashture I did rebase but the tests are failing now. I will retest locally and will do any changes, if necessary. Thanks

Sure

savitaashture avatar May 05 '25 12:05 savitaashture

@savitaashture I did the necessary changes and the tests are passing now. Could you take a look? Thanks

danielfbm avatar May 06 '25 08:05 danielfbm

@savitaashture sorry for being pushy, but could you please take a look? thank you 😺

danielfbm avatar May 10 '25 06:05 danielfbm

@savitaashture

@danielfbm can you please rebase the PR so that we can merge this thank you

@savitaashture I did rebase but the tests are failing now. I will retest locally and will do any changes, if necessary. Thanks

Sure

Can you take a look again? thanks

danielfbm avatar May 16 '25 06:05 danielfbm

@savitaashture @khrm hi guys, pinging again 😄

danielfbm avatar Jun 10 '25 10:06 danielfbm

Sorry. I miss this. I will merge this when tests succeed.

khrm avatar Jul 01 '25 08:07 khrm

/lgtm

khrm avatar Jul 01 '25 09:07 khrm

/approve

khrm avatar Jul 01 '25 09:07 khrm

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: khrm

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

tekton-robot avatar Jul 01 '25 09:07 tekton-robot