Skip to content
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

Resolve the observed state for TF-based resources #1053

Conversation

maqiuyujoyce
Copy link
Collaborator

Change description

Fixes b/309651934

  • Supported the logic to resolve the observed state for TF-based resources during a successful reconciliation.
    • Reference fields, labels field, name field, sensitive fields, and fields under an array are currently not supported in the observed state.
    • Added unit tests.
  • Added more unit tests for service mappings to ensure the same limitations are enforced.

Tests you have done

  • [N/A] Run make ready-pr to ensure this PR is ready for review.
  • Perform necessary E2E testing for changed resources.

Added the following configurations in the resource config for StorageBucket and generated the CRD.

observedFields:
  - versioning.enabled
  - location

Ran the integration test with the following YAML:

apiVersion: storage.cnrm.cloud.google.com/v1beta1
kind: StorageBucket
metadata:
  name: storagebucket-sample-${uniqueId}
spec:
  versioning:
    enabled: true
  location: US-WEST1
  lifecycleRule:
    - action:
        type: Delete
      condition:
        age: 7

The created StorageBucket contains the observed state in status:

         "observedState": {
            "location": "US-WEST1",
            "versioning": {
                "enabled": true
            }
        },

Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maqiuyujoyce

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Collaborator

@diviner524 diviner524 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mock test looks to be flaky recently. There is a timeout error:

harness.go:153: error stopping envtest environment: timeout waiting for process kube-apiserver to stop

I think you can retry it.

@diviner524
Copy link
Collaborator

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Dec 7, 2023
@google-oss-prow google-oss-prow bot merged commit 1e21f6a into GoogleCloudPlatform:master Dec 7, 2023
6 checks passed
@maqiuyujoyce
Copy link
Collaborator Author

Found a related topic about the error message: kubernetes-sigs/controller-runtime#1571 though we may have already done the workaround for it:

ctx, cancel := context.WithCancel(ctx)
I wondered if context cancel() is indeed called before env.Stop(), or if there are any other issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants