-
Notifications
You must be signed in to change notification settings - Fork 16
refactor: Generalize mig Terraform module so it can be reused for TrusTEE #3351
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
Conversation
SanjayVas
left a comment
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.
@SanjayVas reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @georgi, @Marco-Premier, and @renjiezh).
src/main/terraform/gcloud/modules/mig/main.tf line 24 at r1 (raw file):
"tee-image-reference" = var.docker_image "tee-cmd" = jsonencode(var.tee_cmd), "tee-env-OTEL_SERVICE_NAME" = "edpa.results_fulfiller",
How do these removed metadata items get applied? Were they just extraneous here?
renjiezh
left a comment
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.
@renjiezh made 1 comment.
Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @georgi and @Marco-Premier).
src/main/terraform/gcloud/modules/mig/main.tf line 24 at r1 (raw file):
Previously, SanjayVas (Sanjay Vasandani) wrote…
How do these removed metadata items get applied? Were they just extraneous here?
These removed metadata fails the launch of MIG instance. EDPA has the same issue. We didn't noticed this because we don't have mechanism to detect the failure. It means the old EDPA instances are running while the new instance with these metadata never successfully launched. Deleting them is just a reversion of this PR.
8320805#diff-b0341929b87fef0862fe574e3d33fe325fa26790022f32f53729cfe956f86cadR24-R30
@georgi Could you please check this. Thanks
SanjayVas
left a comment
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.
@SanjayVas reviewed 1 file and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @georgi and @Marco-Premier).
stevenwarejones
left a comment
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.
@stevenwarejones reviewed 3 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @georgi and @Marco-Premier).
6fe7a5f to
dfa2032
Compare
renjiezh
left a comment
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.
@renjiezh reviewed 3 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @renjiezh).
No description provided.