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

module(gke-cluster-standard): align monitoring_config with logging_config #1665

Closed
gustavovalverde opened this issue Sep 9, 2023 · 5 comments · Fixed by #1680
Closed

module(gke-cluster-standard): align monitoring_config with logging_config #1665

gustavovalverde opened this issue Sep 9, 2023 · 5 comments · Fixed by #1680

Comments

@gustavovalverde
Copy link
Contributor

Logging and Monitoring for GKE have almost the same options (configuration), but after this PR the way to activate both differs considerably:

I'd expect Monitoring to have a similar approach, like:

variable "monitoring_config" {
  description = "Monitoring components."
  type = object({
    enable_system_monitoring             = optional(bool, true)
    enable_workloads_monitoring          = optional(bool, false)
    enable_api_server_monitoring         = optional(bool, false)
    enable_scheduler_monitoring          = optional(bool, false)
    enable_controller_manager_monitoring = optional(bool, false)
  })
@juliocc
Copy link
Collaborator

juliocc commented Sep 12, 2023

@gustavovalverde can I assign this to you?

Also, the latest version of the provider added even more options

@olliefr
Copy link
Collaborator

olliefr commented Sep 14, 2023

Also, the latest version of the provider added even more options

Yay, I've been waiting for this to happen! I have a local commit which includes those but as I run Terraform plan at the time I discovered that the provider is not quite there yet in terms of supporting them! 😅 I'm glad the latest provider supports it.

@olliefr
Copy link
Collaborator

olliefr commented Sep 14, 2023

I'd expect Monitoring to have a similar approach

I agree! What you are seeing is the old interface to this functionality 🙂

There's a bit of context here. Maybe it would help if I briefly recap what happened.

I've been experimenting with Autopilot clusters a few weeks ago and I had noticed that Autopilot clusters module did not have any logging or monitoring configuration support at all. So I added logging because I needed it (#1625). My next step would have been to add monitoring, but @juliocc liked my object interface and asked if I could do the same to the Standard clusters module so that we have a user-friendly and uniform interface between the two modules. And so I did (#1638). It took me longer because Standard clusters are more flexible when it comes to logging and monitoring configuration so the validation is harder. Also, because Standard clusters already had an interface to configure logging, it took me some time to update the blueprints and parts of FAST that relied on the old interface. But that was done.

Then I moved on to add monitoring configuration for Autopilot clusters (#1646) because there was none.

So at this point we have the following situation with the configuration interface:

  • Autopilot: new logging, new monitoring
  • Standard: new logging, old monitoring

I've been working on the new configuration for monitoring for Standard clusters but my real (paid) work got in the way 😞

The reason Standard is taking longer is because just like with the logging, monitoring configuration is more flexible in Standard so the validation is harder and I am aiming to get that right. And there are existing uses in blueprints and probably in FAST. So I am treading carefully.

If you feel confident taking over, @gustavovalverde, I'd be happy to leave this task to you! My original interest was in Autopilot clusters and they already work the way I like 😄 Alternatively, if you are happy to wait a bit, the functionality that you are after is going to come some time soon :shipit:

@olliefr
Copy link
Collaborator

olliefr commented Sep 14, 2023

Also, the latest version of the provider added even more options

@juliocc I'm eager to add those Kube state metrics introduced in v4.82.0 but I'm being conscious that they are not GA. Please let me know if it's OK to add them and I'll raise a separate PR for it 🙃

@juliocc
Copy link
Collaborator

juliocc commented Sep 15, 2023

@juliocc I'm eager to add those Kube state metrics introduced in v4.82.0 but I'm being conscious that they are not GA. Please let me know if it's OK to add them and I'll raise a separate PR for it 🙃

I don't see an issue, if you need/want them then probably others do too. Go for it.

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