Skip to content

Support Managed Prometheus #1298

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.

Sign up for GitHub

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

Closed
wants to merge 13 commits into from
Closed

Support Managed Prometheus #1298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
e339502
Support Managed Prometheus
abousias Jun 20, 2022
abd5174
Fix by setting the block
abousias Jun 20, 2022
e44ab23
Merge branch 'master' into master
abousias Jun 21, 2022
6969b0c
Nested managed_prometheus dynamic as review suggested
abousias Jun 21, 2022
7f2ecf5
Ignore vscode files as in PR #1294
abousias Jun 21, 2022
c5170bb
managed_prometheus with a static block
abousias Jun 22, 2022
64cdf62
Revert "managed_prometheus with a static block"
abousias Jun 22, 2022
284e7db
Revert "Nested managed_prometheus dynamic as review suggested"
abousias Jun 22, 2022
b17ccb1
Merge branch 'master' into master
abousias Jun 22, 2022
e0b4718
Back to having a nested block with fix, thanks @jroiseux
abousias Jun 22, 2022
535949e
Merge branch 'master' into master
abousias Jun 22, 2022
32e3440
Merge branch 'master' into master
abousias Jun 23, 2022
9709ff2
Revert "Back to having a nested block with fix, thanks @jroiseux"
abousias Jun 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ Session.vim
# IntelliJ IDEA files:
.idea/

# Visual Studio Code files:
.vscode/

### https://raw.github.com/github/gitignore/90f149de451a5433aebd94d02d11b0e28843a1af/Terraform.gitignore

# Local .terraform directories
Expand Down
9 changes: 9 additions & 0 deletions autogen/main/cluster.tf.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,15 @@ resource "google_container_cluster" "primary" {
enable_components = var.monitoring_enabled_components
}
}
dynamic "monitoring_config" {
Copy link
Member

Choose a reason for hiding this comment

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

There is an existing dynamic monitoring_config, I think we can nest managed_prometheus as a dynamic under that.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for quick response and the review comment @bharathkkb! I kept it separate to have a separate for_each check as in one case we have var.monitoring_enabled_components=[] but we still want managed prometheus enabled. I didn't think of nesting with another dynamic! I will try that!

Copy link
Author

Choose a reason for hiding this comment

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

@bharathkkb the nested dynamic, the way I have it now, doesn't seem to be working as expected. With the var.enable_managed_prometheus set to true, the service didn't get enabled and the tfstate is like this:

...
"monitoring_config": [
  {
    "enable_components": [
      "SYSTEM_COMPONENTS"
    ],
    "managed_prometheus": []
  }
],
...

I am looking into it.

Copy link
Author

@abousias abousias Jun 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Interestingly switching to a static block:

https://github.com/abousias/terraform-google-kubernetes-engine/blob/c5170bb58cab6706377baf24e58cadc28e685205/autogen/main/cluster.tf.tmpl#L91-L100

It gives an api error:

image

I am reverting to the initial as I don't have a working solution at the moment.

Copy link
Author

Choose a reason for hiding this comment

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

I am having the error, with the reverted code too. The issue didn't appear because in my test I was applying the change to an existing cluster. Now I am getting this with a brand new cluster.

https://github.com/abousias/terraform-google-kubernetes-engine/blob/c5170bb58cab6706377baf24e58cadc28e685205/autogen/main/cluster.tf.tmpl#L81

I am looking into it.

Copy link
Author

Choose a reason for hiding this comment

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

Hey @jroiseux I was about to check exactly the same conditional 😄! I will take a look and copy! Many thanks for the help! Nice to know that it will work 😉!

Copy link
Author

Choose a reason for hiding this comment

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

@bharathkkb ready for re-review. It works fine now.

Copy link
Author

Choose a reason for hiding this comment

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

⚠️ I discovered that the current version is trying to disable SYSTEM_COMPONENTS, here is the plan against an existing cluster:

image

Whereas the separate block one leaves the enabled_components untouched:

image

This is because the dynamic is "forced" to produce the block but the var.monitoring_enabled_components is []. My existing cluster is also based on the main module instead of the beta ones. I am checking if that is an issue for me.

@jroiseux this is most probably happening with your version, isn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

@abousias Yes, it happens on my version too.

This is what the statefile looks like when I use the current module for "beta-private-cluster" (without the changes from this PR) without setting var.monitoring_enabled_components :

Screenshot from 2022-06-23 09-16-34

I think it might be an issue with the default value. As enabled_components seems to end up as ["SYSTEM_COMPONENTS"], even though the default is [].

Copy link
Author

Choose a reason for hiding this comment

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

@bharathkkb I am reverting back to the original implementation, leaving TF to merge the blocks in one well after the evaluation of the the for_eachs. As @jroiseux also points out, the other option would be to change the default value of var.monitoring_enabled_components to something like [ "SYSTEM_COMPONENTS" ] which I don't know if it is a good idea.

for_each = var.enable_managed_prometheus ? [1] : []

content {
managed_prometheus {
enabled = var.enable_managed_prometheus
}
}
}
{% else %}
logging_service = var.logging_service
monitoring_service = var.monitoring_service
Expand Down
6 changes: 6 additions & 0 deletions autogen/main/variables.tf.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,12 @@ variable "monitoring_enabled_components" {
default = []
}

