- Notifications
You must be signed in to change notification settings - Fork927
chore(examples): update kubernetes devcontainer template with envbuilder provider#14267
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
15af3ce
to71c8e39
Compare@@ -222,7 +257,7 @@ resource "kubernetes_deployment" "main" { | |||
container { | |||
name = "dev" | |||
image = local.devcontainer_builder_image | |||
image =var.cache_repo == "" ?local.devcontainer_builder_image : envbuilder_cached_image.cached.0.image | |||
image_pull_policy = "Always" | |||
security_context {} | |||
env { |
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.
review: leaving as-is untilcoder/terraform-provider-envbuilder#31 is resolved.
There may be some additional Terraform jiggery pokery requried to convertlocal.envbuilder_env
into a block list.
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.
A for-loop would be nice here to set all the envs, wdyt?
Right now it can come fromlocals.envbuilder_env
, but eventually fromenvbuilder_cached_image.cached.0.env
.
env {for_each=locals.envbuilder_envname=each.keyvalue=each.value}
Not sure iftomap
is required (for_each = tomap(locals.envbuilder_env)
), probably not?
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.
Yeah, but then that needs to be referenced in thekubernetes_pod
spec below. I think that may need adynamic{}
block?
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.
Possibly, my tf foo is not strong enough 😄
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.
I'll address this in a follow-up. The current way the envbuilder provider outputs the computed env isn't really conducive to this scenario right now anyway.
Uh oh!
There was an error while loading.Please reload this page.
type = string | ||
} | ||
variable "insecure_cache_repo" { |
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.
Nice solution 👍🏻
@@ -222,7 +257,7 @@ resource "kubernetes_deployment" "main" { | |||
container { | |||
name = "dev" | |||
image = local.devcontainer_builder_image | |||
image =var.cache_repo == "" ?local.devcontainer_builder_image : envbuilder_cached_image.cached.0.image | |||
image_pull_policy = "Always" | |||
security_context {} | |||
env { |
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.
A for-loop would be nice here to set all the envs, wdyt?
Right now it can come fromlocals.envbuilder_env
, but eventually fromenvbuilder_cached_image.cached.0.env
.
env {for_each=locals.envbuilder_envname=each.keyvalue=each.value}
Not sure iftomap
is required (for_each = tomap(locals.envbuilder_env)
), probably not?
71c8e39
to821ea40
CompareCo-authored-by: Mathias Fredriksson <mafredri@gmail.com>
91a74f0
intomainUh oh!
There was an error while loading.Please reload this page.
See also:#14199
Updates the devcontainer-kubernetes template to use the coder/envbuilder provider.
Caching is completely optional here.