tests: add acquire tests #1741
tests: add acquire tests #1741
Conversation
Signed-off-by: Alex Pana <[email protected]>
Captured with: ```bash $ WRITE_GOLDEN_OUTPUT=1 GOLDEN_REQUEST_CHECKS=1 E2E_KUBE_TARGET=envtest E2E_GCP_TARGET=real RUN_E2E=1 go test -test.count=1 -timeout 360s -v ./tests/e2e -run TestE2EScript/scenarios/direct/logging/acquire 2>&1 ``` Signed-off-by: Alex Pana <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| PUT https://logging.googleapis.com/v2/projects/${projectId}/metrics/logginglogmetric-to-acquire?alt=json | ||
| Content-Type: application/json | ||
| User-Agent: kcc/controller-manager DeclarativeClientLib/0.0.1 | ||
|
|
||
| { | ||
| "filter": "resource.type=gae_app AND severity\u003c=INFO", | ||
| "metricDescriptor": { | ||
| "labels": [], | ||
| "metricKind": "DELTA", | ||
| "unit": "1", | ||
| "valueType": "INT64" | ||
| } | ||
| } |
There was a problem hiding this comment.
surprised to see this PUT from the dcl controller while acquiring. Curious what the diff computed was that required an upgrade, it looks like it's adding the labels slice at first sight.
There was a problem hiding this comment.
Yeah, maybe we need the PR that logs the diff in the controller to be sure.
|
I ran this against |
| @@ -265,6 +271,40 @@ func TestE2EScript(t *testing.T) { | |||
| }) | |||
| } | |||
|
|
|||
| // applies objects directly to the cloud provider | |||
There was a problem hiding this comment.
Could we instead apply and then abandon them? Even if we use a custom verb to do that, this feels like it's duplicating our direct controller layer (?)
There was a problem hiding this comment.
We could do that but technically we will be testing the acquisition or adoption of a resource that we created and then un-managed. As opposed to to acquiring a previsouly unknown resource to us. Although I am not convinced that this distinction is meaningful That is, we can probably use the scenario you proposed to better effect.
If we do want to acquire a "clean" resource, I can expose the adapter or reuse some of the ResourceContext methods I added in #1768
| PUT https://logging.googleapis.com/v2/projects/${projectId}/metrics/logginglogmetric-to-acquire?alt=json | ||
| Content-Type: application/json | ||
| User-Agent: kcc/controller-manager DeclarativeClientLib/0.0.1 | ||
|
|
||
| { | ||
| "filter": "resource.type=gae_app AND severity\u003c=INFO", | ||
| "metricDescriptor": { | ||
| "labels": [], | ||
| "metricKind": "DELTA", | ||
| "unit": "1", | ||
| "valueType": "INT64" | ||
| } | ||
| } |
There was a problem hiding this comment.
Yeah, maybe we need the PR that logs the diff in the controller to be sure.
|
My inclination here is that we use the fact that this passes for confidence in our release, but we don't merge this in 1.118. WDYT @acpana ? |
|
Yes, this should not be a blocker for 1.118. |
This patch adds an acquire test for step based testing.