fix(vertexai): implement Read for EndpointWithModelGardenDeployment - #18547
fix(vertexai): implement Read for EndpointWithModelGardenDeployment#18547syangcode wants to merge 1 commit into
Conversation
|
Github actions was done earlier today. Starting the build now. |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes for commit a31310d: Diff reportYour PR generated the following diffs in downstream repositories:
Test reportAnalytics
Affected Service Packages
Step 1: Replaying Mode Action takenFound 7 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
View the replaying VCR build log Step 2: Recording Mode
Caution Issues requiring attention before PR completion 🔴 Initial Recording Failed: Some tests failed during the recording step. See the table above for details. Please address these issues to complete your PR. If you believe these detections are incorrect or unrelated to your change, please raise the concern with your reviewer. View the recording VCR build log or the debug logs folder for detailed results. @syangcode VCR tests complete for a31310d! |
c2thorn
left a comment
There was a problem hiding this comment.
Error: Error waiting to create EndpointWithModelGardenDeployment: Error waiting for Creating EndpointWithModelGardenDeployment: Error code 13, message: Internal error occurred. Contact Vertex AI.
Any idea @syangcode ?
a31310d to
1ce1eab
Compare
|
Hi there, I'm the Modular magician. I've detected the following information about your changes for commit 1ce1eab: Diff reportYour PR generated the following diffs in downstream repositories:
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_vertex_ai_endpoint_with_model_garden_deployment" "primary" {
model_config {
hugging_face_cache_enabled = # value needed
model_display_name = # value needed
}
}
Test reportAnalytics
Affected Service Packages
Step 1: Replaying Mode Action takenFound 7 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
View the replaying VCR build log Step 2: Recording Mode
Caution Issues requiring attention before PR completion 🔴 Initial Recording Failed: Some tests failed during the recording step. See the table above for details. Please address these issues to complete your PR. If you believe these detections are incorrect or unrelated to your change, please raise the concern with your reviewer. View the recording VCR build log or the debug logs folder for detailed results. @syangcode, @c2thorn VCR tests complete for 1ce1eab! |
1ce1eab to
989be23
Compare
|
Hi there, I'm the Modular magician. I've detected the following information about your changes for commit 989be23: Diff reportYour PR generated the following diffs in downstream repositories:
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_vertex_ai_endpoint_with_model_garden_deployment" "primary" {
deploy_config {
dedicated_resources {
machine_spec {
multihost_gpu_node_count = # value needed
}
required_replica_count = # value needed
spot = # value needed
}
fast_tryout_enabled = # value needed
system_labels = # value needed
}
model_config {
hugging_face_cache_enabled = # value needed
model_display_name = # value needed
}
}
Test reportAnalytics
Affected Service Packages
Step 1: Replaying Mode Action takenFound 6 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
View the replaying VCR build log Step 2: Recording Mode
Caution Issues requiring attention before PR completion 🔴 Initial Recording Failed: Some tests failed during the recording step. See the table above for details. Please address these issues to complete your PR. If you believe these detections are incorrect or unrelated to your change, please raise the concern with your reviewer. View the recording VCR build log or the debug logs folder for detailed results. @syangcode, @c2thorn VCR tests complete for 989be23! |
989be23 to
39bf995
Compare
|
Hi @c2thorn could you please help to rebuild? |
started the build (it's in queue) FYI go/terraform-contribution-guide#before-you-begin outlines steps to get automatic build approvals |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes for commit 39bf995: Diff reportYour PR generated the following diffs in downstream repositories:
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_vertex_ai_endpoint_with_model_garden_deployment" "primary" {
deploy_config {
system_labels = # value needed
}
model_config {
hugging_face_cache_enabled = # value needed
model_display_name = # value needed
}
}
Test reportAnalytics
Affected Service Packages
Step 1: Replaying Mode Action takenFound 6 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
View the replaying VCR build log Step 2: Recording Mode
Caution Issues requiring attention before PR completion 🔴 Initial Recording Failed: Some tests failed during the recording step. See the table above for details. Please address these issues to complete your PR. If you believe these detections are incorrect or unrelated to your change, please raise the concern with your reviewer. View the recording VCR build log or the debug logs folder for detailed results. @syangcode, @c2thorn VCR tests complete for 39bf995! |
|
@c2thorn This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
c2thorn
left a comment
There was a problem hiding this comment.
@syangcode do you know how we can fix the failing TestAccVertexAIEndpointWithModelGardenDeployment tests? We are making a significant change to the ModelGardenDeployment resource, so now is the time to repair them.
4070db8 to
26214af
Compare
26214af to
c153650
Compare
…ployment exclude_read: true made the generated Read a no-op, so state was never reconciled against the live Endpoint and terraform plan reported "No changes" even after the deployment had drifted. Adds a decoder that reads min, max and required replica count off the DeployedModel matching deployed_model_id, and clears the id when that model is gone. Those are the only fields mutateDeployedModel can change, so they are the only ones worth reading. Everything else is immutable and the Endpoint response does not carry most of it, so model_config, endpoint_config and machine_spec are kept from state by custom flatteners rather than read back. deploy_config and max_replica_count are default_from_api to absorb what the API supplies when a config omits them. Covers the drift-detection half of hashicorp/terraform-provider-google#27250.
c153650 to
069e656
Compare
|
@c2thorn, I believe these tests require the L4 GPU quota in your CI projects, Could you please run And increase the quota for us-central1 region? Thanks |
doubled the quota from 28 -> 56. I'll retry. |
|
/gcbrun |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes for commit 069e656: Diff reportYour PR generated the following diffs in downstream repositories:
Test reportAnalytics
Affected Service Packages
Step 1: Replaying Mode Action takenFound 5 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
View the replaying VCR build log Step 2: Recording Mode
Caution Issues requiring attention before PR completion 🔴 Initial Recording Failed: Some tests failed during the recording step. See the table above for details. Please address these issues to complete your PR. If you believe these detections are incorrect or unrelated to your change, please raise the concern with your reviewer. View the recording VCR build log or the debug logs folder for detailed results. @syangcode, @c2thorn VCR tests complete for 069e656! |
|
@c2thorn This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
|
We’re currently requesting a change in process for Googlers (or individuals on behalf of Googlers) contributing changes to the Terraform Provider for Google Cloud. Please see go/terraform-ssp-adjustment for details. |
|
reopening as this is a continuation from b/514579514 |
@syangcode I increased the quota but no difference on the tests. |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes for commit 069e656: Diff reportYour PR generated the following diffs in downstream repositories:
Test reportAnalytics
Affected Service Packages
Step 1: Replaying Mode Action takenFound 5 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
View the replaying VCR build log Step 2: Recording Mode
Caution Issues requiring attention before PR completion 🔴 Initial Recording Failed: Some tests failed during the recording step. See the table above for details. Please address these issues to complete your PR. If you believe these detections are incorrect or unrelated to your change, please raise the concern with your reviewer. View the recording VCR build log or the debug logs folder for detailed results. @syangcode, @c2thorn VCR tests complete for 069e656! |
The resource is generated with
exclude_read: true, so the generated Read is a no-op (return nil). State is never reconciled against the live Endpoint, so out-of-band changes go unnoticed andterraform planreports "No changes" even when the deployment has drifted.This wires Read up, but reads back only the replica counts.
Why only the replica counts
Almost every field on this resource is immutable. The only ones the API can change in place are those
mutateDeployedModelaccepts, which #18116 already wired into Update. A difference anywhere else could only mean the deployment was replaced out of band, and that case is already covered by clearing the id when the DeployedModel is gone.This covers the drift-detection half of hashicorp/terraform-provider-google#27250. The in-place update half shipped in #18116.
Testing
Tested by hand against a live deployment (paligemma-224-float32 on
g2-standard-12+NVIDIA_L4, us-west1):min/max = 1 2.terraform planis clean, exit 0. Worth checking separately from step 4, since a decoder that catches real drift but also introduces phantom diffs would be a net regression.maxReplicaCountto 4 out of band viaendpoints:mutateDeployedModel.terraform planexits 2 and reports~ max_replica_count = 4 -> 2.Step 4 exits 0 with "No changes" on released v7.42.0.