From 1ec999de9c538c294b05d1bdc3e3f8d094eb1a60 Mon Sep 17 00:00:00 2001 From: Gabe Conradi Date: Fri, 20 Jun 2014 19:02:35 -0400 Subject: [PATCH 1/6] fix getNextAvailableAddress to use local maximums instead of just last address --- app/models/IpAddresses.scala | 2 +- app/models/shared/IpAddressable.scala | 45 +++++++++++++++++++++------ 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/app/models/IpAddresses.scala b/app/models/IpAddresses.scala index 9c90f9e23..1a85262c4 100644 --- a/app/models/IpAddresses.scala +++ b/app/models/IpAddresses.scala @@ -62,7 +62,7 @@ object IpAddresses extends IpAddressStorage[IpAddresses] with IpAddressCacheMana case failed => logger.info("Address cache dirty. Failed cache lookup so using DB") populateCacheIfNeeded(scope, true) - getCurrentMaxAddress(calc.minAddressAsLong, calc.maxAddressAsLong) + getCurrentLowestLocalMaxAddress(calc.minAddressAsLong, calc.maxAddressAsLong) } val address: Long = calc.nextAvailableAsLong(currentMax) (gateway, address, netmask) diff --git a/app/models/shared/IpAddressable.scala b/app/models/shared/IpAddressable.scala index 7f6971655..ffe5f73d8 100644 --- a/app/models/shared/IpAddressable.scala +++ b/app/models/shared/IpAddressable.scala @@ -27,7 +27,7 @@ trait IpAddressable extends ValidatedEntity[Long] { def getId(): Long = id def getAssetId(): Long = asset_id def getAsset(): Asset = Asset.findById(getAssetId()).get - + } trait IpAddressStorage[T <: IpAddressable] extends Schema with AnormAdapter[T] { @@ -75,15 +75,15 @@ trait IpAddressStorage[T <: IpAddressable] extends Schema with AnormAdapter[T] { } def getNextAvailableAddress(overrideStart: Option[String] = None)(implicit scope: Option[String]): Tuple3[Long,Long,Long] = { + //this is used by ip allocation without pools (i.e. IPMI) val network = getNetwork val startAt = overrideStart.orElse(getStartAddress) val calc = IpAddressCalc(network, startAt) val gateway: Long = getGateway().getOrElse(calc.minAddressAsLong) val netmask: Long = calc.netmaskAsLong - val currentMax: Option[Long] = getCurrentMaxAddress( - calc.minAddressAsLong, calc.maxAddressAsLong - ) - val address: Long = calc.nextAvailableAsLong(currentMax) + // look for the local maximum address (i.e. the last used address in a continuous sequence from startAddress) + val localMax: Option[Long] = getCurrentLowestLocalMaxAddress(calc.minAddressAsLong, calc.maxAddressAsLong) + val address: Long = calc.nextAvailableAsLong(localMax) (gateway, address, netmask) } @@ -95,7 +95,7 @@ trait IpAddressStorage[T <: IpAddressable] extends Schema with AnormAdapter[T] { var i = 0 do { try { - res = Some(f(i)) + res = Some(f(i)) } catch { case e: SQLException => logger.info("createAddressWithRetry attempt %d: %s".format((i + 1), e.getMessage)) @@ -115,14 +115,39 @@ trait IpAddressStorage[T <: IpAddressable] extends Schema with AnormAdapter[T] { ) } - protected def getCurrentMaxAddress(minAddress: Long, maxAddress: Long)(implicit scope: Option[String]): Option[Long] = inTransaction { - from(tableDef)(t => + /* + * returns the lowest last used address in a continuous sequence from minAddress + * If maxAddress is the same as the lowest last used address, we return None which signifies + * that you should start allocating addresses from the start of the range. + * + * Ex: For a range 0L..20L, used addresses List(1,2,3,4,5,13,14,15,19,20), the result will be Some(5) + * For a range 0L..20L, used addresses List(5,6,7,8,19,20), the result will be Some(8) + * For a range 0L..20L, used addresses List(17,18,19,20), the result will be None (allocate from beginning) + */ + protected def getCurrentLowestLocalMaxAddress(minAddress: Long, maxAddress: Long)(implicit scope: Option[String]): Option[Long] = { + val sortedAddresses = from(tableDef)(t => where( (t.address gte minAddress) and (t.address lte maxAddress) ) - compute(max(t.address)) - ) + select(t.address) + orderBy(t.address asc) + ).toList + val localMaximaAddresses = for ( + localMax <- sortedAddresses if !sortedAddresses.contains(localMax + 1) + ) yield localMax + + localMaximaAddresses.isEmpty match { + case true => None + case false => { + val localMax = localMaximaAddresses.head + localMax match { + //if we are at the end of our range, start from the beginning + case limit if localMax == maxAddress => None + case _ => Some(localMax) + } + } + } } protected def getGateway()(implicit scope: Option[String]): Option[Long] = getConfig() match { From 8a5408eda1364e026e7cc63e9b39d5c53625c597 Mon Sep 17 00:00:00 2001 From: Gabe Conradi Date: Sat, 21 Jun 2014 18:04:56 -0400 Subject: [PATCH 2/6] use sorted set instead --- app/models/shared/IpAddressable.scala | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/models/shared/IpAddressable.scala b/app/models/shared/IpAddressable.scala index ffe5f73d8..8f6bfe2f1 100644 --- a/app/models/shared/IpAddressable.scala +++ b/app/models/shared/IpAddressable.scala @@ -6,6 +6,7 @@ import util.{IpAddress, IpAddressCalc} import org.squeryl.Schema import play.api.Logger import java.sql.SQLException +import collection.immutable.SortedSet trait IpAddressable extends ValidatedEntity[Long] { @@ -125,14 +126,15 @@ trait IpAddressStorage[T <: IpAddressable] extends Schema with AnormAdapter[T] { * For a range 0L..20L, used addresses List(17,18,19,20), the result will be None (allocate from beginning) */ protected def getCurrentLowestLocalMaxAddress(minAddress: Long, maxAddress: Long)(implicit scope: Option[String]): Option[Long] = { - val sortedAddresses = from(tableDef)(t => + val addresses = from(tableDef)(t => where( (t.address gte minAddress) and (t.address lte maxAddress) ) select(t.address) orderBy(t.address asc) - ).toList + ).toSet + val sortedAddresses = SortedSet[Long]() ++ addresses val localMaximaAddresses = for ( localMax <- sortedAddresses if !sortedAddresses.contains(localMax + 1) ) yield localMax From eee92c323194cfbf2ad41e14af15f5597c800266 Mon Sep 17 00:00:00 2001 From: Gabe Conradi Date: Sat, 21 Jun 2014 18:37:33 -0400 Subject: [PATCH 3/6] use headOption and if instead --- app/models/shared/IpAddressable.scala | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/app/models/shared/IpAddressable.scala b/app/models/shared/IpAddressable.scala index 8f6bfe2f1..172b80463 100644 --- a/app/models/shared/IpAddressable.scala +++ b/app/models/shared/IpAddressable.scala @@ -134,21 +134,21 @@ trait IpAddressStorage[T <: IpAddressable] extends Schema with AnormAdapter[T] { select(t.address) orderBy(t.address asc) ).toSet + val sortedAddresses = SortedSet[Long]() ++ addresses val localMaximaAddresses = for ( - localMax <- sortedAddresses if !sortedAddresses.contains(localMax + 1) + localMax <- sortedAddresses if !sortedAddresses.contains(localMax + 1L) ) yield localMax - localMaximaAddresses.isEmpty match { - case true => None - case false => { - val localMax = localMaximaAddresses.head - localMax match { + localMaximaAddresses.headOption match { + case Some(localMax) => + if (localMax == maxAddress) { //if we are at the end of our range, start from the beginning - case limit if localMax == maxAddress => None - case _ => Some(localMax) + None + } else { + Some(localMax) } - } + case _ => None } } From 5b878c05908d5297d912619fc8c6ac2121aec4f5 Mon Sep 17 00:00:00 2001 From: Gabe Conradi Date: Sat, 21 Jun 2014 18:49:40 -0400 Subject: [PATCH 4/6] use more elegant flatMap and for comprehension --- app/models/shared/IpAddressable.scala | 36 +++++++++++++-------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/app/models/shared/IpAddressable.scala b/app/models/shared/IpAddressable.scala index 172b80463..21f325c70 100644 --- a/app/models/shared/IpAddressable.scala +++ b/app/models/shared/IpAddressable.scala @@ -126,30 +126,30 @@ trait IpAddressStorage[T <: IpAddressable] extends Schema with AnormAdapter[T] { * For a range 0L..20L, used addresses List(17,18,19,20), the result will be None (allocate from beginning) */ protected def getCurrentLowestLocalMaxAddress(minAddress: Long, maxAddress: Long)(implicit scope: Option[String]): Option[Long] = { - val addresses = from(tableDef)(t => + val sortedAddresses = from(tableDef)(t => where( (t.address gte minAddress) and (t.address lte maxAddress) ) select(t.address) orderBy(t.address asc) - ).toSet - - val sortedAddresses = SortedSet[Long]() ++ addresses - val localMaximaAddresses = for ( - localMax <- sortedAddresses if !sortedAddresses.contains(localMax + 1L) - ) yield localMax - - localMaximaAddresses.headOption match { - case Some(localMax) => - if (localMax == maxAddress) { - //if we are at the end of our range, start from the beginning - None - } else { - Some(localMax) - } - case _ => None - } + ).toSeq + + val localMaximaAddresses = for { + i <- 0 to sortedAddresses.size -2 + curr = sortedAddresses(i) + next = sortedAddresses(i+1) + if (next > curr + 1) + } yield curr + + localMaximaAddresses.headOption.flatMap(localMax => + if (localMax == maxAddress) { + //if we are at the end of our range, start from the beginning + None + } else { + Some(localMax) + } + ) } protected def getGateway()(implicit scope: Option[String]): Option[Long] = getConfig() match { From 3be2e7598532af730188693a6b3ed502a1cf9152 Mon Sep 17 00:00:00 2001 From: Gabe Conradi Date: Sun, 22 Jun 2014 20:20:12 -0400 Subject: [PATCH 5/6] use lazy streams --- app/models/shared/IpAddressable.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/shared/IpAddressable.scala b/app/models/shared/IpAddressable.scala index 21f325c70..a84678db1 100644 --- a/app/models/shared/IpAddressable.scala +++ b/app/models/shared/IpAddressable.scala @@ -135,8 +135,8 @@ trait IpAddressStorage[T <: IpAddressable] extends Schema with AnormAdapter[T] { orderBy(t.address asc) ).toSeq - val localMaximaAddresses = for { - i <- 0 to sortedAddresses.size -2 + lazy val localMaximaAddresses = for { + i <- Stream.range(0, sortedAddresses.size-2) curr = sortedAddresses(i) next = sortedAddresses(i+1) if (next > curr + 1) From f70673128e3d6a12d6c79c268440fb1e05b99953 Mon Sep 17 00:00:00 2001 From: Gabe Conradi Date: Mon, 23 Jun 2014 10:43:43 -0400 Subject: [PATCH 6/6] make the function actually work now --- app/models/shared/IpAddressable.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/shared/IpAddressable.scala b/app/models/shared/IpAddressable.scala index a84678db1..28376ed3d 100644 --- a/app/models/shared/IpAddressable.scala +++ b/app/models/shared/IpAddressable.scala @@ -136,10 +136,10 @@ trait IpAddressStorage[T <: IpAddressable] extends Schema with AnormAdapter[T] { ).toSeq lazy val localMaximaAddresses = for { - i <- Stream.range(0, sortedAddresses.size-2) + i <- Range(0, sortedAddresses.size-1).inclusive.toStream curr = sortedAddresses(i) - next = sortedAddresses(i+1) - if (next > curr + 1) + next = sortedAddresses.lift(i+1) + if (next.map{_ > curr + 1}.getOrElse(true)) } yield curr localMaximaAddresses.headOption.flatMap(localMax =>