Merge pull request #624 from justinsb/fix_623

Compute server_names_hash_bucket_size correctly

controllers/nginx/pkg/cmd/controller/nginx.go

@@ -363,7 +363,7 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) ([]byte, er
 	// if is required.
 	// https://trac.nginx.org/nginx/ticket/352
 	// https://trac.nginx.org/nginx/ticket/631
-	nameHashBucketSize := nextPowerOf2(longestName)
+	nameHashBucketSize := nginxHashBucketSize(longestName)
 	if cfg.ServerNameHashBucketSize == 0 {
 		glog.V(3).Infof("adjusting ServerNameHashBucketSize variable to %v", nameHashBucketSize)
 		cfg.ServerNameHashBucketSize = nameHashBucketSize
@@ -449,6 +449,16 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) ([]byte, er
 	return content, nil
 }
 
+// nginxHashBucketSize computes the correct nginx hash_bucket_size for a hash with the given longest key
+func nginxHashBucketSize(longestString int) int {
+	// See https://github.com/kubernetes/ingress/issues/623 for an explanation
+	wordSize := 8 // Assume 64 bit CPU
+	n := longestString + 2
+	aligned := (n + wordSize - 1) & ^(wordSize - 1)
+	rawSize := wordSize + wordSize + aligned
+	return nextPowerOf2(rawSize)
+}
+
 // Name returns the healthcheck name
 func (n NGINXController) Name() string {
 	return "Ingress Controller"

controllers/nginx/pkg/cmd/controller/nginx_test.go

@@ -0,0 +1,59 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package main
+
+import "testing"
+
+func TestNginxHashBucketSize(t *testing.T) {
+	tests := []struct {
+		n        int
+		expected int
+	}{
+		{0, 32},
+		{1, 32},
+		{2, 32},
+		{3, 32},
+		// ...
+		{13, 32},
+		{14, 32},
+		{15, 64},
+		{16, 64},
+		// ...
+		{45, 64},
+		{46, 64},
+		{47, 128},
+		{48, 128},
+		// ...
+		// ...
+		{109, 128},
+		{110, 128},
+		{111, 256},
+		{112, 256},
+		// ...
+		{237, 256},
+		{238, 256},
+		{239, 512},
+		{240, 512},
+	}
+
+	for _, test := range tests {
+		actual := nginxHashBucketSize(test.n)
+		if actual != test.expected {
+			t.Errorf("Test nginxHashBucketSize(%d): expected %d but returned %d", test.n, test.expected, actual)
+		}
+	}
+}