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

port-forwarding in skaffold dev is basically broken #1594

Closed
dgageot opened this issue Feb 4, 2019 · 12 comments · Fixed by #1616
Closed

port-forwarding in skaffold dev is basically broken #1594

dgageot opened this issue Feb 4, 2019 · 12 comments · Fixed by #1616
Assignees
Labels
area/portforward kind/bug Something isn't working priority/p0 Highest priority. We are actively looking at delivering it.

Comments

@dgageot
Copy link
Contributor

dgageot commented Feb 4, 2019

Now that we track for port collision, skaffold dev will use a new port each time a pod is redeployed.

For example:

  • The first time I deploy a pod that's on port 8080, it will be exposed on local port 8080
  • When I make a file change, it's redeployed and exposed on port 4503
  • Then 4504...
@dgageot dgageot added kind/bug Something isn't working priority/p0 Highest priority. We are actively looking at delivering it. area/portforward labels Feb 4, 2019
@cmoad
Copy link

cmoad commented Feb 4, 2019

Also experiencing this after trying 0.22.0. Old behavior would maintain the exposed service port. Now that only happens on the first deploy and all subsequent deploys change to a different port as described above.

@priyawadhwa
Copy link
Contributor

This issue should have been fixed by #1616 -- we'd recommend using skaffold built from master, or using v0.21.0 until v0.23.0 comes out next week with the fix!

@lsowen
Copy link

lsowen commented Mar 23, 2019

This behaviour seems to be happening again in v0.25.0.

@botwhytho
Copy link

I can confirm this behavior is still around in v0.26.0. I reverted to v.0.23.0 and the problem is indeed fixed on that version. Adding some tests around this would be great so this regression doesn't occur again.

Port forwarding during skaffold dev is a great value add for developers and can't really get more adoption from developer teams if this feature is broken. Too bad I'm having to stay on an older version for this to work at the moment.

@arshbot
Copy link

arshbot commented Apr 17, 2019

@priyawadhwa any idea on when a fix could make it's way into master? Just tested v0.27.0 and it still has this issue

@priyawadhwa
Copy link
Contributor

Hey @arshbot -- I suspect this is happening because "pod name" was re-added to the port-forwarding key in #1780, so every time a pod is recreated it's assigned to a new port.

I'm not sure if removing it will work because I don't want to re-introduce the race condition that PR was meant to fix. I'll go ahead and reopen this issue, our team is going to look at fixing port-forwarding soon & we will definitely take a look at this.

cc @nkubala just wanted to confirm, pod name is necessary in the key, right?

@priyawadhwa priyawadhwa reopened this Apr 17, 2019
@nkubala
Copy link
Contributor

nkubala commented Apr 18, 2019

@priyawadhwa pod name is not necessary, I should have thought about it a bit more. this will be addressed with some of the upcoming port forwarding improvements.

@avloss
Copy link

avloss commented Apr 29, 2019

Thank you for taking note of this issue - I just want to make sure you understand the magnitude of this error for independent developers. This issue makes local development of Microservices with tools like Postman practically unfeasible - I've reverted to 0.23.0 for local development. While much praised "Cloud Code" needs a more recent version. So currently I have TWO versions of skaffold locally.

@tejal29
Copy link
Contributor

tejal29 commented Apr 30, 2019

Related to #1815

priyawadhwa pushed a commit to priyawadhwa/skaffold-1 that referenced this issue May 1, 2019
This way, when pods are regenerated, they will be mapped to the same
port.

This should fix some of the issues users have been facing in GoogleContainerTools#1815 and GoogleContainerTools#1594.
@priyawadhwa
Copy link
Contributor

I just removed the podname from the key in #2047 -- could someone please confirm if this fixes their issue? You can find instructions for installing the latest version of skaffold at HEAD here under Latest bleeding edge binary

@arshbot
Copy link

arshbot commented May 2, 2019

I can confirm that at first glance Latest bleeding edge binary works great on my machine. I'll continue using it and post any issues here.

If I could make a suggestion, do you think it'd be possible to add in some tests for this feature ( and hopefully others like this ) to prevent regression in the future? Tests are really useful in detecting subtle behavior changes that occur outside the scope of a newly implemented feature.

priyawadhwa pushed a commit to priyawadhwa/skaffold-1 that referenced this issue May 2, 2019
This should help make sure we don't add podname to the key again. Ref GoogleContainerTools#1594.
@priyawadhwa
Copy link
Contributor

Cool, thanks @arshbot. Added a unit test in #2059.

I'll go ahead and close this issue then, and if anyone sees more errors please comment here and I'll open it up again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/portforward kind/bug Something isn't working priority/p0 Highest priority. We are actively looking at delivering it.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants