DO NOT MERGE #2702
DO NOT MERGE #2702
Conversation
|
[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 |
| @@ -247,7 +71,7 @@ X-Xss-Protection: 0 | |||
| { | |||
| "error": { | |||
| "code": 404, | |||
| "message": "Resource 'projects/${projectId}/locations/us-west1/attachedClusters/kcc-attached-cluster-127' was not found", | |||
| "message": "cluster not found", | |||
There was a problem hiding this comment.
Could you update the MockGCP to match this message?
| "endTime": "2024-04-01T12:34:56.123456Z", | ||
| "target": "projects/${projectId}/locations/us-west1/attachedClusters/kcc-attached-cluster-127", | ||
| "target": "projects/${projectNumber}/locations/us-west1/attachedClusters/kcc-attached-cluster-127", | ||
| "verb": "create" | ||
| }, | ||
| "name": "projects/${projectId}/locations/us-west1/operations/${operationID}", | ||
| "name": "projects/${projectNumber}/locations/us-west1/operations/${operationID}", | ||
| "response": { | ||
| "@type": "type.googleapis.com/google.cloud.gkemulticloud.v1.AttachedCluster", | ||
| "binaryAuthorization": { | ||
| "evaluationMode": "DISABLED" | ||
| }, | ||
| "createTime": "2024-04-01T12:34:56.123456Z", | ||
| "description": "Test attached cluster", | ||
| "distribution": "eks", | ||
| "etag": "abcdef0123A=", | ||
| "fleet": { | ||
| "membership": "projects/${projectNumber}/locations/global/memberships/kcc-attached-cluster-127", | ||
| "project": "projects/${projectNumber}" | ||
| }, | ||
| "kubernetesVersion": "1.28", | ||
| "loggingConfig": {}, | ||
| "name": "projects/${projectId}/locations/us-west1/attachedClusters/kcc-attached-cluster-127", | ||
| "monitoringConfig": { | ||
| "managedPrometheusConfig": {} | ||
| }, | ||
| "name": "projects/${projectNumber}/locations/us-west1/attachedClusters/kcc-attached-cluster-127", | ||
| "oidcConfig": { | ||
| "issuerUrl": "https://oidc.eks.us-west-2.amazonaws.com/id/A115FE1C770C2452C75219524036FC0F" | ||
| }, | ||
| "platformVersion": "1.27.0-gke.2", | ||
| "state": "RUNNING" | ||
| "state": "RUNNING", | ||
| "uid": "111111111111111111111", | ||
| "updateTime": "2024-04-01T12:34:56.123456Z" |
There was a problem hiding this comment.
Could you update the MockGCP to match the response metadata? This is important for KCC to monitor the GCP service behavior and maintain the available features (etag, updateTime, etc).
| "binaryAuthorization": { | ||
| "evaluationMode": "DISABLED" | ||
| }, |
There was a problem hiding this comment.
ditto: Could you update the mockgcp to match the real GCP for this field? We want to guardrail the default values given from the GCP service (otherwise KCC could overrule this)
| # organizationRef: | ||
| # external: ${TEST_ORG_ID} | ||
| folderRef: | ||
| external: "182571047842" |
There was a problem hiding this comment.
We could ignore the change here.
| createTime: "1970-01-01T00:00:00Z" | ||
| kubernetesVersion: "1.28" | ||
| observedGeneration: 2 | ||
| state: RUNNING | ||
| uid: "12345678" | ||
| updateTime: "1970-01-01T00:00:00Z" |
There was a problem hiding this comment.
We want to update the merge-in code to reflect this part, this is important for KCC to maintain the resource and align the expected behavior with the GCP service.
There was a problem hiding this comment.
You can ignore the logs related to Project and Folder, but for http logs related to the attachedCluster resource, we need to make sure the HTTP response and metadata matches the real GCP record (otherwise, KCC could easily override and miss the default values handling and overrule some feature related behavior like timestamp checking).
If you can fix the project/folder log, that would be a bonus! You only need to replace the spec.projectRef.name to spec.projectRef.external, and no dependencies.yaml any more
No description provided.