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

Switch to the new API for marking as required the region field of the Cloud Function resource #576

Conversation

trampfox
Copy link
Contributor

Description of your changes

This PR switches to the new config.Resource.MarkAsRequired to mark as required the region field of the Cloud Function resource.

Fixes #575

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Signed-off-by: trampfox <trampfox@gmail.com>
@trampfox trampfox force-pushed the fix/cloud-functions-switch-to-the-new-required-api branch from bfead46 to 34ea968 Compare July 16, 2024 14:48
@jeanduplessis
Copy link
Collaborator

/test-examples="examples/cloudfunctions/v1beta2/function.yaml"

Copy link
Collaborator

@turkenf turkenf left a comment

Choose a reason for hiding this comment

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

Hey @trampfox, thanks for the fix. The change looks good to me, but it would be good if you add the manual test results to the description before continuing. The resource's uptest skipped because it contains manual-intervention. So, can you test manually and verify that the problem is gone

@clarkritchie
Copy link

Chiming in to provide moral support on merging this one.

@turkenf
Copy link
Collaborator

turkenf commented Sep 6, 2024

Hi @clarkritchie, thank you for your interest. Are you willing to test the resource manually?

@clarkritchie
Copy link

@turkenf Sure thing, apologies that I missed your comment.

@turkenf
Copy link
Collaborator

turkenf commented Sep 19, 2024

@clarkritchie, could you please test with the following:

apiVersion: pkg.crossplane.io/v1
kind: Provider
metadata:
  name: provider-gcp-cloudfunctions
spec:
  package: index.docker.io/turkenf/provider-gcp-cloudfunctions:v1.6.0-rc.0.15.g34ea968e1

@chakrii
Copy link

chakrii commented Sep 20, 2024

@turkenf I can confirm CloudFunction creation is working when pointing to package: index.docker.io/turkenf/provider-gcp-cloudfunctions:v1.6.0-rc.0.15.g34ea968e1
Failure
Provider pointing xpkg.upbound.io/upbound/provider-gcp-cloudfunctions:v1.0.2
Screenshot 2024-09-20 at 3 26 29 PM
Success
Provider pointing to package: index.docker.io/turkenf/provider-gcp-cloudfunctions:v1.6.0-rc.0.15.g34ea968e1
Screenshot 2024-09-20 at 3 30 20 PM

@clarkritchie
Copy link

Awesome, thank you @chakrii!

@turkenf
Copy link
Collaborator

turkenf commented Sep 21, 2024

@chakrii, thank you for sharing results,I couldn't quite understand from the screenshots you shared, does the issue go away even if the provider is not healthy?

Edit: Ahh, I just saw that the config package is unhealthy

@chakrii
Copy link

chakrii commented Sep 23, 2024

@chakrii, thank you for sharing results,I couldn't quite understand from the screenshots you shared, does the issue go away even if the provider is not healthy?

Edit: Ahh, I just saw that the config package is unhealthy

@turkenf Config package is unhealthy but at least the CloudFunction got created and no longer seeing issue.

Copy link
Collaborator

@turkenf turkenf left a comment

Choose a reason for hiding this comment

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

Thanks for your patience and interaction here, folks. LGTM.

@turkenf turkenf merged commit dd13de0 into crossplane-contrib:main Oct 9, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Cloud Function creation fails with SetNew only operates on computed keys - region is not one
5 participants