headlamp icon indicating copy to clipboard operation
headlamp copied to clipboard

Allow Port forwarding from the headlamp UI

Open ashu8912 opened this issue 2 years ago • 2 comments

[Title: describe the change in one sentence]

In the container info section of pod details we now have a button to allow port forwarding to the target port this button is situated right beside the port option Screenshot 2022-08-08 at 8 48 15 PM

Testing done

Screenshot 2022-08-08 at 8 48 07 PM

ashu8912 avatar Aug 08 '22 15:08 ashu8912

  • [x] Allow to port forward to a pod from desktop app UI

ashu8912 avatar Aug 08 '22 15:08 ashu8912

@ashu8912 , this is indeed a good start. I think there's still some things to consider which hopefully I covered in my review. Any questions, let me know.

joaquimrocha avatar Aug 08 '22 17:08 joaquimrocha

@joaquimrocha can you mention the steps you followed to run the port forward??

ashu8912 avatar Sep 23 '22 08:09 ashu8912

(FTR, responded today in a call)

joaquimrocha avatar Sep 26 '22 15:09 joaquimrocha

Screenshot 2022-11-14 at 11 01 23 PM Screenshot 2022-11-14 at 11 01 15 PM

ashu8912 avatar Nov 14 '22 17:11 ashu8912

Screenshot 2022-11-16 at 10 57 00 PM Screenshot 2022-11-16 at 10 57 11 PM

ashu8912 avatar Nov 16 '22 17:11 ashu8912

@ashu8912 , I tried the PR and it works fine for the Pod's port forwarding. The Service forwarding was still not working correctly when I tried that named port (non-numeric).

In order to make progress and be more efficient about the feature, I am proposing we drop the Service's port forwarding for now.

I also added a fix up commit that I'd like you to check. This makes the style closer to the latest mockups.

joaquimrocha avatar Nov 18 '22 16:11 joaquimrocha

This is reviewed and tested. We should merge after merging its base PR (#843 ).

joaquimrocha avatar Nov 23 '22 21:11 joaquimrocha

This was deleted automatically after I merged + deleted the base branch, but it should have been automatically rebased on main... Maybe this is a bug on Github. Reopening so we merge this.

joaquimrocha avatar Nov 24 '22 13:11 joaquimrocha