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

ConsistentHashLB.minimumRingSize defaults to 0 #13261

Open
FengLiqiong opened this issue Apr 12, 2019 · 1 comment

Comments

3 participants
@FengLiqiong
Copy link

commented Apr 12, 2019

Describe the bug
ConsistentHashLB.minimumRingSize defaults to 0.

Expected behavior
It should default to 1024 as specified in https://istio.io/docs/reference/config/networking/v1alpha3/destination-rule/#LoadBalancerSettings-ConsistentHashLB.

Steps to reproduce the bug
Add the following into destinationrule:

  trafficPolicy:
    loadBalancer:
      consistentHash:
        httpCookie:
          name: id
          ttl: 0s

run istioctl proxy-config clusters -n istio-system $(kubectl get pods -n istio-system | grep ingressgateway | awk '{print $1}') -o json, then you can see the following envoy config in the corresponding cluster:

        "ringHashLbConfig": {
            "minimumRingSize": "0"
        },

If there are 2 endpoints in the cluster, and you send http requests to the cluster, you can see around 90% http requests are sent to one of the endpoint and 10% to the other.

Version

$ bin/istioctl version --remote
client version: version.BuildInfo{Version:"1.1.1", GitRevision:"2b1331886076df103179e3da5dc9077fed59c989", User:"root", Host:"7077232d-4c6c-11e9-813c-0a580a2c0506", GolangVersion:"go1.10.4", DockerHub:"docker.io/istio", BuildStatus:"Clean", GitTag:"1.1.0-17-g2b13318"}
citadel version: version.BuildInfo{Version:"1.1.0", GitRevision:"82797c0c0649a3f73029b33957ae105260458c6e-dirty", User:"root", Host:"1f4e9557-49cd-11e9-86e9-0a580a2c0304", GolangVersion:"go1.10.4", DockerHub:"docker.io/istio", BuildStatus:"Modified", GitTag:"1.1.0-rc.6"}
galley version: version.BuildInfo{Version:"1.1.0", GitRevision:"82797c0c0649a3f73029b33957ae105260458c6e-dirty", User:"root", Host:"1f4e9557-49cd-11e9-86e9-0a580a2c0304", GolangVersion:"go1.10.4", DockerHub:"docker.io/istio", BuildStatus:"Modified", GitTag:"1.1.0-rc.6"}
ingressgateway version: version.BuildInfo{Version:"1.1.0", GitRevision:"82797c0c0649a3f73029b33957ae105260458c6e", User:"root", Host:"1f4e9557-49cd-11e9-86e9-0a580a2c0304", GolangVersion:"go1.10.4", DockerHub:"docker.io/istio", BuildStatus:"Clean", GitTag:"1.1.0-rc.6"}
pilot version: version.BuildInfo{Version:"1.1.0", GitRevision:"82797c0c0649a3f73029b33957ae105260458c6e-dirty", User:"root", Host:"1f4e9557-49cd-11e9-86e9-0a580a2c0304", GolangVersion:"go1.10.4", DockerHub:"docker.io/istio", BuildStatus:"Modified", GitTag:"1.1.0-rc.6"}
policy version: version.BuildInfo{Version:"1.1.0", GitRevision:"82797c0c0649a3f73029b33957ae105260458c6e-dirty", User:"root", Host:"1f4e9557-49cd-11e9-86e9-0a580a2c0304", GolangVersion:"go1.10.4", DockerHub:"docker.io/istio", BuildStatus:"Modified", GitTag:"1.1.0-rc.6"}
sidecar-injector version: version.BuildInfo{Version:"1.1.0", GitRevision:"82797c0c0649a3f73029b33957ae105260458c6e-dirty", User:"root", Host:"1f4e9557-49cd-11e9-86e9-0a580a2c0304", GolangVersion:"go1.10.4", DockerHub:"docker.io/istio", BuildStatus:"Modified", GitTag:"1.1.0-rc.6"}
telemetry version: version.BuildInfo{Version:"1.1.0", GitRevision:"82797c0c0649a3f73029b33957ae105260458c6e-dirty", User:"root", Host:"1f4e9557-49cd-11e9-86e9-0a580a2c0304", GolangVersion:"go1.10.4", DockerHub:"docker.io/istio", BuildStatus:"Modified", GitTag:"1.1.0-rc.6"}

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"13", GitVersion:"v1.13.0", GitCommit:"ddf47ac13c1a9483ea035a79cd7c10005ff21a6d", GitTreeState:"clean", BuildDate:"2018-12-03T21:04:45Z", GoVersion:"go1.11.2", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"13", GitVersion:"v1.13.2", GitCommit:"cff46ab41ff0bb44d8584413b598ad8360ec1def", GitTreeState:"clean", BuildDate:"2019-01-10T23:28:14Z", GoVersion:"go1.11.4", Compiler:"gc", Platform:"linux/amd64"}

Installation
https://istio.io/docs/setup/kubernetes/install/helm/#option-2-install-with-helm-and-tiller-via-helm-install

@GregHanson

This comment has been minimized.

Copy link
Member

commented Apr 12, 2019

definitely a bug with how the proto was generated. cluster.go sets the field as:

		cluster.LbConfig = &apiv2.Cluster_RingHashLbConfig_{
			RingHashLbConfig: &apiv2.Cluster_RingHashLbConfig{
				MinimumRingSize: &types.UInt64Value{Value: consistentHash.GetMinimumRingSize()},
			},
		}

And unfortunately the generated method in DestinationRule for GetMinimumRingSize() returns 0 in the nil case, rather than respecting the field as unset:

func (m *LoadBalancerSettings_ConsistentHashLB) GetMinimumRingSize() uint64 {
	if m != nil {
		return m.MinimumRingSize
	}
	return 0
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.