Use Ecosystem-Core for Terraform-Ces-Module #122
Conversation
There was a problem hiding this comment.
While it seems to work, I see some space for improvement, especially in the case of default values terms of the ecosystem-core values.yaml.
Maybe I would also create sub-modules within ces-module to improve readability and maintainability:
- ces-preparation
- ces-core
- ces-blueprint
This matches also the phases in ArgoCD.
There was a problem hiding this comment.
Is this something we need in the ces-module?
| name: blueprint-ces-module | ||
| namespace: ${ces_namespace} | ||
| spec: | ||
| displayName: "Blueprint Sample CES-Module" |
There was a problem hiding this comment.
| displayName: "Blueprint Sample CES-Module" | |
| displayName: "Blueprint Terraform CES-Module" |
There was a problem hiding this comment.
should follow the naming conventions: values.yaml.tftpl
terraform/ces-module/variables.tf
Outdated
| variable "ecosystem_core_default_config_image" { | ||
| description = "The Image:Version of the ecosystem_core default config. Optional with version like cloudogu/ecosystem-core-default-config:0.1.0" | ||
| type = string | ||
| default = "cloudogu/ecosystem-core-default-config:0.2.2" | ||
| } |
There was a problem hiding this comment.
Do we need to override the default config image at all?
| env: | ||
| logLevel: info | ||
| resourceLimits: | ||
| memory: 105M | ||
| resourceRequests: | ||
| cpu: 15m | ||
| memory: 105M | ||
| networkPolicies: | ||
| enabled: true |
There was a problem hiding this comment.
Why do we need to override default values with default values?
terraform/ces-module/variables.tf
Outdated
| version = string | ||
| helmNamespace = optional(string) | ||
| disabled = optional(bool, false) | ||
| valuesObject = optional(any, null) |
There was a problem hiding this comment.
the zero value of optional should already be null
terraform/ces-module/variables.tf
Outdated
| } | ||
|
|
||
| variable "externalIP" { | ||
| description = "Contains the external IP, my overwrite the loadbalancer external ip, defaults to empty -> so the loadbalancer ip will not be patched" |
There was a problem hiding this comment.
| description = "Contains the external IP, my overwrite the loadbalancer external ip, defaults to empty -> so the loadbalancer ip will not be patched" | |
| description = "Contains the external IP, may overwrite the loadbalancer external ip, defaults to empty -> so the loadbalancer ip will not be patched" |
There was a problem hiding this comment.
Do we still need resource_patches? I thought this has been replaced?
No description provided.