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

doc: Add quickstart guide to README #126

Merged
merged 12 commits into from
Dec 12, 2022
Merged

doc: Add quickstart guide to README #126

merged 12 commits into from
Dec 12, 2022

Conversation

hessjcg
Copy link
Collaborator

@hessjcg hessjcg commented Dec 6, 2022

Adds quickstart instructions to the README. This tells the user how to follow the existing
Cloud SQL Quickstart on GKE guide, adding a few extra steps in the middle, to try out
the operator.

@hessjcg hessjcg requested a review from a team December 6, 2022 16:07
README.md Outdated
your kuberentes cluster:

```shell
curl -o install.sh https://storage.googleapis.com/cloud-sql-connectors/cloud-sql-proxy-operator-dev/0.0.5/install.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

The version in the version.txt is 0.0.2-dev?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've updated make generate to ensure this is consistent this going forward.

README.md Outdated

```shell
curl -o install.sh https://storage.googleapis.com/cloud-sql-connectors/cloud-sql-proxy-operator-dev/0.0.5/install.sh
bash install.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just pipe the url to bash?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed.

README.md Outdated
Comment on lines 55 to 57
portEnvName: "DB_PORT"
hostEnvName: "INSTANCE_HOST"
socketType: "tcp"
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these required to specify or just the default?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You have to specify portEnvName and hostEnvName or else the environment variables will not be set.

README.md Outdated
@@ -9,34 +9,140 @@ which specifies the Cloud SQL Auth Proxy configuration for a workload. The opera
reads this resource and adds a properly configured Cloud SQL Auth Proxy container
to the matching workload pods.

## Setting up the initial project
These commands will be run to initialize the kubebuilder project
# Quick Start
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this should be an h2?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed.

README.md Outdated
Comment on lines 135 to 136
Note that there are now two containers on the pods, while there is only one
container on the deployment. The operator adds a second proxy container configured
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Note that there are now two containers on the pods, while there is only one
container on the deployment. The operator adds a second proxy container configured
Note that there are now two containers in the pods, while there is only one
container in the original deployment. The operator adds a second proxy container configured

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed.

README.md Outdated
@@ -9,34 +9,140 @@ which specifies the Cloud SQL Auth Proxy configuration for a workload. The opera
reads this resource and adds a properly configured Cloud SQL Auth Proxy container
to the matching workload pods.

## Setting up the initial project
These commands will be run to initialize the kubebuilder project
# Quick Start
Copy link
Contributor

Choose a reason for hiding this comment

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

More of a meta discussion: is QuickStart the right format for the README?

The rest of our repos start with Installations and follow up with Usage instructions that specify how to configure it in different ways. Should we adopt a similar format?

Maybe a Quickstart is a something that could go in the examples folder, and we can link to it as a "try this out quickly option" at the top the README

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea. I'll move it around.

README.md Outdated
rm -rf bin
.bin/kubebuilder init --owner "Google LLC" --project-name "cloud-sql-proxy-operator" --domain cloud.google.com --repo github.com/GoogleCloudPlatform/cloud-sql-proxy-operator
```shell
curl https://storage.googleapis.com/cloud-sql-connectors/cloud-sql-proxy-operator/v0.0.2-dev/install.sh | bash
Copy link
Member

Choose a reason for hiding this comment

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

v0.0.2-dev

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed. This is automatically updated by make generate match version.txt

name: "gke-cloud-sql-app" # named 'gke-cloud-sql-app'
instances:
- connectionString: "my-project:us-central1:instance" # from your Cloud SQL Database instance
socketType: "tcp" # Proxy will open a TCP socket for this instance
Copy link
Member

Choose a reason for hiding this comment

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

If this isn't required, let's leave it off.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed.

name: gke-cloud-sql-operator-demo
type: Opaque
data:
DB_PASS: cGFzc3dvcmQ= # "password"
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth adding a comment about how to generate these?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@@ -0,0 +1,131 @@
# Quick Start -# Quick Start
Follow the instructions in the Quick Start Guide for Cloud SQL: [Connect to Cloud SQL for PostgreSQL from Google Kubernetes Engine](https://cloud.google.com/sql/docs/postgres/connect-instance-kubernetes)
Copy link
Member

Choose a reason for hiding this comment

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

Let's wrap the lines at 80 characters and use shortlinks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

your kuberentes cluster:

```shell
curl https://storage.googleapis.com/cloud-sql-connectors/cloud-sql-proxy-operator/v0.0.2-dev/install.sh | bash
Copy link
Member

Choose a reason for hiding this comment

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

v0.0.2-dev

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed. This automatically is updated by make generate

- connectionString: "<INSTANCE_CONNECTION_NAME>"
portEnvName: "DB_PORT"
hostEnvName: "INSTANCE_HOST"
socketType: "tcp"
Copy link
Member

Choose a reason for hiding this comment

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

Again socketType can go?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed.

set -euxo
SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
PROJECT_DIR=$( dirname "$SCRIPT_DIR")

cd "$PROJECT_DIR"


RELEASE_PROJECT_ID="cloud-sql-connectors"
Copy link
Member

Choose a reason for hiding this comment

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

Should this be part of a separate commit?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this is actually done in #136. It will disappear from this PR once #136 is merged.

@hessjcg hessjcg merged commit 891d6d8 into main Dec 12, 2022
@hessjcg hessjcg deleted the onboard-docs branch December 13, 2022 18:07
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.

3 participants