-
Notifications
You must be signed in to change notification settings - Fork 78
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
gcp : add alloydb group #373
Conversation
/test-examples="examples/alloydb/instance.yaml" |
@turkenf are you able to assist with the failure on |
Hi @jcockbain, After running make generate there seem to be changes you didn't commit, can you run make generate again and commit all the changes? |
ace6a9e
to
615dcc3
Compare
@turkenf thanks. I have updated my branch, and changed the commits to show the manual and then the generated changes. |
Hi @turkenf, I run |
@jcockbain, could you please rebase your branch to the main, run make generate, and commit all the changes again? |
615dcc3
to
dcdde76
Compare
Thanks @turkenf, there were extra files generated after rebasing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jcockbain, many thanks for your contribution. I left a few comments for you to consider.
Could you please fill in how it was tested after the last changes for each resource as here?
Thanks very much for your comments @turkenf, I have attempted to resolve them all in the above commit. I also retested with the examples and added screenshots to the summary of this PR. |
One thing I am not certain on is what to name example resources, for my testing I gave the resources different names as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the quick update @jcockbain, the names look good for now, they can stay that way. I just left two minor comments that fix the example-ids.
/test-examples="examples/alloydb/cluster.yaml" |
1 similar comment
/test-examples="examples/alloydb/cluster.yaml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jcockbain, Thank you again for your contribution and clean work. LGTM.
Description of your changes
Fixes #371
Adds resources for:
google_alloydb_backup
google_alloydb_instance
google_alloydb_cluster
I have:
make reviewable test
to ensure this PR is ready for review.How has this code been tested
I deployed instances of the 3 new kinds to to a minikube cluster. These were based on the generated examples, adapted for my test GCP project.
I then used
make run
, which created the expected resources in my project.Each of these resource became "ready". Deleting the k8s resources deletes the resources from GCP.
Cluster: https://github.com/upbound/provider-gcp/actions/runs/6110391551
Instance:
Backup: