Skip to content

Commit

Permalink
Compute server_names_hash_bucket_size correctly
Browse files Browse the repository at this point in the history
There were some edge cases where we did not calculate hash_bucket_size
correctly.

Fix #623
  • Loading branch information
justinsb committed Apr 19, 2017
1 parent 827e6f1 commit 322be61
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 1 deletion.
12 changes: 11 additions & 1 deletion controllers/nginx/pkg/cmd/controller/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down
59 changes: 59 additions & 0 deletions controllers/nginx/pkg/cmd/controller/nginx_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}

0 comments on commit 322be61

Please sign in to comment.