Skip to content

Commit

Permalink
Merge pull request #246 from broadinstitute/lockNew
Browse files Browse the repository at this point in the history
add lock
  • Loading branch information
Qi77Qi committed Nov 20, 2018
2 parents 79417ba + b0a8e7e commit b8870b9
Show file tree
Hide file tree
Showing 27 changed files with 353 additions and 194 deletions.
6 changes: 4 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ crashlytics-build.properties

*.pid

newrelic-agent-4.6.0.jar

### Automation things
automation/report.xml

newrelic-agent-4.6.0.jar
automation/test-reports/
automation/src/test/resources/
2 changes: 1 addition & 1 deletion automation/Dockerfile-tests
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM iflavoursbv/sbt-openjdk-8-alpine
FROM bigtruedata/sbt

COPY src /app/src
COPY test.sh /app
Expand Down
24 changes: 5 additions & 19 deletions automation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,6 @@ See [firecloud-automated-testing](https://github.com/broadinstitute/firecloud-au

### Set Up

```
brew install chromedriver
```

Note: Leonardo integration tests are not currently web-based but may fail due to dependencies without chromedriver

Render configs:
```bash
./render-local-env.sh [branch of firecloud-automated-testing] [vault token] [env] [service root]
Expand All @@ -31,22 +25,14 @@ Render configs:
* service root
* the name of your local clone of sam if not `sam`

##### Using a local UI

Set `LOCAL_UI=true` before calling `render-local-env.sh`. When starting your UI, run:

```bash
FIAB=true ./config/docker-rsync-local-ui.sh
```

### Run tests

#### From IntelliJ

First, you need to set some default VM parameters for ScalaTest run configurations. In IntelliJ, go to `Run` > `Edit Configurations...`, select `ScalaTest` under `Defaults`, and add these VM parameters:

```
-Djsse.enableSNIExtension=false -Dheadless=false
-Djsse.enableSNIExtension=false
```

Also make sure that there is a `Build` task configured to run before launch.
Expand All @@ -58,21 +44,21 @@ Now, simply open the test spec, right-click on the class name or a specific test
To run all tests:

```bash
sbt test -Djsse.enableSNIExtension=false -Dheadless=false
sbt test
```

To run a single suite:

```bash
sbt -Djsse.enableSNIExtension=false -Dheadless=false "testOnly *GoogleSpec"
sbt "testOnly *SamApiSpec"
```

To run a single test within a suite:

```bash
# matches test via substring
sbt -Djsse.enableSNIExtension=false -Dheadless=false "testOnly *GoogleSpec -- -z \"have a search field\""
sbt "testOnly *SamApiSpec -- -z \"have a search field\""
```

For more information see [SBT's documentation](http://www.scala-sbt.org/0.13/docs/Testing.html#Test+Framework+Arguments).
For more information see [SBT's documentation](https://www.scala-sbt.org/1.x/docs/Testing.html#testOnly).

10 changes: 4 additions & 6 deletions automation/project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ object Dependencies {
val akkaHttpV = "10.0.10"

val workbenchModelV = "0.13-7e86fba"
val workbenchGoogleV = "0.16-847c3ff"
val workbenchServiceTestV = "0.15-7e86fba"
val workbenchGoogleV = "0.18-6942040"
val workbenchServiceTestV = "0.15-6942040"

val workbenchModel: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-model" % workbenchModelV
val excludeWorkbenchModel = ExclusionRule(organization = "org.broadinstitute.dsde.workbench", name = "workbench-model_" + scalaV)
Expand All @@ -34,15 +34,13 @@ object Dependencies {
"com.google.api-client" % "google-api-client" % "1.22.0" excludeAll (
ExclusionRule("com.google.guava", "guava-jdk5"),
ExclusionRule("org.apache.httpcomponents", "httpclient")),
"org.webjars" % "swagger-ui" % "2.2.5",
"com.typesafe.akka" %% "akka-http-core" % akkaHttpV,
"com.typesafe.akka" %% "akka-stream-testkit" % akkaV,
"com.typesafe.akka" %% "akka-http" % akkaHttpV,
"com.typesafe.akka" %% "akka-testkit" % akkaV % "test",
"com.typesafe.akka" %% "akka-slf4j" % akkaV,
"org.specs2" %% "specs2-core" % "3.8.6" % "test",
"org.scalatest" %% "scalatest" % "3.0.1" % "test",
"org.seleniumhq.selenium" % "selenium-java" % "3.8.1" % "test",
"org.scalatest" %% "scalatest" % "3.0.5" % "test",
"org.scalacheck" %% "scalacheck" % "1.14.0" % "test",
"com.typesafe.scala-logging" %% "scala-logging" % "3.5.0",

workbenchServiceTest,
Expand Down
6 changes: 4 additions & 2 deletions automation/project/Settings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ object Settings {
"-deprecation",
"-feature",
"-encoding", "utf8",
"-language:postfixOps",
"-target:jvm-1.8",
"-Xmax-classfile-name", "100"
"-Xmax-classfile-name", "100",
"-Ypartial-unification" // Enable partial unification in type constructor inference
)

// test parameters explanation:
Expand All @@ -39,7 +41,7 @@ object Settings {
//common settings for all sbt subprojects
val commonSettings =
commonBuildSettings ++ testSettings ++ List(
organization := "org.broadinstitute.d sde.firecloud",
organization := "org.broadinstitute.dsde.firecloud",
scalaVersion := "2.12.7",
resolvers ++= commonResolvers,
scalacOptions ++= commonCompilerSettings
Expand Down
2 changes: 1 addition & 1 deletion automation/project/build.properties
Original file line number Diff line number Diff line change
@@ -1 +1 @@
sbt.version = 0.13.15
sbt.version = 1.2.6
4 changes: 3 additions & 1 deletion automation/project/plugins.sbt
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
logLevel := Level.Warn
addSbtPlugin("net.virtual-void" % "sbt-dependency-graph" % "0.9.2")

addSbtPlugin("ch.epfl.scala" % "sbt-bloop" % "1.0.0")
8 changes: 8 additions & 0 deletions automation/render-local-env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ render_configs() {
original_dir=$WORKING_DIR
cd ../..
docker pull broadinstitute/dsde-toolbox:dev

docker run -it --rm -e VAULT_TOKEN=${VAULT_TOKEN} \
-e ENVIRONMENT=${ENV} -e ROOT_DIR=${WORKING_DIR} -v $PWD/firecloud-automated-testing/configs:/input -v $PWD/$SCRIPT_ROOT:/output \
-e OUT_PATH=/output/src/test/resources -e INPUT_PATH=/input -e LOCAL_UI=$LOCAL_UI -e FC_INSTANCE=$FC_INSTANCE \
Expand All @@ -72,6 +73,13 @@ render_configs() {
-e ENVIRONMENT=${ENV} -e ROOT_DIR=${WORKING_DIR} -v $PWD/firecloud-automated-testing/configs/$SERVICE:/input -v $PWD/$SCRIPT_ROOT:/output \
-e OUT_PATH=/output/src/test/resources -e INPUT_PATH=/input -e LOCAL_UI=$LOCAL_UI -e FC_INSTANCE=$FC_INSTANCE \
broadinstitute/dsde-toolbox:dev render-templates.sh

# copy files under extra
docker run -it --rm -e VAULT_TOKEN=${VAULT_TOKEN} \
-e ENVIRONMENT=${ENV} -e ROOT_DIR=${WORKING_DIR} -v $PWD/firecloud-automated-testing/configs/$SERVICE/extra:/input/extra -v $PWD/$SCRIPT_ROOT:/output \
-e OUT_PATH=/output/src/test/resources/extra -e INPUT_PATH=/input/extra -e LOCAL_UI=$LOCAL_UI -e FC_INSTANCE=$FC_INSTANCE \
broadinstitute/dsde-toolbox:dev render-templates.sh

cd $original_dir
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.broadinstitute.dsde.workbench.test

import org.broadinstitute.dsde.workbench.google.{CollectionName, Document}
import org.broadinstitute.dsde.workbench.google.util.LockPath
import org.scalacheck.Gen
import scala.concurrent.duration._

object Generators {
val genLockPath: Gen[LockPath] = for {
collectionName <- Gen.alphaStr.map(x => CollectionName(s"test$x"))
document <- Gen.alphaStr.map(x => Document(s"test$x"))
} yield LockPath(collectionName, document, 5 seconds)
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package org.broadinstitute.dsde.test
package org.broadinstitute.dsde.workbench.test

import org.broadinstitute.dsde.workbench.config.CommonConfig

object SamConfig extends CommonConfig {
// from common: qaEmail, pathToQAPem
object GCS extends CommonGCS {
val appsDomain = gcsConfig.getString("appsDomain")
val pathToSamTestFirestoreAccountPath = gcsConfig.getString("firestoreAccountPath")
}
}
Original file line number Diff line number Diff line change
@@ -1,27 +1,26 @@
package org.broadinstitute.dsde.test.api
package org.broadinstitute.dsde.workbench.test.api


import java.util.UUID

import akka.actor.ActorSystem
import akka.testkit.TestKitBase
import org.broadinstitute.dsde.test.SamConfig
import org.broadinstitute.dsde.workbench.service._
import org.broadinstitute.dsde.workbench.service.SamModel._
import org.broadinstitute.dsde.workbench.auth.{AuthToken, ServiceAccountAuthTokenFromJson, ServiceAccountAuthTokenFromPem}
import org.broadinstitute.dsde.workbench.config.{Credentials, ServiceTestConfig, UserPool}
import org.broadinstitute.dsde.workbench.model._
import org.broadinstitute.dsde.workbench.config.{Credentials, UserPool}
import org.broadinstitute.dsde.workbench.dao.Google.{googleDirectoryDAO, googleIamDAO}
import org.broadinstitute.dsde.workbench.fixture.BillingFixtures
import org.broadinstitute.dsde.workbench.service.test.CleanUp
import org.broadinstitute.dsde.workbench.model._
import org.broadinstitute.dsde.workbench.model.google.{GoogleProject, ServiceAccountName}
import org.broadinstitute.dsde.workbench.service.SamModel._
import org.broadinstitute.dsde.workbench.service.test.CleanUp
import org.broadinstitute.dsde.workbench.service.{Orchestration, Sam, Thurloe, _}
import org.broadinstitute.dsde.workbench.test.SamConfig
import org.scalatest.concurrent.{Eventually, ScalaFutures}
import org.scalatest.time.{Seconds, Span}
import org.scalatest.{FreeSpec, Matchers}

import scala.concurrent.{Await, ExecutionContext, ExecutionContextExecutor}
import scala.concurrent.duration.Duration
import scala.concurrent.duration._
import scala.concurrent.Await
import scala.concurrent.duration.{Duration, _}

class SamApiSpec extends FreeSpec with BillingFixtures with Matchers with ScalaFutures with CleanUp with Eventually with TestKitBase {
implicit override val patienceConfig: PatienceConfig = PatienceConfig(timeout = scaled(Span(5, Seconds)))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
package org.broadinstitute.dsde.workbench.test.util

import java.util.concurrent.TimeUnit

import cats.effect.IO
import cats.kernel.Eq
import cats.implicits._
import org.broadinstitute.dsde.workbench.google.GoogleFirestoreOpsInterpreters
import org.broadinstitute.dsde.workbench.google.util.{
DistributedLock,
DistributedLockConfig
}
import org.broadinstitute.dsde.workbench.model.WorkbenchException
import org.scalatest.{AsyncFlatSpec, Matchers}
import org.broadinstitute.dsde.workbench.test.Generators.genLockPath
import org.broadinstitute.dsde.workbench.test.SamConfig.GCS

import scala.collection.JavaConverters._
import scala.concurrent.duration._

class DistributedLockSpec extends AsyncFlatSpec with Matchers {
implicit val cs = IO.contextShift(scala.concurrent.ExecutionContext.global)
implicit val timer = IO.timer(scala.concurrent.ExecutionContext.global)
implicit val eqWorkbenchException: Eq[WorkbenchException] =
(x: WorkbenchException, y: WorkbenchException) =>
x.getMessage == y.getMessage

val config = DistributedLockConfig(5 seconds, 5)

val lockResource: cats.effect.Resource[IO, DistributedLock[IO]] = for {
db <- GoogleFirestoreOpsInterpreters.firestore[IO](
GCS.pathToSamTestFirestoreAccountPath
)
} yield {
val googeFireStoreOps = GoogleFirestoreOpsInterpreters.ioFirestore(db)
DistributedLock("samServiceTest", config, googeFireStoreOps)
}

"acquireLock" should "succeed if a lock can be retrieved" in {
val lockPath = genLockPath.sample.get
val res = lockResource.use { lock => lock.acquireLock(lockPath)}

res.attempt.map(r => r.isRight shouldBe true).unsafeToFuture()
}

it should "fail if there's same lock has already been set within 30 seconds" in {
val lockPath = genLockPath.sample.get
val res = lockResource.use { lock =>
for {
_ <- lock.acquireLock(lockPath)
_ <- IO.sleep(2 seconds)
failed <- lock.acquireLock(lockPath).attempt
} yield {
failed.swap.toOption.get.asInstanceOf[WorkbenchException].getMessage shouldBe(s"can't get lock: $lockPath")
}
}
res.unsafeToFuture()
}

"releaseLock" should "remove lockPath" in {
val lockPath = genLockPath.sample.get
val res = lockResource.use { dl =>
for {
lock <- dl.acquireLock(lockPath)
_ <- dl.releaseLock(lockPath)
released <- dl.googleFirestoreOps
.get(lockPath.collectionName, lockPath.document)
} yield {
released.getData.asScala shouldBe (null)
}
}

res.unsafeToFuture()
}

"withLock" should "eventually get a lock with max retry" in {
val lockPath = genLockPath.sample.get
val collectionNameWithPrefix = lockPath.collectionName.copy(asString = "samServiceTest-" + lockPath.collectionName.asString)
val lockPathWithPrefix = lockPath.copy(collectionName = collectionNameWithPrefix, expiresIn = 7 seconds) //fix expiresIn so that we won't be waiting for too long in the unit test
val res = lockResource.use { lock =>
for {
current <- timer.clock.realTime(TimeUnit.MILLISECONDS)
_ <- lock.acquireLock(lockPathWithPrefix)
_ <- IO.sleep(2 seconds)
acquireTime <- lock.withLock(lockPath).use { _ =>
timer.clock
.realTime(TimeUnit.MILLISECONDS)
}
} yield {
acquireTime - current should be > lockPathWithPrefix.expiresIn.toMillis
}
}

res.unsafeToFuture()
}

it should "release the lock after it's used" in {
val lockPath = genLockPath.sample.get.copy(expiresIn = 5 seconds) //fix expiresIn so that we won't be waiting for too long in the unit test
val collectionNameWithPrefix = lockPath.collectionName.copy(asString = "samServiceTest-" + lockPath.collectionName.asString)
val lockPathWithPrefix = lockPath.copy(collectionName = collectionNameWithPrefix, expiresIn = 7 seconds) //fix expiresIn so that we won't be waiting for too long in the unit test

val res = lockResource.use { lock =>
for {
currentTime <- timer.clock.monotonic(DistributedLock.EXPIRESATTIMEUNIT)
lockData <- lock.withLock(lockPath).use{
_ =>
lock.googleFirestoreOps.get(collectionNameWithPrefix, lockPathWithPrefix.document)
}
released <- lock.googleFirestoreOps
.get(collectionNameWithPrefix, lockPathWithPrefix.document) //withLock will prefix the lockPrefix in the path
} yield {
// validate we actually set and release the same lockPath
lockData.getLong(DistributedLock.EXPIRESAT).longValue() should be >= (currentTime + lockPath.expiresIn.toMillis)
released.getData.asScala shouldBe (null)
}
}

res.unsafeToFuture()
}

it should "fail to get a lock if max retry is reached" in {
val lockPath = genLockPath.sample.get
val collectionNameWithPrefix = lockPath.collectionName.copy(asString = "samServiceTest-" + lockPath.collectionName.asString)
val lockPathWithPrefix = lockPath.copy(collectionName = collectionNameWithPrefix, expiresIn = 10 seconds) //fix expiresIn so that we won't be waiting for too long in the unit test

val config = DistributedLockConfig(1 seconds, 2)
val lockResource: cats.effect.Resource[IO, DistributedLock[IO]] = for {
db <- GoogleFirestoreOpsInterpreters.firestore[IO](
GCS.pathToSamTestFirestoreAccountPath
)
} yield {
val googeFireStoreOps = GoogleFirestoreOpsInterpreters.ioFirestore(db)
DistributedLock("samServiceTest", config, googeFireStoreOps)
}

val res = lockResource.use { lock =>
for {
current <- timer.clock.realTime(TimeUnit.MILLISECONDS)
_ <- lock.acquireLock(lockPathWithPrefix)
failed <- lock
.withLock(lockPath)
.use(_ => IO.unit)
.attempt //this will fail to aquire lock
endTime <- timer.clock.realTime(TimeUnit.MILLISECONDS)
} yield {
failed.swap.toOption.get.asInstanceOf[WorkbenchException].getMessage should startWith (s"Reached max retry:")
// validate we actually retried certain amount of time
endTime - current should be > (config.maxRetry * config.retryInterval.toMillis)
}
}

res.unsafeToFuture()
}
}
1 change: 1 addition & 0 deletions jenkins/jenkins_build.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#!/bin/bash


set -eux
SVCACCT_FILE="dspci-wb-gcr-service-account.json"
GCR_SVCACCT_VAULT="secret/dsde/dsp-techops/common/$SVCACCT_FILE"
Expand Down
Loading

0 comments on commit b8870b9

Please sign in to comment.