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

feat(cloud_sql): add ssl_mode to PostgreSQL example #538

Merged
merged 6 commits into from
Dec 18, 2023

Conversation

feng-zhe
Copy link
Contributor

Description

Fixes #310700535

Checklist

Readiness

  • Yes, merge this PR after it is approved
  • No, don't merge this PR after it is approved

Style

Testing

Intended location

  • Yes, this sample will be (or already is) included on cloud.google.com
    Location(s):

  • No, this sample won't be included on cloud.google.com
    Reason:

API enablement

  • If the sample needs an API enabled to pass testing, I have added the service to the Test setup file

Review

  • If this sample adds a new directory, I have added codeowners to the CODEOWNERS file

@feng-zhe feng-zhe requested review from a team as code owners November 28, 2023 01:35
@msampathkumar msampathkumar self-assigned this Nov 28, 2023
@msampathkumar
Copy link
Contributor

/gcbrun

@msampathkumar
Copy link
Contributor

@feng-zhe - Thank you for sharing testing logs

@msampathkumar
Copy link
Contributor

/gcbrun

@feng-zhe
Copy link
Contributor Author

Hi @msampathkumar, thanks for reviewing the change. I saw the above
terraform-docs-samples-int-trigger (cloud-foundation-cicd) failed with --- FAIL: TestSamples/2/sqlserver_instance_private_ip (603.18s). I feel my change should not impact it. Please advice. Thanks.

@msampathkumar
Copy link
Contributor

Hi @msampathkumar, thanks for reviewing the change. I saw the above terraform-docs-samples-int-trigger (cloud-foundation-cicd) failed with --- FAIL: TestSamples/2/sqlserver_instance_private_ip (603.18s). I feel my change should not impact it. Please advice. Thanks.

Sorry for the trouble. The CICD is the process of being upgraded and we would have wait for it be completed or this bug to cleared up.

@msampathkumar
Copy link
Contributor

/gcbrun

@feng-zhe
Copy link
Contributor Author

feng-zhe commented Dec 1, 2023

Thanks @msampathkumar! Somehow the CICD still fails and I cannot find anything related to this change.

Update: I saw it says my branch is outdated from main. So I just clicked the "update branch" button. Hope it helps. Thanks.

@feng-zhe
Copy link
Contributor Author

feng-zhe commented Dec 5, 2023

Hi @msampathkumar , any updates? Please let me know what I should to make the CICD errors go away. Thanks.

@msampathkumar
Copy link
Contributor

/gcbrun

@msampathkumar msampathkumar changed the title docs: Introduce the ssl_mode into the PostgreSQL example that enforce… feat(cloud_sql): add ssl_mode to PostgreSQL example Dec 6, 2023
@msampathkumar msampathkumar enabled auto-merge (squash) December 6, 2023 16:07
auto-merge was automatically disabled December 7, 2023 19:07

Head branch was pushed to by a user without write access

@feng-zhe
Copy link
Contributor Author

feng-zhe commented Dec 7, 2023

Thanks, I've changed it to POSTGRES_14.

@feng-zhe
Copy link
Contributor Author

Hi @msampathkumar, just a friendly ping. Thanks.

@msampathkumar
Copy link
Contributor

/gcbrun

@msampathkumar msampathkumar enabled auto-merge (squash) December 15, 2023 12:25
@msampathkumar
Copy link
Contributor

Error:

Step #5 - "test-samples-2": TestSamples/2/sqlserver_instance_private_ip 2023-12-15T13:27:57Z command.go:185: Error: Unable to remove Service Networking Connection, err: Error waiting for Delete Service Networking Connection: Error code 9, message: Failed to delete connection; Producer services (e.g. CloudSQL, Cloud Memstore, etc.) are still using this connection.

@msampathkumar
Copy link
Contributor

I will be doing for manual testing for approval.

Copy link
Contributor

@msampathkumar msampathkumar left a comment

Choose a reason for hiding this comment

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

Manually tested. LGTM.

@msampathkumar msampathkumar merged commit 3e9f924 into terraform-google-modules:main Dec 18, 2023
4 of 5 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.

None yet

2 participants