Merge pull request #1752 from jcaamano/merge-2023-07-11

[DownstreamMerge] 11 jul 23

contrib/kind.sh

@@ -1169,8 +1169,19 @@ docker_create_second_disconnected_interface() {
   local bridge_name="${1:-kindexgw}"
   echo "bridge: $bridge_name"
 
+  if [ "${OCI_BIN}" = "podman" ]; then
+    # docker and podman do different things with the --internal parameter:
+    # - docker installs iptables rules to drop traffic on a different subnet
+    #   than the bridge and we don't want that.
+    # - podman does not set the bridge as default gateway and we want that.
+    # So we need it with podman but not with docker. Neither allows us to create
+    # a bridge network without IPAM which would be ideal, so perhaps the best
+    # option would be a manual setup.
+    local podman_params="--internal"
+  fi
+
   # Create the network without subnets; ignore if already exists.
-  "$OCI_BIN" network create --internal --driver=bridge "$bridge_name" || true
+  "$OCI_BIN" network create --driver=bridge ${podman_params-} "$bridge_name" || true
 
   KIND_NODES=$(kind get nodes --name "${KIND_CLUSTER_NAME}")
   for n in $KIND_NODES; do
@@ -1288,4 +1299,4 @@ if [ "$KIND_INSTALL_METALLB" == true ]; then
 fi
 if [ "$KIND_INSTALL_PLUGINS" == true ]; then
   install_plugins
-fi
+fi

go-controller/hack/test-go.sh

@@ -59,7 +59,7 @@ function testrun {
         echo "Increasing timeout to 20m for package ${pkg}"
         args="${args} -test.timeout=20m"
     fi
-    if grep -q -r "ginkgo" ."${path}"; then
+    if grep -q "ginkgo" ."${path}"/*_test.go; then
 	    prefix=$(echo "${path}" | cut -c 2- | sed 's,/,_,g')
         ginkgoargs="-ginkgo.v ${ginkgo_focus} -ginkgo.reportFile ${TEST_REPORT_DIR}/junit-${prefix}.xml"
     fi

go-controller/pkg/ovn/ipallocator/allocator/bitmap.go → go-controller/pkg/allocator/bitmap/bitmap.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package allocator
+package bitmap
 
 import (
 	"errors"

go-controller/pkg/ovn/ipallocator/allocator/bitmap_test.go → go-controller/pkg/allocator/bitmap/bitmap_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package allocator
+package bitmap
 
 import (
 	"testing"

go-controller/pkg/ovn/ipallocator/allocator/interfaces.go → go-controller/pkg/allocator/bitmap/interfaces.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package allocator
+package bitmap
 
 // Interface manages the allocation of items out of a range. Interface
 // should be threadsafe.

go-controller/pkg/ovn/ipallocator/allocator/utils.go → go-controller/pkg/allocator/bitmap/utils.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package allocator
+package bitmap
 
 import "math/big"
 

go-controller/pkg/ovn/ipallocator/allocator/utils_test.go → go-controller/pkg/allocator/bitmap/utils_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package allocator
+package bitmap
 
 import (
 	"math/big"

go-controller/pkg/clustermanager/id_allocator.go → go-controller/pkg/allocator/id/allocator.go

@@ -1,35 +1,50 @@
-package clustermanager
+package id
 
 import (
 	"fmt"
 	"sync"
 
-	bitmapallocator "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/ovn/ipallocator/allocator"
+	bitmapallocator "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/bitmap"
 )
 
 const (
 	invalidID = -1
 )
 
+// Allocator of IDs for a set of resources identified by name
+type Allocator interface {
+	AllocateID(name string) (int, error)
+	ReserveID(name string, id int) error
+	ReleaseID(name string)
+	ForName(name string) NamedAllocator
+}
+
+// NamedAllocator of IDs for a specific resource
+type NamedAllocator interface {
+	AllocateID() (int, error)
+	ReserveID(int) error
+	ReleaseID()
+}
+
 // idAllocator is used to allocate id for a resource and store the resource - id in a map
 type idAllocator struct {
 	nameIdMap sync.Map
 	idBitmap  *bitmapallocator.AllocationBitmap
 }
 
 // NewIDAllocator returns an IDAllocator
-func NewIDAllocator(name string, maxIds int) (*idAllocator, error) {
-	idBitmap := bitmapallocator.NewContiguousAllocationMap(maxIds, name)
+func NewIDAllocator(name string, maxIds int) (Allocator, error) {
+	idBitmap := bitmapallocator.NewRoundRobinAllocationMap(maxIds, name)
 
 	return &idAllocator{
 		nameIdMap: sync.Map{},
 		idBitmap:  idBitmap,
 	}, nil
 }
 
-// allocateID allocates an id for the resource 'name' and returns the id.
+// AllocateID allocates an id for the resource 'name' and returns the id.
 // If the id for the resource is already allocated, it returns the cached id.
-func (idAllocator *idAllocator) allocateID(name string) (int, error) {
+func (idAllocator *idAllocator) AllocateID(name string) (int, error) {
 	// Check the idMap and return the id if its already allocated
 	v, ok := idAllocator.nameIdMap.Load(name)
 	if ok {
@@ -46,11 +61,11 @@ func (idAllocator *idAllocator) allocateID(name string) (int, error) {
 	return id, nil
 }
 
-// reserveID reserves the id 'id' for the resource 'name'. It returns an
+// ReserveID reserves the id 'id' for the resource 'name'. It returns an
 // error if the 'id' is already reserved by a resource other than 'name'.
 // It also returns an error if the resource 'name' has a different 'id'
 // already reserved.
-func (idAllocator *idAllocator) reserveID(name string, id int) error {
+func (idAllocator *idAllocator) ReserveID(name string, id int) error {
 	v, ok := idAllocator.nameIdMap.Load(name)
 	if ok {
 		if v.(int) == id {
@@ -69,11 +84,35 @@ func (idAllocator *idAllocator) reserveID(name string, id int) error {
 	return nil
 }
 
-// releaseID releases the id allocated for the resource 'name'
-func (idAllocator *idAllocator) releaseID(name string) {
+// ReleaseID releases the id allocated for the resource 'name'
+func (idAllocator *idAllocator) ReleaseID(name string) {
 	v, ok := idAllocator.nameIdMap.Load(name)
 	if ok {
 		idAllocator.idBitmap.Release(v.(int))
 		idAllocator.nameIdMap.Delete(name)
 	}
 }
+
+func (idAllocator *idAllocator) ForName(name string) NamedAllocator {
+	return &namedAllocator{
+		name:      name,
+		allocator: idAllocator,
+	}
+}
+
+type namedAllocator struct {
+	name      string
+	allocator *idAllocator
+}
+
+func (allocator *namedAllocator) AllocateID() (int, error) {
+	return allocator.allocator.AllocateID(allocator.name)
+}
+
+func (allocator *namedAllocator) ReserveID(id int) error {
+	return allocator.allocator.ReserveID(allocator.name, id)
+}
+
+func (allocator *namedAllocator) ReleaseID() {
+	allocator.allocator.ReleaseID(allocator.name)
+}

go-controller/pkg/ovn/ipallocator/allocator.go → go-controller/pkg/allocator/ip/allocator.go

@@ -14,15 +14,15 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package ipallocator
+package ip
 
 import (
 	"errors"
 	"fmt"
 	"math/big"
 	"net"
 
-	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/ovn/ipallocator/allocator"
+	allocator "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/bitmap"
 	utilnet "k8s.io/utils/net"
 )
 
@@ -35,13 +35,19 @@ type Interface interface {
 	ForEach(func(net.IP))
 	CIDR() net.IPNet
 	Has(ip net.IP) bool
+	Reserved(ip net.IP) bool
 }
 
 var (
 	ErrFull      = errors.New("range is full")
 	ErrAllocated = errors.New("provided IP is already allocated")
 )
 
+// IsErrAllocated returns true if err is of type ErrAllocated
+func IsErrAllocated(err error) bool {
+	return errors.Is(err, ErrAllocated)
+}
+
 type ErrNotInRange struct {
 	ValidRange string
 }
@@ -199,6 +205,17 @@ func (r *Range) Has(ip net.IP) bool {
 	return r.alloc.Has(offset)
 }
 
+// Reserved returns true if the provided IP can't be allocated. This is *only*
+// true for the network and broadcast addresses.
+func (r *Range) Reserved(ip net.IP) bool {
+	if !r.net.Contains(ip) {
+		return false
+	}
+
+	offset := calculateIPOffset(r.base, ip)
+	return offset == -1 || offset == r.max
+}
+
 // contains returns true and the offset if the ip is in the range, and false
 // and nil otherwise. The first and last addresses of the CIDR are omitted.
 func (r *Range) contains(ip net.IP) (bool, int) {

go-controller/pkg/ovn/ipallocator/allocator_test.go → go-controller/pkg/allocator/ip/allocator_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package ipallocator
+package ip
 
 import (
 	"net"
@@ -258,3 +258,30 @@ func TestForEach(t *testing.T) {
 		}
 	}
 }
+
+func TestReserved(t *testing.T) {
+	_, cidr, err := net.ParseCIDR("192.168.1.0/24")
+	if err != nil {
+		t.Fatal(err)
+	}
+	r, err := NewCIDRRange(cidr)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if !r.Reserved(net.ParseIP("192.168.1.0")) {
+		t.Errorf("should be a reserved address: %s", "192.168.1.0")
+	}
+
+	if !r.Reserved(net.ParseIP("192.168.1.255")) {
+		t.Errorf("should be a reserved address: %s", "192.168.1.255")
+	}
+
+	if r.Reserved(net.ParseIP("192.168.1.1")) {
+		t.Errorf("should not be a reserved address: %s", "192.168.1.1")
+	}
+
+	if r.Reserved(net.ParseIP("192.168.1.254")) {
+		t.Errorf("should not be a reserved address: %s", "192.168.1.254")
+	}
+}