-
Notifications
You must be signed in to change notification settings - Fork 14
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
Simplify the docs for installing via terraform #948
base: main
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.
LGTM, minor nit
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 the improvement of my docs! Some questions here:
variable "k8s" { | ||
description = "K8s deployment shared configuration" | ||
channel = string | ||
default = "1.xx/stable" |
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 don't quite like this variable declaration here. First it should be in an input file and second this should be closer to where it is set >> sub-module.
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.
so, this channel
likely should match both k8s
and k8s-worker
. We can eliminate this from the example if you think that's a good idea and have folks go read the individual tf modules to learn what variables are optional.
i was trying to use this variable as var.k8s.channel
for both modules. Did i do this incorrectly?
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.
another thing, i wish i could use {{channel}}
here
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.
@louiseschmidtgen did i close the gap here or are we still an open issue. I'm unsure how to resolve
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.
From a docs perspective this looks fine. I'll let Louise give the final approval as the terraform expert
cd3f75b
to
3355e91
Compare
Overview
Ensure that the user provides a deployment channel for the charms rather than rely on a default in the charm modules.
Details
terraform plan
before running apply