change gossip dns conn limit by ENV #5077

yancl · 2018-04-28T16:01:02Z

so that we can run more nodes than 64 :)

yancl · 2018-04-28T16:18:42Z

chrislovecnm

@justinsb why do we even have this?? How about we bump to 1000?

andrewsykim · 2018-04-29T13:03:37Z

protokube/pkg/gossip/mesh/gossip.go

+	if gossipDnsConnLimit != "" {
+		limit, err := strconv.Atoi(gossipDnsConnLimit)
+		if err != nil {
+			return nil, fmt.Errorf("cannot parse env GOSSIP_DNS_CONN_LIMIT value: %v", gossipDnsConnLimit)


also add err to message

ok, thanks.

andrewsykim · 2018-04-29T13:05:04Z

protokube/pkg/gossip/mesh/gossip.go

+		if err != nil {
+			return nil, fmt.Errorf("cannot parse env GOSSIP_DNS_CONN_LIMIT value: %v", gossipDnsConnLimit)
+		}
+		connLimit = limit


This line can just be above,

connLimit, err := strconv.Atoi(gossipDnsConnLimit)

if we put all in one statement, it will output the following error(create a new inner scope var):

protokube/pkg/gossip/mesh/gossip.go:47:3: connLimit declared and not used

so i split it :)

yancl · 2018-04-30T11:28:02Z

Yes, i really have considered make a PR that just changes the default conn limit from 64 to some larger number, for example, 300, and i prefer that:)

I think the problem with a lot of nodes using gossip dns maybe related to the issue.

chrislovecnm · 2018-05-12T18:32:21Z

@yancl thanks for your patience. I pinged @justinsb on this. It would be good to find out why we limited it

justinsb · 2018-05-12T21:51:37Z

1000 would still be a limit :-)

I thought it was number of peers, I didn't realize it was anything more than that. Do we know that this limit is in fact a problem? I'd be inclined to just bump it to 200 - I think that's what weave did.

chrislovecnm · 2018-05-13T01:03:59Z

@Yanci what have you tested?

yancl · 2018-05-24T02:29:19Z

@chrislovecnm sorry for late response:)

Yes, we have setup a k8s cluster with gossip dns enabled. but as we start more nodes than the limit(64), the new nodes doesn't show in kubectl get nodes, and we found ->[xxx] error during connection attempt:xxx in the syslog. the codes are here.

So i think the num of the total nodes of a k8s cluster can't exceed ConnLimit + 1 when using gossip mesh, we can make such check with the following steps:

support the ConnLimit is 1
and we first start 2 nodes, they mesh well.
then we start the 3rd node, it can't connect any one of the 2 nodes above according code here
if the 3rd node can not make connection with any of those 2 nodes, then it can not get information from them and update the apiserver&etcd servers addrs to the /etc/hosts , so the node will not join the cluster.

Is that correct?

@justinsb Yes, 1000 would still be a limit, we can bump it to 200. 64 is too small for a medium k8s cluster:)

thanks.

levpaul · 2018-07-04T00:57:30Z

I ran into this issue today! 65 nodes was the maximum! Is there any way we can bypass this at the moment? How would one pass this arg through to new nodes too?

ghost · 2018-07-04T02:01:30Z

When building this pr myself, k8s recognized more than 64 nodes.
I put the following settings in master nodes.

/etc/environment
GOSSIP_DNS_CONN_LIMIT=100

and I executed the following commands.

$ cd /var/cache/kubernetes-install
$ ./nodeup --conf=/var/cache/kubernetes-install/kube_env.yaml --v=8

idealhack · 2018-07-04T08:59:49Z

/ok-to-test

jaredallard · 2018-07-14T00:40:33Z

