From 4d28391c240c78febb2c26ce4d7bd084e1af6f5e Mon Sep 17 00:00:00 2001 From: Hristo Venev Date: Mon, 26 Oct 2020 01:40:05 -0700 Subject: [PATCH] Fix double counting of IP addresses The range allocator in pkg/controller/nodeipam/ipam/range_allocator.go may call Occupy() on the same range twice: 1. Just before subscribing to the NodeInformer 2. From a callback given to the NodeInformer soon after registration --- .../nodeipam/ipam/cidrset/cidr_set.go | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/pkg/controller/nodeipam/ipam/cidrset/cidr_set.go b/pkg/controller/nodeipam/ipam/cidrset/cidr_set.go index 359f830e3a6..91a946c9eea 100644 --- a/pkg/controller/nodeipam/ipam/cidrset/cidr_set.go +++ b/pkg/controller/nodeipam/ipam/cidrset/cidr_set.go @@ -232,16 +232,20 @@ func (s *CidrSet) Release(cidr *net.IPNet) error { s.Lock() defer s.Unlock() for i := begin; i <= end; i++ { - s.used.SetBit(&s.used, i, 0) - s.allocatedCIDRs-- - cidrSetReleases.WithLabelValues(s.label).Inc() + // Only change the counters if we change the bit to prevent + // double counting. + if s.used.Bit(i) != 0 { + s.used.SetBit(&s.used, i, 0) + s.allocatedCIDRs-- + cidrSetReleases.WithLabelValues(s.label).Inc() + } } cidrSetUsage.WithLabelValues(s.label).Set(float64(s.allocatedCIDRs) / float64(s.maxCIDRs)) return nil } -// Occupy marks the given CIDR range as used. Occupy does not check if the CIDR +// Occupy marks the given CIDR range as used. Occupy succeeds even if the CIDR // range was previously used. func (s *CidrSet) Occupy(cidr *net.IPNet) (err error) { begin, end, err := s.getBeginingAndEndIndices(cidr) @@ -251,9 +255,13 @@ func (s *CidrSet) Occupy(cidr *net.IPNet) (err error) { s.Lock() defer s.Unlock() for i := begin; i <= end; i++ { - s.used.SetBit(&s.used, i, 1) - s.allocatedCIDRs++ - cidrSetAllocations.WithLabelValues(s.label).Inc() + // Only change the counters if we change the bit to prevent + // double counting. + if s.used.Bit(i) == 0 { + s.used.SetBit(&s.used, i, 1) + s.allocatedCIDRs++ + cidrSetAllocations.WithLabelValues(s.label).Inc() + } } cidrSetUsage.WithLabelValues(s.label).Set(float64(s.allocatedCIDRs) / float64(s.maxCIDRs))