8000 Provide custom gRPC dialer to override default proxy dialer by anshulpundir · Pull Request #2419 · moby/swarmkit · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Provide custom gRPC dialer to override default proxy dialer #2419

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 24, 2017
Merged

Provide custom gRPC dialer to override default proxy dialer #2419

merged 1 commit into from
Oct 24, 2017

Conversation

anshulpundir
Copy link
Contributor
@anshulpundir anshulpundir commented Oct 24, 2017 •
edited
Loading

gRPC connects to proxy by default (c23aa65), breaking swarmkit. This change provides a basic custom dialer to override the proxy dialer for manager <-> manager and manager <-> worker connections.

Signed-off-by: Anshul Pundir anshul.pundir@docker.com

@anshulpundir
Provide custom gRPC dialer to override default proxy dialer
6493e25 
Signed-off-by: Anshul Pundir <anshul.pundir@docker.com>
@codecov
Copy link
codecov bot commented Oct 24, 2017 •
edited
Loading

Codecov Report

Merging #2419 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2419      +/-   ##
==========================================
+ Coverage    60.6%   60.61%   +0.01%     
==========================================
  Files         128      128              
  Lines       26362    26366       +4     
==========================================
+ Hits        15976    15982       +6     
+ Misses       8984     8978       -6     
- Partials     1402     1406       +4

@dave-tucker
Copy link

LGTM 👍

@dperny
Copy link
Collaborator
dperny commented Oct 24, 2017

I don't think we should merge this. I think there is a case where a user may want or need to use a proxy, and they can't because we've made it impossible.

@anshulpundir
Copy link
Contributor Author

I don't think we should merge this. I think there is a case where a user may want or need to use a proxy, and they can't because we've made it impossible.

My understanding is that this is not any worse than what it was before the gRPC change to start dialing into proxies first. So, I think it should be ok to make this change. We can easily provide the option to do proxy later. @dperny

@dperny
Copy link
Collaborator
dperny commented Oct 24, 2017

That's fine then.

@anshulpundir anshulpundir merged commit 312be59 into moby:master Oct 24, 2017
@thaJeztah thaJeztah mentioned this pull request Oct 30, 2017
Closed
@RenaudWasTaken RenaudWasTaken mentioned this pull request Oct 31, 2017
Merged
@thaJeztah thaJeztah mentioned this pull request Nov 3, 2017
Closed
marcusmartins added a commit to marcusmartins/docker that referenced this pull request Nov 3, 2017
@marcusmartins
Bump docker/swarmkit to ce5f7b8
127f043 
Upgrade swarmkit dependency.

Changes:
moby/swarmkit@ce5f7b8a (HEAD -> master, origin/master, origin/HEAD) Merge pull request moby/swarmkit#2411 from crunchywelch/2401-arm64_support
moby/swarmkit@b0856099 Merge pull request moby/swarmkit#2423 from thaJeztah/new-misty-handle
moby/swarmkit@2bd294fc Update Misty's GitHub handle
moby/swarmkit@0769c605 Comments for orphaned state/task reaper. (moby/swarmkit#2421)
moby/swarmkit@de950a7e Generic resource cli (moby/swarmkit#2347)
moby/swarmkit@312be598 Provide custom gRPC dialer to override default proxy dialer (moby/swarmkit#2419)
moby/swarmkit@4f12bf79 Merge pull request moby/swarmkit#2415 from cheyang/master
moby/swarmkit@8f9f7dc1 add pid limits
moby/swarmkit@da5ee2a6 Merge pull request moby/swarmkit#2409 from dperny/workaround-attachments
moby/swarmkit@0c7b2fc2 Delete node attachments when node is removed
moby/swarmkit@9d702763 normalize "aarch64" architectures to "arm64"

moby/swarmkit@28f91d8...ce5f7b8

Signed-off-by: Marcus Martins <marcus@docker.com>
@marcusmartins marcusmartins mentioned this pull request Nov 3, 2017
Closed
nishanttotla pushed a commit to nishanttotla/swarmkit that referenced this pull request Nov 3, 2017
@anshulpundir @nishanttotla
Provide custom gRPC dialer to override default proxy dialer (moby#2419)
8c85fdd 
Signed-off-by: Anshul Pundir <anshul.pundir@docker.com>
(cherry picked from commit 312be59)
Signed-off-by: Nishant Totla <nishanttotla@gmail.com>
@nishanttotla nishanttotla mentioned this pull request Nov 3, 2017
Closed
denverdino pushed a commit to AliyunContainerService/swarmkit that referenced this pull request Nov 5, 2017
@anshulpundir @denverdino
Provide custom gRPC dialer to override default proxy dialer (moby#2419)
62866da 
Signed-off-by: Anshul Pundir <anshul.pundir@docker.com>
thaJeztah pushed a commit to thaJeztah/swarmkit that referenced this pull request Nov 20, 2017
@anshulpundir @thaJeztah
Provide custom gRPC dialer to override default proxy dialer (moby#2419)
b7bcaee 
Signed-off-by: Anshul Pundir <anshul.pundir@docker.com>
(cherry picked from commit 312be59)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member

Looks like this PR was merged using the rebase and merge option on GitHub; trying to cherry-pick the original commit gave me a "bad object" error;

git checkout -b 17.09-backport-proxy upstream/bump_v17.09
git cherry-pick -s -S -x 6493e25b7c7df3e3f1616c34467ca0f2f450592e
fatal: bad object 6493e25b7c7df3e3f1616c34467ca0f2f450592e

Can we standardise on one approach (either "merge commit" or "rebase and merge"?

@thaJeztah thaJeztah mentioned this pull request Nov 20, 2017
Merged
@nishanttotla
Copy link
Contributor

@thaJeztah the standard is supposed to always be "merge commit". I believe there was some confusion around that leading to this, but we've uniformly communicated that "merge commits" are the standard.

@thaJeztah
Copy link
Member

@nishanttotla thanks! I think it's possible to disable the other options if you have admin access to the repository

@anshulpundir anshulpundir deleted the proxy branch November 20, 2017 19:13
@nishanttotla
Copy link
Contributor

@thaJeztah done! Now only merge commits are allowed to SwarmKit.

@thaJeztah
Copy link
Member

Thanks!

anshulpundir added a commit that referenced this pull request Nov 20, 2017
@anshulpundir
Merge pull request #2457 from thaJeztah/17.09-backport-proxy
b40ffde 
[17.09] Provide custom gRPC dialer to override default proxy dialer (#2419)
@thaJeztah thaJeztah mentioned this pull request Jan 2, 2018
Closed
@dani-docker dani-docker mentioned this pull request Jan 7, 2019
Merged
This was referenced Mar 6, 2019
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nishanttotla nishanttotla nishanttotla approved these changes

@ijc ijc ijc approved these changes

@thaJeztah thaJeztah Awaiting requested review from thaJeztah

@dperny dperny Awaiting requested review from dperny

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@anshulpundir @dave-tucker @dperny @thaJeztah @nishanttotla @ijc
0