port-forward should be able to select ports by service name
#5009
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5009 +/- ##
==========================================
+ Coverage 72.17% 72.24% +0.06%
==========================================
Files 365 366 +1
Lines 12807 12885 +78
==========================================
+ Hits 9244 9309 +65
- Misses 2878 2885 +7
- Partials 685 691 +6
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This change looks good to me except for the change in the skaffold.proto.
we have existing clients who use this event and changing int to StringOrInt may be break integration.
I will follow up with manually testing the new skaffold event on the CC IDEs side.
Meanwhile, can you instead use a new field and deprecate the existing field.
proto/skaffold.proto
Outdated
| @@ -190,7 +190,7 @@ message ResourceStatusCheckEvent { | |||
| // PortEvent Event describes each port forwarding event. | |||
| message PortEvent { | |||
| int32 localPort = 1; // local port for forwarded resource | |||
| int32 remotePort = 2; // remote port is the resource port that will be forwarded. | |||
| IntOrString remotePort = 2; // remote port is the resource port that will be forwarded. | |||
There was a problem hiding this comment.
unfortunately, this will be a backward incompatible change. Can we instead add a another field and mark this as deprecated.
pkg/skaffold/event/event.go
Outdated
| handler.handle(&proto.Event{ | ||
| EventType: &proto.Event_PortEvent{ | ||
| PortEvent: &proto.PortEvent{ | ||
| RemotePort: &proto.IntOrString{ |
There was a problem hiding this comment.
Can we keep remote port as is if its of type int and use new field to populate the intOrString port value.
|
@anshlykov Can you please rebase ? Would love to get this in before next release since we already merged the PR to upgrade skaffold config. |
|
I have rebased and added changes to the proto schema. |
Fixes: #4464
Merge after: #5010
Description
This PR introduced the new type
IntOrStringand changed the type ofPortForwardResource.PorttoIntOrString. This change allows a named port or an int port to be used in thekubectl port-forward ...command.Also, there are some breaking changes in the proto schema and changes in logging. I think this can be simply resolved but I have not done it yet because I want to validate my approach first (or maybe it is already OK)