LGTM, this has impacted us as well. Little insane that this is an issue :(

justinsb · 2018-07-20T03:11:12Z

So I don't think this should be an env var that has to be set. I'm going to merge this PR, but I'm then going to send a PR that tries to set it automatically. I'd like to remove the env var at least from having to be set every time you call kops.

My understanding is that 64 nodes shouldn't be a hard limit, but rather each node will limit itself to 64 peers. As long as we don't form two disjoint segments, we should be able to go to much bigger sizes.

But I'll test it out!

/approve
/lgtm

k8s-ci-robot · 2018-07-20T03:11:20Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb, yancl

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [justinsb]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

justinsb · 2018-07-20T03:12:01Z

/retest

This simply turns off gossip connection limits, so we shouldn't ever have to manually configure them. Follow on to kubernetes#5077

justinsb · 2018-07-21T20:25:54Z

I haven't been able to reproduce the problem, but I agree that it should happen! But #5486 should just remove the limit entirely.

Edit: was able to reproduce it. I started at 60 nodes + 1 master and then went to 80 nodes. I think it happens if you have 64 members that form a connected clique and refuse to admit anyone else.

This simply turns off gossip connection limits, so we shouldn't ever have to manually configure them. Follow on to kubernetes#5077

shrinandj · 2020-05-02T04:19:21Z

I understand this issue may be long forgotten. But it might help others who land here... hence this question.

If the connection limit has been removed entirely, the original problem this PR (or having the GOSSIP_DNS_CONN_LIMIT env variable) was trying to fix is also fixed. Explicitly setting GOSSIP_DNS_CONN_LIMIT to some value less than the total number of nodes could lead to problems (of running into a clique), right?

change gossip dns conn limit by ENV

ebfb3c2

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 28, 2018

yancl mentioned this pull request Apr 28, 2018

change gossip dns conn limit by ENV #5075

Closed

k8s-ci-robot assigned andrewsykim Apr 28, 2018

chrislovecnm reviewed Apr 28, 2018

View reviewed changes

andrewsykim reviewed Apr 29, 2018

View reviewed changes

add err to log

8d3ad44

justinsb added this to the 1.10 milestone Jun 2, 2018

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 4, 2018

k8s-ci-robot assigned justinsb Jul 20, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 20, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 20, 2018

k8s-ci-robot merged commit 2dbb6e8 into kubernetes:master Jul 20, 2018

justinsb added a commit to justinsb/kops that referenced this pull request Jul 21, 2018

Remove gossip connection limit entirely

281a82d

This simply turns off gossip connection limits, so we shouldn't ever have to manually configure them. Follow on to kubernetes#5077

justinsb mentioned this pull request Jul 21, 2018

Remove gossip connection limit entirely #5486

Merged

justinsb added a commit to justinsb/kops that referenced this pull request Jul 22, 2018

Remove gossip connection limit entirely

9b70f75

This simply turns off gossip connection limits, so we shouldn't ever have to manually configure them. Follow on to kubernetes#5077

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change gossip dns conn limit by ENV #5077

change gossip dns conn limit by ENV #5077

yancl commented Apr 28, 2018

yancl commented Apr 28, 2018

chrislovecnm left a comment

andrewsykim Apr 29, 2018

yancl Apr 30, 2018

andrewsykim Apr 29, 2018

yancl Apr 30, 2018

yancl commented Apr 30, 2018

chrislovecnm commented May 12, 2018

justinsb commented May 12, 2018

chrislovecnm commented May 13, 2018

yancl commented May 24, 2018

levpaul commented Jul 4, 2018

ghost commented Jul 4, 2018

idealhack commented Jul 4, 2018

jaredallard commented Jul 14, 2018

justinsb commented Jul 20, 2018

k8s-ci-robot commented Jul 20, 2018

justinsb commented Jul 20, 2018

justinsb commented Jul 21, 2018 •

edited

Loading

shrinandj commented May 2, 2020

change gossip dns conn limit by ENV #5077

change gossip dns conn limit by ENV #5077

Conversation

yancl commented Apr 28, 2018

yancl commented Apr 28, 2018

chrislovecnm left a comment

Choose a reason for hiding this comment

andrewsykim Apr 29, 2018

Choose a reason for hiding this comment

yancl Apr 30, 2018

Choose a reason for hiding this comment

andrewsykim Apr 29, 2018

Choose a reason for hiding this comment

yancl Apr 30, 2018

Choose a reason for hiding this comment

yancl commented Apr 30, 2018

chrislovecnm commented May 12, 2018

justinsb commented May 12, 2018

chrislovecnm commented May 13, 2018

yancl commented May 24, 2018

levpaul commented Jul 4, 2018

ghost commented Jul 4, 2018

idealhack commented Jul 4, 2018

jaredallard commented Jul 14, 2018

justinsb commented Jul 20, 2018

k8s-ci-robot commented Jul 20, 2018

justinsb commented Jul 20, 2018

justinsb commented Jul 21, 2018 • edited Loading

shrinandj commented May 2, 2020

justinsb commented Jul 21, 2018 •

edited

Loading