feat: Add Apigee Envgroup direct controller #3332

Camila-B · 2024-12-06T17:53:14Z

Change description

Defines the Apigee Envgroup API

Tests you have done

Run make ready-pr to ensure this PR is ready for review.
Perform necessary E2E testing for changed resources.

apis/apigee/v1alpha1/types.generated.go

apis/apigee/v1alpha1/googlecloudapigeev1environmentgroup_types.go

jasonvigil · 2024-12-06T18:48:58Z

Seems to be a failure in presubmit validations, looks like some of the generated files need to be regenerated:

ERROR: Manifests must be regenerated. Please run 'make ready-pr' or 'make manifests' and update your PR.

jasonvigil · 2024-12-06T18:50:54Z

Also, looks like you fixed one of the issues in tests/apichecks/testdata/exceptions/acronyms.txt, and we can remove the exception now! See unit test job output: https://github.com/GoogleCloudPlatform/k8s-config-connector/actions/runs/12203794855/job/34047550729?pr=3332

apis/apigee/v1alpha1/environmentgroup_identity.go

apis/apigee/v1alpha1/environmentgroup_reference.go

apis/apigee/v1alpha1/environmentgroup_types.go

dev/tools/controllerbuilder/pkg/commands/generatetypes/generatetypescommand.go

pkg/controller/direct/apigee/envgroup_mappings.go

apis/refs/v1alpha1/organizationref.go

pkg/controller/direct/apigee/envgroup_controller.go

jasonvigil · 2024-12-19T23:00:25Z

/lgtm

jasonvigil · 2024-12-19T23:41:25Z

/lgtm
/assign yuwenma

yuwenma · 2024-12-20T18:55:17Z

/approve

yuwenma

/hold

yuwenma

/hold

yuwenma

I have a few questions related to the LRO and returned value format. Can we upload the real GCP log to show the git-diff?

apis/apigee/v1alpha1/environmentgroup_identity.go

yuwenma · 2024-12-20T18:59:25Z

apis/apigee/v1alpha1/environmentgroup_identity.go

+// New builds a GoogleCloudApigeeV1EnvironmentGroupIdentity from the Config Connector GoogleCloudApigeeV1EnvironmentGroup object.
+func NewGoogleCloudApigeeV1EnvironmentGroupIdentity(ctx context.Context, reader client.Reader, obj *ApigeeEnvgroup) (*GoogleCloudApigeeV1EnvironmentGroupIdentity, error) {
+	// Get Parent
+	orgRef, err := refs.ResolveOrganization(ctx, reader, obj, obj.Spec.Parent.OrganizationRef)


We should not do the resolve reference here, because it will block this resource deletion.
Here's the scenario:
Users want to delete both env and org. Org is deleted first and this always returns error. Adapter cannot be initialized and therefore cannot be deleted forever. cc @jasonvigil

I am not sure that it will be possible to avoid resolving the reference here, because the organization is the parent for this resource. Even in NormalizeExternal we resolve the resource, if it is not specified in external format.

apis/apigee/v1alpha1/environmentgroup_identity.go

apis/refs/v1alpha1/helper.go

apis/refs/v1alpha1/organizationref.go

yuwenma · 2024-12-20T19:13:45Z

pkg/controller/direct/apigee/envgroup_controller.go

+				return nil
+			}
+		}
+		time.Sleep(2 * time.Second)


How long does this update normally takes?

From testing, it seems to be close to instantaneous (< 1 second)

pkg/controller/direct/apigee/envgroup_mappings.go

...resourcefixture/testdata/basic/apigee/v1alpha1/apigeeenvgroup/apigeeenvgroup-basic/_http.log

...lpha1/apigeeenvgroup/apigeeenvgroup-basic/_generated_object_apigeeenvgroup-basic.golden.yaml

yuwenma · 2024-12-20T19:17:49Z

tests/apichecks/testdata/exceptions/acronyms.txt

@@ -19,7 +19,6 @@
 [acronyms] crd=apigeeaddonsconfigs.apigee.cnrm.cloud.google.com version=v1alpha1: field ".spec.addonsConfig.advancedApiOpsConfig.enabled" should be ".spec.addonsConfig.advancedAPIOpsConfig.enabled"
 [acronyms] crd=apigeeendpointattachments.apigee.cnrm.cloud.google.com version=v1alpha1: field ".spec.orgId" should be ".spec.orgID"
 [acronyms] crd=apigeeenvgroupattachments.apigee.cnrm.cloud.google.com version=v1alpha1: field ".spec.envgroupId" should be ".spec.envgroupID"
-[acronyms] crd=apigeeenvgroups.apigee.cnrm.cloud.google.com version=v1alpha1: field ".spec.orgId" should be ".spec.orgID"


yuwenma · 2024-12-20T19:20:35Z

/remove-approve

google-oss-prow · 2024-12-20T19:20:56Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from yuwenma. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jasonvigil · 2025-01-13T18:04:30Z

/lgtm

acpana · 2025-01-14T18:30:40Z

@yuwenma could we merge this in so I can build on top of it too 😆 ?

google-oss-prow bot added the do-not-merge/work-in-progress label Dec 6, 2024

jasonvigil reviewed Dec 6, 2024

View reviewed changes

apis/apigee/v1alpha1/types.generated.go Outdated Show resolved Hide resolved

apis/apigee/v1alpha1/googlecloudapigeev1environmentgroup_types.go Outdated Show resolved Hide resolved

jasonvigil mentioned this pull request Dec 12, 2024

fix: Specify that MockGCP will not pass due to API changes #3324

Merged

