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

[IBCDPE-1007] Templatizing items out to a root level #15

Merged
merged 194 commits into from
Aug 16, 2024

Conversation

BryanFauble
Copy link
Contributor

Problem:

  1. As noted by @BWMac in:
  1. There are a bunch of areas that are not templatized, and are using static values that we had set up for development.

Solution:

  1. Setting up more variable and environment value passing between the stacks. Allowing us to define all of the levers within the root main.tf and deployments/main.tf
  2. Removing the dev directory and instead allowing us to create different dev/prod stacks from the same terraform scripts depending on how we are defining it's usage in deployments/main.tf

Testing:

  1. Tested the deployment of the dev stack and verified that it came up healthy.

@spacelift-int-sagebionetworks spacelift-int-sagebionetworks bot temporarily deployed to spacelift/root-spacelift-administrative-stack August 2, 2024 16:48 Inactive
@spacelift-int-sagebionetworks spacelift-int-sagebionetworks bot temporarily deployed to spacelift/root-spacelift-administrative-stack August 2, 2024 16:51 Inactive
@spacelift-int-sagebionetworks spacelift-int-sagebionetworks bot temporarily deployed to spacelift/root-spacelift-administrative-stack August 2, 2024 17:00 Inactive
@spacelift-int-sagebionetworks spacelift-int-sagebionetworks bot temporarily deployed to spacelift/root-spacelift-administrative-stack August 2, 2024 17:33 Inactive
}
}
}
resource "kubectl_manifest" "vmservicescrape" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Swapping over to this is to resolve a race condition issue. Since the VMServiceScrape kind is a resource that depends on a CRD (Custom Resource Definition) being installed already on the cluster the terraform plan stage was failing with:

│ Error: API did not recognize GroupVersionKind from manifest (CRD may not be installed)
│ 
│   with module.trivy-operator.kubernetes_manifest.vmservicescrape,
│   on .terraform/modules/trivy-operator/main.tf line 20, in resource "kubernetes_manifest" "vmservicescrape":
│   20: resource "kubernetes_manifest" "vmservicescrape" {
│ 
│ no matches for kind "VMServiceScrape" in group
│ "operator.victoriametrics.com"

The suggestion was to try out a different provider (https://registry.terraform.io/providers/gavinbunney/kubectl/latest/docs/resources/kubectl_manifest) to get around this issue. This allows the resources to be created as expected in the right order.

Copy link
Contributor

@BWMac BWMac Aug 2, 2024

Choose a reason for hiding this comment

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

Interesting that this resource has you pass a whole YAML configuration to the yaml_body arg instead of something more similar to how kubernetes_manifest is configured

@BryanFauble BryanFauble marked this pull request as ready for review August 2, 2024 18:12
@BryanFauble BryanFauble requested a review from a team as a code owner August 2, 2024 18:12
@@ -16,7 +16,7 @@ victoria-metrics-operator:
cleanupImage:
repository: bitnami/kubectl
# use image tag that matches k8s API version by default
# tag: 1.29.6
tag: 1.30.2
Copy link
Contributor

@BWMac BWMac Aug 2, 2024

Choose a reason for hiding this comment

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

Is pinning this tag better than using the default as was done before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was a bug that I had reported around this. Basically a '+' sign that was getting included in the tag image that prevented the cleanup job from running. I updated the helm chart to the newer version so we don't need to set this tag manually anymore. I will remove

@BWMac BWMac self-requested a review August 2, 2024 21:46
@spacelift-int-sagebionetworks spacelift-int-sagebionetworks bot temporarily deployed to spacelift/root-spacelift-administrative-stack August 12, 2024 18:54 Inactive
Base automatically changed from ibcdpe-1007-monitoring to main August 15, 2024 23:07
@BryanFauble BryanFauble merged commit 782556d into main Aug 16, 2024
9 of 11 checks passed
@BryanFauble BryanFauble deleted the ibcdpe-1007-split-into-vars branch August 16, 2024 00:14
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