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

Fix CLI failed to set up port-forwarding when multiple controller pods exist in the same namespace #712

Merged
merged 3 commits into from Aug 2, 2018

Conversation

life1347
Copy link
Member

@life1347 life1347 commented May 30, 2018

Root cause

runPortForward only checks amount of controllers not the namespace

Solution

Use map to filter same namespace and check length of namespace list.


This change is Reviewable

nsList = append(nsList, p.Namespace)
}
}
if len(nsList) > 1 {
Copy link
Member

@soamvasani soamvasani Jun 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you have to handle the case where there are more pods but only one namespace? For some reason, just in case it happens? In that case you have to talk to any pod that's in a healthy state...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank for pointing that out! Just push a new commit to fix the problem.

@life1347 life1347 force-pushed the handle-multiple-controller-pod-in-one-ns branch 2 times, most recently from f821c33 to b163931 Compare June 12, 2018 03:43
@life1347 life1347 force-pushed the handle-multiple-controller-pod-in-one-ns branch from b163931 to c770f2f Compare June 21, 2018 10:00
@life1347 life1347 added this to the 0.10.0 milestone Jul 11, 2018
@vishal-biyani vishal-biyani self-requested a review August 1, 2018 16:14
@vishal-biyani
Copy link
Member

LGTM

@life1347 life1347 merged commit 1a1ac49 into master Aug 2, 2018
@life1347 life1347 deleted the handle-multiple-controller-pod-in-one-ns branch August 2, 2018 09:20
garyyeap pushed a commit to garyyeap/fission that referenced this pull request Sep 14, 2018
…s exist in the same namespace (fission#712)

* Fix CLI failed to set up port-forwarding when multiple controller pods exist in the same namespace
* Check pod healthy state before establishing the connection
garyyeap pushed a commit to garyyeap/fission that referenced this pull request Sep 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants