patch: update mongos TF#242
Conversation
patriciareinoso
added
the
enhancement
There was a problem hiding this comment.
I think this is missing the constraints for the architecture ?
Something like:
variable "constraints" {
description = "String listing constraints for this application"
type = string
default = "arch=amd64"
}
There was a problem hiding this comment.
we cannot set constraints to a subordinate charm. I had to remove it from a previous commit
│ Error: Client Error
│
│ with module.mongos.juju_application.mongos,
│ on ../main.tf line 4, in resource "juju_application" "mongos":
│ 4: resource "juju_application" "mongos" {
│
│ Unable to create application, got error: subordinate application must be
│ deployed without constraints
╵
There was a problem hiding this comment.
yes, but we can define it in the variables for unifying the UX, and we just don't use it in the main.tf
Mehdi-Bendriss
left a comment
Mehdi-Bendriss
left a comment
There was a problem hiding this comment.
Thanks Patricia, looks good, I have a few small comments
| lifecycle { | ||
| precondition { | ||
| condition = length(var.machines) == 0 || length(var.machines) == var.units | ||
| error_message = "Machine count does not match unit count" | ||
| } | ||
| precondition { | ||
| condition = length(var.storage) == 0 || lookup(var.storage, "count", 0) <= 1 | ||
| error_message = "Only one storage is supported" | ||
| } |
There was a problem hiding this comment.
2 points here:
- why was this validation removed?
- seems like we're missing
storage_directivesandplacement?
There was a problem hiding this comment.
this section defines the mandatory input variables. Neither machines, storage_directives are mandatory
- I meant to remove the machines variable since it is subordinate charm and to me it does not make sense to define the machine
- Mongos does not define storage
- I think
placementis a legacy parameter. I do not see it in the app definition
| description = "Object representing the deployed mongos application." | ||
| value = juju_application.mongos | ||
| } | ||
|
|
There was a problem hiding this comment.
it seems like we're missing offers and provides outputs, should they be omitted or declared but set to None or empty object?
There was a problem hiding this comment.
those are optional parameters, I decided to omit them
There was a problem hiding this comment.
The problem is on consistency, as the outputs are mostly gonna be used by other terraform modules or teams that may not have the mongodb knowledge about why we're omitting.
Also, consistency with the k8s charm (which will have a provides: mongos_proxy), and since it is a standalone charm, nothing stops users from deploying it in a separate model.
There was a problem hiding this comment.
yes, but we can define it in the variables for unifying the UX, and we just don't use it in the main.tf
3 participants
Description
This PR update the terraform modules.
mongos is a charm module. Variables, outputs and file name were changed according to CC008 and DA219
~> 2.0Example
Modules are still under development so for now no version release