Skip to content
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

feat(safer-cluster): add support for observability_metrics and -_relay #2291

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

foosinn
Copy link
Contributor

@foosinn foosinn commented Feb 21, 2025

This adds the missing observability options to the safer cluster variants. It seems to work without issues.

Would love to see this upstreamed.

Thanks!

@foosinn foosinn requested review from apeabody, ericyz and a team as code owners February 21, 2025 13:46
Copy link
Collaborator

@apeabody apeabody left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @foosinn!

Can you please update the template, so this is applied to all relevant sub-modules: https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/main/CONTRIBUTING.md#templating

@foosinn
Copy link
Contributor Author

foosinn commented Feb 25, 2025

Hey @apeabody,

maybe I'm blind here, but I only was able to find the templates for the normal and the safer cluster.

Can you please point me to a file?

Thanks!

@apeabody
Copy link
Collaborator

Hey @apeabody,

maybe I'm blind here, but I only was able to find the templates for the normal and the safer cluster.

Can you please point me to a file?

Thanks!

Sure, here is an example from the linter.

Checking submodule's files generation
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/safer-cluster/README.md /tmp/tmp.KmfyghFSvl/workspace/modules/safer-cluster/README.md
255,256d254
< | monitoring\_enable\_observability\_metrics | Whether or not the advanced datapath metrics are enabled. | `bool` | `false` | no |
< | monitoring\_enable\_observability\_relay | Whether or not the advanced datapath relay is enabled. | `bool` | `false` | no |
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/safer-cluster/main.tf /tmp/tmp.KmfyghFSvl/workspace/modules/safer-cluster/main.tf
106,109c106,107
<   monitoring_enable_managed_prometheus    = var.monitoring_enable_managed_prometheus
<   monitoring_enable_observability_metrics = var.monitoring_enable_observability_metrics
<   monitoring_enable_observability_relay   = var.monitoring_enable_observability_relay
<   monitoring_enabled_components           = var.monitoring_enabled_components
---
>   monitoring_enable_managed_prometheus = var.monitoring_enable_managed_prometheus
>   monitoring_enabled_components        = var.monitoring_enabled_components
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/safer-cluster/variables.tf /tmp/tmp.KmfyghFSvl/workspace/modules/safer-cluster/variables.tf
283,294d282
< variable "monitoring_enable_observability_metrics" {
<   type        = bool
<   description = "Whether or not the advanced datapath metrics are enabled."
<   default     = false
< }
< 
< variable "monitoring_enable_observability_relay" {
<   type        = bool
<   description = "Whether or not the advanced datapath relay is enabled."
<   default     = false
< }
< 
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/safer-cluster-update-variant/README.md /tmp/tmp.KmfyghFSvl/workspace/modules/safer-cluster-update-variant/README.md
255,256d254
< | monitoring\_enable\_observability\_metrics | Whether or not the advanced datapath metrics are enabled. | `bool` | `false` | no |
< | monitoring\_enable\_observability\_relay | Whether or not the advanced datapath relay is enabled. | `bool` | `false` | no |
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/safer-cluster-update-variant/main.tf /tmp/tmp.KmfyghFSvl/workspace/modules/safer-cluster-update-variant/main.tf
106,109c106,107
<   monitoring_enable_managed_prometheus    = var.monitoring_enable_managed_prometheus
<   monitoring_enable_observability_metrics = var.monitoring_enable_observability_metrics
<   monitoring_enable_observability_relay   = var.monitoring_enable_observability_relay
<   monitoring_enabled_components           = var.monitoring_enabled_components
---
>   monitoring_enable_managed_prometheus = var.monitoring_enable_managed_prometheus
>   monitoring_enabled_components        = var.monitoring_enabled_components
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/safer-cluster-update-variant/variables.tf /tmp/tmp.KmfyghFSvl/workspace/modules/safer-cluster-update-variant/variables.tf
283,294d282
< variable "monitoring_enable_observability_metrics" {
<   type        = bool
<   description = "Whether or not the advanced datapath metrics are enabled."
<   default     = false
< }
< 
< variable "monitoring_enable_observability_relay" {
<   type        = bool
<   description = "Whether or not the advanced datapath relay is enabled."
<   default     = false
< }
< 
Error: submodule's files generation has not been run, please run the
'make build' command and commit changes

So I would start by making your first change here: https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/main/autogen/safer-cluster/main.tf.tmpl#L110, and then run make build to generate the sub-modules and update their documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants