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

Thanos fixes #161

Merged
merged 3 commits into from
Feb 20, 2024
Merged

Thanos fixes #161

merged 3 commits into from
Feb 20, 2024

Conversation

RMcVelia
Copy link
Collaborator

@RMcVelia RMcVelia commented Feb 19, 2024

Context

Thanos fixes

Changes proposed in this pull request

  1. Some resources are failing due to a namespace dependency.
    I couldn't find out why it suddenly started occurring, so have added a dependency in terraform.

e.g.
│ Error: Failed to create deployment: namespaces "monitoring" not found

│ with kubernetes_deployment.thanos-compactor,
│ on thanos.tf line 247, in resource "kubernetes_deployment" "thanos-compactor":
│ 247: resource "kubernetes_deployment" "thanos-compactor" {

│ Error: Failed to create deployment: namespaces "infra" not found

│ with kubernetes_deployment.welcome_app[0],
│ on welcome_app.tf line 1, in resource "kubernetes_deployment" "welcome_app":
│ 1: resource "kubernetes_deployment" "welcome_app" {

  1. updated the dev cluster thanos storage account name, as it was using the same name for all 6 dev clusters.

  2. changes to allow the querier to see the store correctly

Guidance to review

make development terraform-apply ENVIRONMENT=clusterx
make platform-test terraform-plan (shouldn't see any changes)

scripts/pfwd.sh
http://localhost:8082
checking the stores, should see the sidecar and the store

Checklist

  • I have performed a self-review of my code, including formatting and typos
  • I have cleaned the commit history
  • I have added the Devops label
  • I have attached the pull request to the trello card

@@ -176,4 +176,9 @@ locals {
storage-account-name = azurerm_storage_account.thanos.name
storage-account-key = azurerm_storage_account.thanos.primary_access_key
}

cluster_sa_name = (var.cluster_short != "dv" ?
Copy link
Member

Choose a reason for hiding this comment

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

Can we avoid hardcording the environment name?
Maybe compare environment and config as in https://github.com/DFE-Digital/teacher-services-cloud/blob/main/cluster/terraform_kubernetes/variables.tf#L132

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated as suggested

We don't need to label by replica as it's a single prometheus
Prometheus service should be clusterip not nodeport
Thanos store gateway is a headless service
@RMcVelia RMcVelia merged commit ca06f57 into main Feb 20, 2024
4 checks passed
@RMcVelia RMcVelia deleted the thanos-fixes branch February 20, 2024 13:42
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