-
Notifications
You must be signed in to change notification settings - Fork 696
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
update document and script to use gcloud beta terraform vet #729
update document and script to use gcloud beta terraform vet #729
Conversation
gsutil cp gs://terraform-validator/releases/${ENV_TERRAFORM_VALIDATOR_RELEASE}/terraform-validator-linux-amd64 /builder/terraform/terraform-validator && \ | ||
chmod +x /builder/terraform/terraform-validator && \ | ||
/builder/google-cloud-sdk/bin/gcloud components update && \ | ||
/builder/google-cloud-sdk/bin/gcloud components install terraform-tools && \ |
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.
Hi @iyabchen Thanks for the PR.
This change to add
/builder/google-cloud-sdk/bin/gcloud components install terraform-tools && \
would also need to be done at the bootstrap module before been able to switch to gcloud beta terraform vet
since the image
is used to run the cloud builds with the tf-wrapper.sh
script for the steps 1 to 4
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.
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 updated the dockerfile based on bootstrap repository.
The tests are failing, and one of them are failing for cloud resource manager API not enabled. The other is failing to set IAM policy. It doesn't seem to be related to the changes, unless this is related to base image change.
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.
test failure might be related to terraform-google-modules/terraform-google-bootstrap#148.
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.
LGTM, I will wait for @daniel-cit to do an e2e and ensure this works across all of our stages.
0-bootstrap/main.tf
Outdated
@@ -101,8 +100,7 @@ resource "google_billing_account_iam_member" "tf_billing_admin" { | |||
|
|||
// Comment-out the cloudbuild_bootstrap module and its outputs if you want to use Jenkins instead of Cloud Build | |||
module "cloudbuild_bootstrap" { | |||
source = "terraform-google-modules/bootstrap/google//modules/cloudbuild" | |||
version = "~> 5.0" | |||
source = "github.com/terraform-google-modules/terraform-google-bootstrap//modules/cloudbuild" |
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 just cut a 6.0 release so we can switch this back to the registry and "~> 6.0"
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.
Done, switch it back to 6.0
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.
Hi @iyabchen @bharathkkb
I tested all stages with terraform 0.13.7
and 1.1.9
.
we have two issues:
-
in step 3-networks we have this old issue not directly related to
gcloud beta terraform vet
but that fails the build:
google_compute_subnetwork enable_flow_logs deprecated in google terraform 3.0.0 and GCPNetworkEnableFlowLogsConstraintV1 broken GoogleCloudPlatform/policy-library#414 -
in steps
0-bootstrap
, due togoogle_folder_iam_member
andgoogle_folder_iam_binding
, and in step4-projects
due togoogle_folder_iam_member
we have this crash:
fail to parse terraform 1.1.9 plan withgoogle_folder_iam_member
in a new folder GoogleCloudPlatform/terraform-validator#708
I removed the usage of google_folder_iam_member
and google_folder_iam_binding
from steps 0 and 4 and there is no other issues related to gcloud beta terraform vet
on those steps.
the crash has been fixed but it in not yet in a gcloud release
I can retest the json files created for the stages after a new gcloud
release that contains the fix
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.
@daniel-cit , gcloud cli 393.0.0 is released, and it is with terraform-tools 0.5.0, which should cover the fix for crash issue you mentioned.
Would you be possible to do a retest?
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.
Hi @iyabchen I will do the retest
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.
@daniel-cit I updated to 393.0.0, and the tests are failing for 409 errors... I don't quite get it.
If the bootstrap repo also need to update to 393.0.0 and trigger a new release, let me know.
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 Error 409: Requested entity already exists
is a name collision with a deleted project that is still in the 30 days interval in which it can be restored.
I rerun the builds and I will keep track of it
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.
Seems the test run into something new Error deleting folder 'fldr-production': googleapi: Error 400: Failed to delete folder [folders/38748887386] due to: The folder being deleted is non-empty.
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.
It failed differently on rerun: Error enabling the Compute Engine API required to delete the default network
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.
all checks passed
1ab3c3f
to
ac4d547
Compare
#714
(the cloud foundation repo's generate doc is replacing some strings, but not impacting the README in general. )