Camila-B force-pushed the apigee-envgroup-2 branch from f1e817b to 849e051 Compare December 18, 2024 22:19

Camila-B changed the title ~~[WIP] Define the Apigee Envgroup API~~ [WIP] feat: Add Apigee Envgroup direct controller Dec 18, 2024

Camila-B changed the title ~~[WIP] feat: Add Apigee Envgroup direct controller~~ feat: Add Apigee Envgroup direct controller Dec 18, 2024

Camila-B requested a review from jasonvigil December 18, 2024 23:47

Camila-B force-pushed the apigee-envgroup-2 branch from f4b7885 to 8e734cb Compare December 19, 2024 00:33

Camila-B marked this pull request as ready for review December 19, 2024 00:33

google-oss-prow bot removed the do-not-merge/work-in-progress label Dec 19, 2024

jasonvigil reviewed Dec 19, 2024

View reviewed changes

Camila-B requested a review from jasonvigil December 19, 2024 22:00

jasonvigil reviewed Dec 19, 2024

View reviewed changes

pkg/controller/direct/apigee/envgroup_controller.go Show resolved Hide resolved

google-oss-prow bot assigned jasonvigil Dec 19, 2024

google-oss-prow bot added the lgtm label Dec 19, 2024

Camila-B force-pushed the apigee-envgroup-2 branch from e471f42 to 4988777 Compare December 19, 2024 23:28

google-oss-prow bot removed the lgtm label Dec 19, 2024

Camila-B requested a review from jasonvigil December 19, 2024 23:28

google-oss-prow bot assigned yuwenma Dec 19, 2024

google-oss-prow bot added the lgtm label Dec 19, 2024

google-oss-prow bot added the approved label Dec 20, 2024

yuwenma reviewed Dec 20, 2024

View reviewed changes

google-oss-prow bot added the do-not-merge/hold label Dec 20, 2024

yuwenma reviewed Dec 20, 2024

View reviewed changes

google-oss-prow bot removed approved lgtm labels Dec 20, 2024

Camila-B force-pushed the apigee-envgroup-2 branch 3 times, most recently from bd24dff to 9637802 Compare December 20, 2024 23:36

Camila-B force-pushed the apigee-envgroup-2 branch from 03caf93 to 282f9fb Compare January 3, 2025 01:24

Camila-B requested a review from yuwenma January 6, 2025 17:28

Camila-B added 4 commits January 7, 2025 18:46

Generate types

f47f132

Generate mappers

c3aa6c2

Add direct controller

8a329c3

Update mockapigee to support the envgroup direct controller

a1e3698

Camila-B force-pushed the apigee-envgroup-2 branch from 1a0a85c to bad6c64 Compare January 7, 2025 18:47

Camila-B added 8 commits January 7, 2025 18:52

refactor: Envgroup API and controller

0530181

refactor: envgroup mappings and apigee organization ref

98aecf8

Use int64 when normalizing createdAt and lastModifiedAt

87980cc

Split apigeeEnvgroupClient into envgroups client and operations client

c2d484b

Change desired envgroup from api obj to krm

37c2cf1

Add PATCH tests for Apigee Envgroup

c9169b5

log: real gcp

bb8aa41

log: mock gcp

dba7bfd

Camila-B force-pushed the apigee-envgroup-2 branch from bad6c64 to dba7bfd Compare January 7, 2025 19:18

yuwenma assigned acpana Jan 9, 2025

google-oss-prow bot added the lgtm label Jan 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add Apigee Envgroup direct controller #3332

feat: Add Apigee Envgroup direct controller #3332

Camila-B commented Dec 6, 2024 •

edited

Loading

jasonvigil commented Dec 6, 2024

jasonvigil commented Dec 6, 2024

jasonvigil commented Dec 19, 2024

jasonvigil commented Dec 19, 2024

yuwenma commented Dec 20, 2024

yuwenma left a comment

yuwenma left a comment

yuwenma left a comment

yuwenma Dec 20, 2024

jasonvigil Dec 20, 2024 •

edited

Loading

yuwenma Dec 20, 2024

Camila-B Jan 6, 2025

yuwenma Dec 20, 2024

yuwenma commented Dec 20, 2024

google-oss-prow bot commented Dec 20, 2024

jasonvigil commented Jan 13, 2025

acpana commented Jan 14, 2025

feat: Add Apigee Envgroup direct controller #3332

Are you sure you want to change the base?

feat: Add Apigee Envgroup direct controller #3332

Conversation

Camila-B commented Dec 6, 2024 • edited Loading

Change description

Tests you have done

jasonvigil commented Dec 6, 2024

jasonvigil commented Dec 6, 2024

jasonvigil commented Dec 19, 2024

jasonvigil commented Dec 19, 2024

yuwenma commented Dec 20, 2024

yuwenma left a comment

Choose a reason for hiding this comment

yuwenma left a comment

Choose a reason for hiding this comment

yuwenma left a comment

Choose a reason for hiding this comment

yuwenma Dec 20, 2024

Choose a reason for hiding this comment

jasonvigil Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

yuwenma Dec 20, 2024

Choose a reason for hiding this comment

Camila-B Jan 6, 2025

Choose a reason for hiding this comment

yuwenma Dec 20, 2024

Choose a reason for hiding this comment

yuwenma commented Dec 20, 2024

google-oss-prow bot commented Dec 20, 2024

jasonvigil commented Jan 13, 2025

acpana commented Jan 14, 2025

Camila-B commented Dec 6, 2024 •

edited

Loading

jasonvigil Dec 20, 2024 •

edited

Loading