-
Notifications
You must be signed in to change notification settings - Fork 40
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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) |
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the docs for this? https://getnelson.io/documentation/blueprints.html
|
||
val vaultPolicies = "vault_policies" | ||
val vaultChangeMode = "vault_change_mode" | ||
val vaultChangeSignal = "vault_change_signal" |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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] | ||
} >>= |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work again? 🎉
* 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 Report
@@ Coverage Diff @@
## master #221 +/- ##
========================================
Coverage ? 56.4%
========================================
Files ? 133
Lines ? 4340
Branches ? 111
========================================
Hits ? 2448
Misses ? 1892
Partials ? 0
Continue to review full report at Codecov.
|
No description provided.