fix: Move resolvers out of Adapter #2758
Conversation
gemmahou
force-pushed
the
direct-delete
branch
from
134fd49 to
29bb0c3
Compare
gemmahou
force-pushed
the
direct-delete
branch
2 times, most recently
from
5a977ed to
c6bec2b
Compare
|
The presubmit pause job failed, I think we need to increase the timeout of this job. |
| @@ -673,7 +673,7 @@ func MaybeSkip(t *testing.T, name string, resources []*unstructured.Unstructured | |||
| case schema.GroupKind{Group: "compute.cnrm.cloud.google.com", Kind: "ComputeNodeGroup"}: | |||
| case schema.GroupKind{Group: "compute.cnrm.cloud.google.com", Kind: "ComputeNodeTemplate"}: | |||
| case schema.GroupKind{Group: "compute.cnrm.cloud.google.com", Kind: "ComputeManagedSSLCertificate"}: | |||
| //case schema.GroupKind{Group: "compute.cnrm.cloud.google.com", Kind: "ComputeServiceAttachment"}: | |||
There was a problem hiding this comment.
Yes, we started noticing this deletion issue in a forwarding rule test with service attachment dependency, and it blocked presubmits. So, we disabled this mock service. #1069
I added it back here because the issue has been addressed in this PR, and the presubmits all passed now.
|
|
||
| --- | ||
|
|
||
| POST https://compute.googleapis.com/compute/v1/projects/example-project-01/regions/us-west1/forwardingRules/${forwardingRuleID}/setLabels |
There was a problem hiding this comment.
Do you know how the code change ends up with a new setLabels call? This looks weird
There was a problem hiding this comment.
I reason is I disabled this test before #1069, so I haven't updated its log to reflect the recent code changes(the set labels behavior during creation). I can collect the realGCP w/TF log to verify this change in the log is intended.
| @@ -1187,6 +1187,86 @@ X-Content-Type-Options: nosniff | |||
| X-Frame-Options: SAMEORIGIN | |||
| X-Xss-Protection: 0 | |||
|
|
|||
| { | |||
| "IPProtocol": "TCP", | |||
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yuwenma The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
gemmahou
force-pushed
the
direct-delete
branch
from
c6bec2b to
3d71f57
Compare
gemmahou
force-pushed
the
direct-delete
branch
2 times, most recently
from
1a82067 to
4e1eb35
Compare
gemmahou
force-pushed
the
direct-delete
branch
from
4e1eb35 to
578953c
Compare
google-oss-prow
bot
merged commit a37ae6a
into
GoogleCloudPlatform:master
Change description
Deleting a resource does not require resolving dependencies. However, if dependencies are deleted prior to the resource itself, it will prevent the deletion of that resource.
Therefore, we should move resolvers to their own function outside the Adapter. We can resolve them during the create and update only.
Fixes #2621
Tests you have done
make ready-prto ensure this PR is ready for review.