variable "enable_managed_prometheus" {
type = bool
description = "(Beta) Whether or not the managed collection is enabled."
default = false
}

variable "istio" {
description = "(Beta) Enable Istio addon"
default = false
Expand Down
2 changes: 1 addition & 1 deletion autogen/main/versions.tf.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ terraform {
required_providers {
google-beta = {
source = "hashicorp/google-beta"
version = ">= 4.10.0, < 5.0"
version = ">= 4.25.0, < 5.0"
}
kubernetes = {
source = "hashicorp/kubernetes"
Expand Down
2 changes: 1 addition & 1 deletion modules/beta-autopilot-private-cluster/versions.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ terraform {
required_providers {
google-beta = {
source = "hashicorp/google-beta"
version = ">= 4.10.0, < 5.0"
version = ">= 4.25.0, < 5.0"
}
kubernetes = {
source = "hashicorp/kubernetes"
Expand Down
2 changes: 1 addition & 1 deletion modules/beta-autopilot-public-cluster/versions.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ terraform {
required_providers {
google-beta = {
source = "hashicorp/google-beta"
version = ">= 4.10.0, < 5.0"
version = ">= 4.25.0, < 5.0"
}
kubernetes = {
source = "hashicorp/kubernetes"
Expand Down
1 change: 1 addition & 0 deletions modules/beta-private-cluster-update-variant/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ Then perform the following commands on the root folder:
| enable\_intranode\_visibility | Whether Intra-node visibility is enabled for this cluster. This makes same node pod to pod traffic visible for VPC network | `bool` | `false` | no |
| enable\_kubernetes\_alpha | Whether to enable Kubernetes Alpha features for this cluster. Note that when this option is enabled, the cluster cannot be upgraded and will be automatically deleted after 30 days. | `bool` | `false` | no |
| enable\_l4\_ilb\_subsetting | Enable L4 ILB Subsetting on the cluster | `bool` | `false` | no |
| enable\_managed\_prometheus | (Beta) Whether or not the managed collection is enabled. | `bool` | `false` | no |
| enable\_network\_egress\_export | Whether to enable network egress metering for this cluster. If enabled, a daemonset will be created in the cluster to meter network egress traffic. | `bool` | `false` | no |
| enable\_pod\_security\_policy | enabled - Enable the PodSecurityPolicy controller for this cluster. If enabled, pods must be valid under a PodSecurityPolicy to be created. | `bool` | `false` | no |
| enable\_private\_endpoint | (Beta) Whether the master's internal IP address is used as the cluster endpoint | `bool` | `false` | no |
Expand Down
9 changes: 9 additions & 0 deletions modules/beta-private-cluster-update-variant/cluster.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,15 @@ resource "google_container_cluster" "primary" {
enable_components = var.monitoring_enabled_components
}
}
dynamic "monitoring_config" {
for_each = var.enable_managed_prometheus ? [1] : []

content {
managed_prometheus {
enabled = var.enable_managed_prometheus
}
}
}
cluster_autoscaling {
enabled = var.cluster_autoscaling.enabled
dynamic "auto_provisioning_defaults" {
Expand Down
6 changes: 6 additions & 0 deletions modules/beta-private-cluster-update-variant/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,12 @@ variable "monitoring_enabled_components" {
default = []
}

variable "enable_managed_prometheus" {
type = bool
description = "(Beta) Whether or not the managed collection is enabled."
default = false
}

variable "istio" {
description = "(Beta) Enable Istio addon"
default = false
Expand Down
2 changes: 1 addition & 1 deletion modules/beta-private-cluster-update-variant/versions.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ terraform {
required_providers {
google-beta = {
source = "hashicorp/google-beta"
version = ">= 4.10.0, < 5.0"
version = ">= 4.25.0, < 5.0"
}
kubernetes = {
source = "hashicorp/kubernetes"
Expand Down
1 change: 1 addition & 0 deletions modules/beta-private-cluster/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ Then perform the following commands on the root folder:
| enable\_intranode\_visibility | Whether Intra-node visibility is enabled for this cluster. This makes same node pod to pod traffic visible for VPC network | `bool` | `false` | no |
| enable\_kubernetes\_alpha | Whether to enable Kubernetes Alpha features for this cluster. Note that when this option is enabled, the cluster cannot be upgraded and will be automatically deleted after 30 days. | `bool` | `false` | no |
| enable\_l4\_ilb\_subsetting | Enable L4 ILB Subsetting on the cluster | `bool` | `false` | no |
| enable\_managed\_prometheus | (Beta) Whether or not the managed collection is enabled. | `bool` | `false` | no |
| enable\_network\_egress\_export | Whether to enable network egress metering for this cluster. If enabled, a daemonset will be created in the cluster to meter network egress traffic. | `bool` | `false` | no |
| enable\_pod\_security\_policy | enabled - Enable the PodSecurityPolicy controller for this cluster. If enabled, pods must be valid under a PodSecurityPolicy to be created. | `bool` | `false` | no |
| enable\_private\_endpoint | (Beta) Whether the master's internal IP address is used as the cluster endpoint | `bool` | `false` | no |
Expand Down
9 changes: 9 additions & 0 deletions modules/beta-private-cluster/cluster.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,15 @@ resource "google_container_cluster" "primary" {
enable_components = var.monitoring_enabled_components
}
}
dynamic "monitoring_config" {
for_each = var.enable_managed_prometheus ? [1] : []

content {
managed_prometheus {
enabled = var.enable_managed_prometheus
}
}
}
cluster_autoscaling {
enabled = var.cluster_autoscaling.enabled
dynamic "auto_provisioning_defaults" {
Expand Down
6 changes: 6 additions & 0 deletions modules/beta-private-cluster/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,12 @@ variable "monitoring_enabled_components" {
default = []
}

variable "enable_managed_prometheus" {
type = bool
description = "(Beta) Whether or not the managed collection is enabled."
default = false
}

variable "istio" {
description = "(Beta) Enable Istio addon"
default = false
Expand Down
2 changes: 1 addition & 1 deletion modules/beta-private-cluster/versions.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ terraform {
required_providers {
google-beta = {
source = "hashicorp/google-beta"
version = ">= 4.10.0, < 5.0"
version = ">= 4.25.0, < 5.0"
}
kubernetes = {
source = "hashicorp/kubernetes"
Expand Down
1 change: 1 addition & 0 deletions modules/beta-public-cluster-update-variant/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ Then perform the following commands on the root folder:
| enable\_intranode\_visibility | Whether Intra-node visibility is enabled for this cluster. This makes same node pod to pod traffic visible for VPC network | `bool` | `false` | no |
| enable\_kubernetes\_alpha | Whether to enable Kubernetes Alpha features for this cluster. Note that when this option is enabled, the cluster cannot be upgraded and will be automatically deleted after 30 days. | `bool` | `false` | no |
| enable\_l4\_ilb\_subsetting | Enable L4 ILB Subsetting on the cluster | `bool` | `false` | no |
| enable\_managed\_prometheus | (Beta) Whether or not the managed collection is enabled. | `bool` | `false` | no |
| enable\_network\_egress\_export | Whether to enable network egress metering for this cluster. If enabled, a daemonset will be created in the cluster to meter network egress traffic. | `bool` | `false` | no |
| enable\_pod\_security\_policy | enabled - Enable the PodSecurityPolicy controller for this cluster. If enabled, pods must be valid under a PodSecurityPolicy to be created. | `bool` | `false` | no |
| enable\_resource\_consumption\_export | Whether to enable resource consumption metering on this cluster. When enabled, a table will be created in the resource export BigQuery dataset to store resource consumption data. The resulting table can be joined with the resource usage table or with BigQuery billing export. | `bool` | `true` | no |
Expand Down
9 changes: 9 additions & 0 deletions modules/beta-public-cluster-update-variant/cluster.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,15 @@ resource "google_container_cluster" "primary" {
enable_components = var.monitoring_enabled_components
}
}
dynamic "monitoring_config" {
for_each = var.enable_managed_prometheus ? [1] : []

content {
managed_prometheus {
enabled = var.enable_managed_prometheus
}
}
}
cluster_autoscaling {
enabled = var.cluster_autoscaling.enabled
dynamic "auto_provisioning_defaults" {
Expand Down
6 changes: 6 additions & 0 deletions modules/beta-public-cluster-update-variant/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,12 @@ variable "monitoring_enabled_components" {
default = []
}

variable "enable_managed_prometheus" {
type = bool
description = "(Beta) Whether or not the managed collection is enabled."
default = false
}

variable "istio" {
description = "(Beta) Enable Istio addon"
default = false
Expand Down
2 changes: 1 addition & 1 deletion modules/beta-public-cluster-update-variant/versions.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ terraform {
required_providers {
google-beta = {
source = "hashicorp/google-beta"
version = ">= 4.10.0, < 5.0"
version = ">= 4.25.0, < 5.0"
}
kubernetes = {
source = "hashicorp/kubernetes"
Expand Down
1 change: 1 addition & 0 deletions modules/beta-public-cluster/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ Then perform the following commands on the root folder:
| enable\_intranode\_visibility | Whether Intra-node visibility is enabled for this cluster. This makes same node pod to pod traffic visible for VPC network | `bool` | `false` | no |
| enable\_kubernetes\_alpha | Whether to enable Kubernetes Alpha features for this cluster. Note that when this option is enabled, the cluster cannot be upgraded and will be automatically deleted after 30 days. | `bool` | `false` | no |
| enable\_l4\_ilb\_subsetting | Enable L4 ILB Subsetting on the cluster | `bool` | `false` | no |
| enable\_managed\_prometheus | (Beta) Whether or not the managed collection is enabled. | `bool` | `false` | no |
| enable\_network\_egress\_export | Whether to enable network egress metering for this cluster. If enabled, a daemonset will be created in the cluster to meter network egress traffic. | `bool` | `false` | no |
| enable\_pod\_security\_policy | enabled - Enable the PodSecurityPolicy controller for this cluster. If enabled, pods must be valid under a PodSecurityPolicy to be created. | `bool` | `false` | no |
| enable\_resource\_consumption\_export | Whether to enable resource consumption metering on this cluster. When enabled, a table will be created in the resource export BigQuery dataset to store resource consumption data. The resulting table can be joined with the resource usage table or with BigQuery billing export. | `bool` | `true` | no |
Expand Down
9 changes: 9 additions & 0 deletions modules/beta-public-cluster/cluster.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,15 @@ resource "google_container_cluster" "primary" {
enable_components = var.monitoring_enabled_components
}
}
dynamic "monitoring_config" {
for_each = var.enable_managed_prometheus ? [1] : []

content {
managed_prometheus {
enabled = var.enable_managed_prometheus
}
}
}
cluster_autoscaling {
enabled = var.cluster_autoscaling.enabled
dynamic "auto_provisioning_defaults" {
Expand Down
6 changes: 6 additions & 0 deletions modules/beta-public-cluster/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,12 @@ variable "monitoring_enabled_components" {
default = []
}

variable "enable_managed_prometheus" {
type = bool
description = "(Beta) Whether or not the managed collection is enabled."
default = false
}

variable "istio" {
description = "(Beta) Enable Istio addon"
default = false
Expand Down
2 changes: 1 addition & 1 deletion modules/beta-public-cluster/versions.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ terraform {
required_providers {
google-beta = {
source = "hashicorp/google-beta"
version = ">= 4.10.0, < 5.0"
version = ">= 4.25.0, < 5.0"
}
kubernetes = {
source = "hashicorp/kubernetes"
Expand Down