plugin/loadbalance: Improve weights update (#5906)
Don't lock weights for duration of parsing weight file. Add missing check to reject zero weight values. Signed-off-by: Gabor Dozsa <gabor.dozsa@ibm.com>
This commit is contained in:
parent
156da74ad3
commit
52f1c64e7d
2 changed files with 26 additions and 20 deletions
|
@ -254,26 +254,26 @@ func (w *weightedRR) updateWeights() error {
|
|||
scanner := bufio.NewScanner(&buf)
|
||||
|
||||
// Parse the weight file contents
|
||||
err = w.parseWeights(scanner)
|
||||
domains, err := w.parseWeights(scanner)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// access to weights must be protected
|
||||
w.mutex.Lock()
|
||||
w.domains = domains
|
||||
w.mutex.Unlock()
|
||||
|
||||
log.Infof("Successfully reloaded weight file %s", w.fileName)
|
||||
return nil
|
||||
}
|
||||
|
||||
// Parse the weight file contents
|
||||
func (w *weightedRR) parseWeights(scanner *bufio.Scanner) error {
|
||||
// access to weights must be protected
|
||||
w.mutex.Lock()
|
||||
defer w.mutex.Unlock()
|
||||
|
||||
// Reset domains
|
||||
w.domains = make(map[string]weights)
|
||||
|
||||
func (w *weightedRR) parseWeights(scanner *bufio.Scanner) (map[string]weights, error) {
|
||||
var dname string
|
||||
var ws weights
|
||||
domains := make(map[string]weights)
|
||||
|
||||
for scanner.Scan() {
|
||||
nextLine := strings.TrimSpace(scanner.Text())
|
||||
if len(nextLine) == 0 || nextLine[0:1] == "#" {
|
||||
|
@ -285,7 +285,7 @@ func (w *weightedRR) parseWeights(scanner *bufio.Scanner) error {
|
|||
case 1:
|
||||
// (domain) name sanity check
|
||||
if net.ParseIP(fields[0]) != nil {
|
||||
return fmt.Errorf("Wrong domain name:\"%s\" in weight file %s. (Maybe a missing weight value?)",
|
||||
return nil, fmt.Errorf("Wrong domain name:\"%s\" in weight file %s. (Maybe a missing weight value?)",
|
||||
fields[0], w.fileName)
|
||||
}
|
||||
dname = fields[0]
|
||||
|
@ -295,35 +295,35 @@ func (w *weightedRR) parseWeights(scanner *bufio.Scanner) error {
|
|||
dname += "."
|
||||
}
|
||||
var ok bool
|
||||
ws, ok = w.domains[dname]
|
||||
ws, ok = domains[dname]
|
||||
if !ok {
|
||||
ws = make(weights, 0)
|
||||
w.domains[dname] = ws
|
||||
domains[dname] = ws
|
||||
}
|
||||
case 2:
|
||||
// IP address and weight value
|
||||
ip := net.ParseIP(fields[0])
|
||||
if ip == nil {
|
||||
return fmt.Errorf("Wrong IP address:\"%s\" in weight file %s", fields[0], w.fileName)
|
||||
return nil, fmt.Errorf("Wrong IP address:\"%s\" in weight file %s", fields[0], w.fileName)
|
||||
}
|
||||
weight, err := strconv.ParseUint(fields[1], 10, 8)
|
||||
if err != nil {
|
||||
return fmt.Errorf("Wrong weight value:\"%s\" in weight file %s", fields[1], w.fileName)
|
||||
if err != nil || weight == 0 {
|
||||
return nil, fmt.Errorf("Wrong weight value:\"%s\" in weight file %s", fields[1], w.fileName)
|
||||
}
|
||||
witem := &weightItem{address: ip, value: uint8(weight)}
|
||||
if dname == "" {
|
||||
return fmt.Errorf("Missing domain name in weight file %s", w.fileName)
|
||||
return nil, fmt.Errorf("Missing domain name in weight file %s", w.fileName)
|
||||
}
|
||||
ws = append(ws, witem)
|
||||
w.domains[dname] = ws
|
||||
domains[dname] = ws
|
||||
default:
|
||||
return fmt.Errorf("Could not parse weight line:\"%s\" in weight file %s", nextLine, w.fileName)
|
||||
return nil, fmt.Errorf("Could not parse weight line:\"%s\" in weight file %s", nextLine, w.fileName)
|
||||
}
|
||||
}
|
||||
|
||||
if err := scanner.Err(); err != nil {
|
||||
return fmt.Errorf("Weight file %s parsing error:%s", w.fileName, err)
|
||||
return nil, fmt.Errorf("Weight file %s parsing error:%s", w.fileName, err)
|
||||
}
|
||||
|
||||
return nil
|
||||
return domains, nil
|
||||
}
|
||||
|
|
|
@ -79,6 +79,11 @@ w1,example.org
|
|||
192.168.1.14 300
|
||||
`
|
||||
|
||||
const zeroWeightWRR = `
|
||||
w1,example.org
|
||||
192.168.1.14 0
|
||||
`
|
||||
|
||||
func TestWeightFileUpdate(t *testing.T) {
|
||||
tests := []struct {
|
||||
weightFilContent string
|
||||
|
@ -95,6 +100,7 @@ func TestWeightFileUpdate(t *testing.T) {
|
|||
{missingDomainWRR, true, nil, "Missing domain name"},
|
||||
{wrongIpWRR, true, nil, "Wrong IP address"},
|
||||
{wrongWeightWRR, true, nil, "Wrong weight value"},
|
||||
{zeroWeightWRR, true, nil, "Wrong weight value"},
|
||||
}
|
||||
|
||||
for i, test := range tests {
|
||||
|
|
Loading…
Add table
Reference in a new issue