-
Notifications
You must be signed in to change notification settings - Fork 472
self-hosted: update CI test to use new aws modular terraform structure #32665
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
base: main
Are you sure you want to change the base?
self-hosted: update CI test to use new aws modular terraform structure #32665
Conversation
@@ -12,32 +12,32 @@ provider "aws" { | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI is currently failing with:
2025-06-05T19:08:53.636032Z ERROR k8s_controller::controller: Materialize reconciliation error. err=reconciler for object Materialize.v1alpha1.materialize.cloud/12345678-1234-1234-1234-123456789012.materialize-environment failed source=failed to apply object: ApiError: "404 page not found\n": Failed to parse error data (ErrorResponse { status: "404 Not Found", message: "\"404 page not found\\n\"", reason: "Failed to parse error data", code: 404 }) source.sources=[ApiError: "404 page not found\n": Failed to parse error data (ErrorResponse { status: "404 Not Found", message: "\"404 page not found\\n\"", reason: "Failed to parse error data", code: 404 }), ApiError: "404 page not found\n": Failed to parse error data (ErrorResponse { status: "404 Not Found", message: "\"404 page not found\\n\"", reason: "Failed to parse error data", code: 404 }), "404 page not found\n": Failed to parse error data] controller="orchestratord.materialize.cloud/materialize"
Seems more like an issue with main
rather than the TF module changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Temporary test is green: https://buildkite.com/materialize/nightly/builds/12236
Upgrade test is green: https://buildkite.com/materialize/nightly/builds/12237
9af232c
to
c6fea29
Compare
resource "random_password" "db_password" { | ||
length = 32 | ||
special = false | ||
provider "kubernetes" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@def- the new structure comes with many breaking changes, so we might should probably theardown the old aws persistent infra for now and then spin it up again using the new module structure.
Also the aws-persistent
and the aws-temporary
projects are identical except the name_prefix
so should probably get rid of one of those and in the mzcompose.py file we could just pass the name_prefix
as a -var
. But we could do it in a follow up PR!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have deleted it!
git commit -m "Bump to operator $CI_HELM_CHART_VERSION, materialize $CI_MZ_VERSION" | ||
git diff HEAD~ | ||
run_if_not_dry git push origin main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a copy of how we currently bump the terraform-helm-materialize
mdouel, open to suggestions if we should do this the same way for the new refactored AWS module.
We could leave the helm chart version as null in the Terraform module so it would default to the latest helm version here, but we would still need the bump for the envd image version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably need a broader discussion about how we manage versions of these modules. That should probably take place verbally or in slack, rather than on this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's here LGTM. We should have a discussion about our repo structure and how we bump versions, though.
git commit -m "Bump to operator $CI_HELM_CHART_VERSION, materialize $CI_MZ_VERSION" | ||
git diff HEAD~ | ||
run_if_not_dry git push origin main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably need a broader discussion about how we manage versions of these modules. That should probably take place verbally or in slack, rather than on this PR.
@@ -346,6 +346,7 @@ def kubectl_setup( | |||
"requests": {"cpu": "100m", "memory": "256Mi"}, | |||
}, | |||
"backendSecretName": "materialize-backend", | |||
"authenticatorKind": "None", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for adding this. The password support code got reverted, and I'm including this in my next try.
Motivation
Depends on MaterializeInc/terraform-aws-materialize#69
Ready for a review but we should not merge it yet until MaterializeInc/terraform-aws-materialize#69 gets merged.