Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add back support for nomad #221

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

goedelsoup
Copy link
Contributor

No description provided.

Copy link
Member

@timperrett timperrett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting this @goedelsoup! Added a few comments for your review

makeVault(List(vault.policies.policyName(sn, ns)), "restart", "")

def pulsar(img: Image, dc: Datacenter, ns: NamespaceName, unit: UnitDef, plan: Plan, sn: StackName): Partial[Id] =
canopus(img, dc, ns, unit, plan, sn)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look right. See the workflow matrix: https://getnelson.io/getting-started/blueprints.html#workflows - where's the Vault support?

val healthCheckPath = "health_check_path"
val healthCheckPort = "health_check_port"
val healthCheckInterval = "health_check_interval"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


val vaultPolicies = "vault_policies"
val vaultChangeMode = "vault_change_mode"
val vaultChangeSignal = "vault_change_signal"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this? Not familiar with it :-)

@@ -67,7 +67,8 @@ final class KubernetesShell(
error.stderr.exists(_.startsWith("Error from server (NotFound)"))

def launch(image: Image, dc: Datacenter, ns: NamespaceName, unit: UnitDef, version: Version, plan: Plan, hash: String): IO[String] = {
val env = Render.makeEnv(image, dc, ns, unit, version, plan, hash)
val sn = StackName(unit.name, version, hash)
val env = Render.canopus(image, dc, ns, unit, plan, sn)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look right - its coupling directly to the workflow?

.traverse {
case Left(_) => IO.raiseError(new IllegalArgumentException("Internal error occured: un-hydrated blueprint passed to scheduler!"))
case Right(bp) => bp.template.pure[IO]
} >>=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: prefer the textual functions instead of the symbols

// NOTE(timperrett): for some reason, this no longer works for consul
// despite trying a range of different things.
// .withReadyChecker(DockerReadyChecker.LogLineContains("Consul agent running!"))
.withReadyChecker(DockerReadyChecker
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work again? 🎉

Cory Parent added 5 commits March 28, 2019 14:41
* master:
  Expose unit name as env var (getnelson#225)
  Check the default elb scheme takes effect (getnelson#224)
  fix typos (getnelson#222)
  Use multiple branches
  Fix bug where the 'only' filter of DC targets was not being parsed (getnelson#220)
  LB name error message should be dynamic length (getnelson#217)
  Have the ability to specify an ELB as internal (getnelson#216)
  Fix LB naming to match the validators expectations (getnelson#215)
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@c74974d). Click here to learn what that means.
The diff coverage is 14.63%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master    #221   +/-   ##
========================================
  Coverage          ?   56.4%           
========================================
  Files             ?     133           
  Lines             ?    4340           
  Branches          ?     111           
========================================
  Hits              ?    2448           
  Misses            ?    1892           
  Partials          ?       0
Impacted Files Coverage Δ
core/src/main/scala/Exceptions.scala 42.5% <ø> (ø)
core/src/main/scala/blueprint/Render.scala 0% <ø> (ø)
core/src/main/scala/docker/Docker.scala 28.33% <ø> (ø)
core/src/main/scala/Datacenter.scala 80% <ø> (ø)
...ore/src/main/scala/scheduler/KubernetesShell.scala 5.12% <0%> (ø)
core/src/main/scala/scheduler/NomadHttp.scala 0% <0%> (ø)
...e/src/main/scala/blueprint/DefaultBlueprints.scala 0% <0%> (ø)
core/src/main/scala/Config.scala 80.07% <60%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c74974d...2f59a63. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants