docs: Update the example healthcheck config #1950

hessjcg · 2023-09-14T18:59:21Z

The configuration example for health checks now matches match the default configuration used by the cloud-sql-proxy-operator.

Fixes #1943

enocom · 2023-09-14T19:04:36Z

examples/k8s-health-check/proxy_with_http_health_check.yaml

+
+          # This flag specifies where the service account key can be found
+          # Remove this argument if you are using workload identity
+        - name: CSQL_CREDENTIALS_FILE


Do we want to show how to do this? Or do we want to use WI by default?

I'll remove the CSQL_CREDENTIALS_FILE.

enocom · 2023-09-14T19:05:43Z

examples/k8s-health-check/proxy_with_http_health_check.yaml

+        # needs. For details, see https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
+        resources:
+          requests:
+            cpu: "500e-3"


This doesn't match what we have elsewhere. Is there a reason?

Updated to match the sidecar config.

enocom · 2023-09-14T19:05:54Z

examples/k8s-health-check/proxy_with_http_health_check.yaml

+          # Do not allow privilege escalation
+          allowPrivilegeEscalation : false
+        volumeMounts:
+        - name: <YOUR-SA-SECRET-VOLUME>


Same here -- do we want to highlight key file usage?

enocom · 2023-09-19T16:36:49Z

examples/k8s-health-check/README.md

+# name in the format: "project_name:region:instance_name"
+# Replace <DB_PORT> with the port that the proxy should open
+# to listen for database connections from the application
+- <INSTANCE_CONNECTION_NAME>?port=<DB_PORT>


Why are we using the query string syntax instead of --port=<DB_PORT>?

Or CSQL_PROXY_PORT=<DB_PORT> below?

This example, like the operator, sets the port number on the instance instead of setting a
a global starting port number. I prefer to make a direct, unambiguous association between database
instance and port number. However, if you prefer, I'll switch it to --port=<DB_PORT>

I think the port flag is clear because there aren't more instances involved. It also matches our other examples.

enocom · 2023-09-19T16:40:53Z

examples/k8s-health-check/README.md

+# to listen for database connections from the application
+- <INSTANCE_CONNECTION_NAME>?port=<DB_PORT>
+
+env:


If we recommend env vars for Kubernetes, shall we change all our other examples too?

I'm on board with that approach. That will be in a different PR though.

Let's file an issue to track that work and link to it here then.

Done. #1969

enocom

Let's use the --port syntax to be consistent with other examples.

hessjcg · 2023-09-20T20:36:54Z

Fixed.

enocom · 2023-09-20T21:25:35Z

examples/k8s-health-check/proxy_with_http_health_check.yaml

+        # name in the format: "project_name:region:instance_name"
+        # Replace <DB_PORT> with the port that the proxy should open
+        # to listen for database connections from the application
+        - <INSTANCE_CONNECTION_NAME>


FYI This could be an environment variable too. CSQL_PROXY_INSTANCE_CONNECTION_NAME=...

Not sure if we want to do that, though.

I avoided that in the operator's implementation because when configuring multiple instances, the envvar was somewhat more confusing than the args.

this would be highly appreciated to have. We can't move from 1.x to 2.x because of this missing. I even opened a bugticket through the support: https://issuetracker.google.com/issues/273982124

@w32-blaster would you mind opening an issue so we can track that here?

enocom · 2023-09-20T21:26:11Z

examples/k8s-health-check/proxy_with_http_health_check.yaml

+        - name: CSQL_PROXY_ADMIN_PORT
+          value: "9092"
+        - name: CSQL_PROXY_USER_AGENT
+          value: cloud-sql-proxy-operator/unknown


I don't think we should add this here -- this is a private implementation detail for our purposes here.

Good catch, fixed.

…ator configuration. chore: code review comments.

enocom · 2023-09-26T18:17:39Z

examples/k8s-health-check/proxy_with_http_health_check.yaml

@@ -49,90 +49,100 @@ spec:
      - name: cloud-sql-proxy
        # It is recommended to use the latest version of the Cloud SQL Auth Proxy
        # Make sure to update on a regular schedule!
-        image: gcr.io/cloud-sql-connectors/cloud-sql-proxy:2.1.0
+        image: gcr.io/cloud-sql-connectors/cloud-sql-proxy:2.6.0


hessjcg requested a review from a team as a code owner September 14, 2023 18:59

blunderbuss-gcf bot assigned jackwotherspoon Sep 14, 2023

hessjcg mentioned this pull request Sep 14, 2023

Explicitly open port 9090 on the cloud-sql-proxy sidecar for k8s to hit the health check endpoints #1943

Closed

enocom reviewed Sep 14, 2023

View reviewed changes

hessjcg requested a review from enocom September 19, 2023 15:23

hessjcg force-pushed the gh-1943-better-k8s-docs branch from 6fcf77e to 772bf9a Compare September 19, 2023 15:59

enocom reviewed Sep 19, 2023

View reviewed changes

hessjcg requested a review from enocom September 20, 2023 17:22

hessjcg mentioned this pull request Sep 20, 2023

Update k8s examples to use envvar instead of cli flags #1969

Open

enocom requested changes Sep 20, 2023

View reviewed changes

hessjcg force-pushed the gh-1943-better-k8s-docs branch from 772bf9a to 8501f03 Compare September 20, 2023 20:36

hessjcg requested a review from enocom September 20, 2023 20:36

enocom reviewed Sep 20, 2023

View reviewed changes

jackwotherspoon mentioned this pull request Sep 26, 2023

k8s readiness check failing #1975

Closed

hessjcg added 3 commits September 26, 2023 10:15

docs: Update the example healthcheck config to match the default oper…

805f20d

…ator configuration. chore: code review comments.

fix: use global --port config.

300063f

fix: code review comments

01bc52f

hessjcg force-pushed the gh-1943-better-k8s-docs branch from 8501f03 to 01bc52f Compare September 26, 2023 16:18

enocom approved these changes Sep 26, 2023

View reviewed changes

enocom merged commit a5fa6f3 into main Oct 16, 2023
12 checks passed

enocom deleted the gh-1943-better-k8s-docs branch October 16, 2023 21:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: Update the example healthcheck config #1950

docs: Update the example healthcheck config #1950

hessjcg commented Sep 14, 2023 •

edited

Loading

enocom Sep 14, 2023

hessjcg Sep 19, 2023

enocom Sep 14, 2023

hessjcg Sep 19, 2023

enocom Sep 14, 2023

hessjcg Sep 19, 2023

enocom Sep 19, 2023

enocom Sep 19, 2023

hessjcg Sep 20, 2023

enocom Sep 20, 2023

enocom Sep 19, 2023

hessjcg Sep 20, 2023

enocom Sep 20, 2023

hessjcg Sep 20, 2023

enocom left a comment

hessjcg commented Sep 20, 2023

enocom Sep 20, 2023

enocom Sep 20, 2023

hessjcg Sep 26, 2023

w32-blaster Nov 7, 2023

enocom Nov 7, 2023

enocom Sep 20, 2023

hessjcg Sep 26, 2023

enocom Sep 26, 2023

docs: Update the example healthcheck config #1950

docs: Update the example healthcheck config #1950

Conversation

hessjcg commented Sep 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enocom left a comment

Choose a reason for hiding this comment

hessjcg commented Sep 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hessjcg commented Sep 14, 2023 •

edited

Loading