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: [DENA-603] Example of init database job #50

Merged
merged 13 commits into from
Jul 22, 2024
Merged

feat: [DENA-603] Example of init database job #50

merged 13 commits into from
Jul 22, 2024

Conversation

MarcinGinszt
Copy link
Collaborator

  • create new resource which ensures that database and user exists and creates cert for the user
  • create example of using it
  • fix the OpsLevel annotations
  • Fix two bugs:
  1. wrong hostname for init job- with current hostname (cockroachdb-proxy) cluster was not initialized correctly and nodes were unable to communicate
  2. ensure init job won't crash for fresh databases

@MarcinGinszt MarcinGinszt requested a review from a team as a code owner July 11, 2024 08:45
Copy link

linear bot commented Jul 11, 2024

@MarcinGinszt MarcinGinszt changed the title [DENA-603] Example of init database job feat: [DENA-603] Example of init database job Jul 11, 2024
kind: Job
name: cockroach-init
version: v1
# This patch creates new client cert (to be mounted within client)
Copy link
Contributor

Choose a reason for hiding this comment

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

confused by this comment: below it looks to modify an existing one, not create a new one (since the patch is replaceing)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We create new client cert. When you were writing this comment, we were doing that by replacing the name. Now, we are doing that by adding name prefix.
We are using base to create new resources with overlays 🙂
Adding new directories like example-database with similar content will cause creation of new jobs and ceriticates. So that you can initialize multiple databases

path: /metadata/name
value: example-database-cockroachdb-init
- op: replace
path: /spec/template/spec/containers/0/env/3/value
Copy link
Contributor

Choose a reason for hiding this comment

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

is a strategic merge patch possible here, so we can set values based on env var names, and not their position, which have the potential to change over time with updates?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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


# If database already exists and contains tables, grant access to them for user.
# This command will fail for freshly initialized databases.
$SQL_CMD --execute "GRANT ALL ON TABLE $DB_NAME.* TO $DB_USER;" || true
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't suppose there's an easy way to make this conditional in the SQL like TABLES_COUNT > 0 && GRANT ... so we avoid any confusing/worrying looking logs in the case no tables exist

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cockroachdb doesn't have conditional statements, it seems.
That's why I came up with this solution. Currently, logs are looking like

Cluster is not ready yet. Retry 1/20. Waiting for 3 seconds...
Cluster is not ready yet. Retry 2/20. Waiting for 3 seconds...
Cluster is not ready yet. Retry 3/20. Waiting for 3 seconds...
Cluster is not ready yet. Retry 4/20. Waiting for 3 seconds...
Cluster is not ready yet. Retry 5/20. Waiting for 3 seconds...
Cluster is not ready yet. Retry 6/20. Waiting for 3 seconds...
CREATE ROLE
CREATE DATABASE
GRANT
ERROR: no object matched
SQLSTATE: 42704
Failed running "sql"

Which is not awful, I think.
But why not, I made them nicer here

Comment on lines +31 to +40
- name: COCKROACH_PORT
valueFrom:
configMapKeyRef:
name: cockroach
key: cockroach.port
- name: COCKROACH_CERTS_DIR
value: "/cockroach-certs/"
# This variables should be replaced by patches
- name: DB_NAME
value: "default"
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we mix some values being set from configMap and some being patched in, what are the different use cases for each approach?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's db init job- it's used as a base to create new jobs for each new db/user.
Some of the env will be the same for each of jobs (set from CM), while some needs to be customized (I almost spelled it through k, lol), and those are patched.

Would you suggest documenting this better?

Choose a reason for hiding this comment

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

Speaking of documentation, do we reference these examples in READMEs?

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 plan to rewrite most of the documentation, so that it does fit the current formula from shared-kustomize-bases. Hence, I'll do it it another ticket

Copy link
Contributor

Post kustomize build diff:

diff --git a/built-manifests/cockroachdb/example/cert-manager/db-init-manifests/example-database/manifests.yaml b/built-manifests/cockroachdb/example/cert-manager/db-init-manifests/example-database/manifests.yaml
new file mode 100644
index 0000000..414e1bc
--- /dev/null
+++ b/built-manifests/cockroachdb/example/cert-manager/db-init-manifests/example-database/manifests.yaml
@@ -0,0 +1,78 @@
+apiVersion: batch/v1
+kind: Job
+metadata:
+  labels:
+    app: cockroach-db-init
+    version: v1
+  name: example-database-cockroach-db-init
+spec:
+  template:
+    spec:
+      containers:
+      - command:
+        - /bin/bash
+        - /opt/scripts/user-schema-bootstrap.sh
+        env:
+        - name: DB_NAME
+          value: example_database
+        - name: DB_USER
+          value: example_user
+        - name: COCKROACH_HOST
+          valueFrom:
+            configMapKeyRef:
+              key: cockroach.host
+              name: cockroach
+        - name: COCKROACH_PORT
+          valueFrom:
+            configMapKeyRef:
+              key: cockroach.port
+              name: cockroach
+        - name: COCKROACH_CERTS_DIR
+          value: /cockroach-certs/
+        image: cockroachdb/cockroach:v23.2.2
+        imagePullPolicy: IfNotPresent
+        name: db-init
+        resources:
+          limits:
+            cpu: 1
+            memory: 512Mi
+          requests:
+            cpu: 0
+            memory: 128Mi
+        volumeMounts:
+        - mountPath: /cockroach-certs
+          name: client-certs
+        - mountPath: /opt/scripts
+          name: cockroachdb-scripts
+          readOnly: true
+      restartPolicy: OnFailure
+      volumes:
+      - name: client-certs
+        projected:
+          defaultMode: 256
+          sources:
+          - secret:
+              items:
+              - key: ca.crt
+                path: ca.crt
+              - key: tls.crt
+                path: client.root.crt
+              - key: tls.key
+                path: client.root.key
+              name: cockroachdb.client.root
+      - configMap:
+          name: cockroachdb-scripts
+        name: cockroachdb-scripts
+---
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: example-database-cockroach-client
+spec:
+  commonName: example_user
+  issuerRef:
+    kind: Issuer
+    name: cockroachdb-ca-issuer
+  secretName: cockroachdb.client.example-user
+  usages:
+  - client auth
diff --git a/root-manifests/cockroachdb/example/cert-manager/manifests.yaml b/built-manifests/cockroachdb/example/cert-manager/manifests.yaml
index b7e4552..1e675e0 100644
--- a/root-manifests/cockroachdb/example/cert-manager/manifests.yaml
+++ b/built-manifests/cockroachdb/example/cert-manager/manifests.yaml
@@ -2,6 +2,8 @@ apiVersion: v1
 kind: ServiceAccount
 metadata:
   annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
     vault.uw.systems/aws-role: <your aws role here>
   name: cockroachdb
   namespace: <your namespace here>
@@ -12,6 +14,9 @@ data:
   cockroach.port: "26257"
 kind: ConfigMap
 metadata:
+  annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
   name: cockroach-g92fdb9h27
 ---
 apiVersion: v1
@@ -20,6 +25,9 @@ data:
   schedule: 0 0,12 * * *
 kind: ConfigMap
 metadata:
+  annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
   name: cockroach.backup.config-mch5d9m542
 ---
 apiVersion: v1
@@ -75,12 +83,17 @@ data:
     EOF
 kind: ConfigMap
 metadata:
+  annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
   name: cockroachdb-scripts
 ---
 apiVersion: v1
 kind: Service
 metadata:
   annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
     prometheus.io/path: _status/vars
     prometheus.io/port: "8001"
     prometheus.io/scrape: "true"
@@ -107,6 +120,9 @@ spec:
 apiVersion: v1
 kind: Service
 metadata:
+  annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
   labels:
     app: cockroachdb
   name: cockroachdb-proxy
@@ -128,10 +144,10 @@ apiVersion: apps/v1
 kind: Deployment
 metadata:
   annotations:
-    app.uw.systems/description: Used to connect and query the cockroachdb databases.
+    app.uw.systems/description: Cockroachdb instance used to store ...
     app.uw.systems/repos: https://github.com/utilitywarehouse/shared-kustomize-bases/cockroachdb
     app.uw.systems/tags.oss: "true"
-    app.uw.systems/tier: tier_4
+    app.uw.systems/tier: tier_1
     secret.reloader.stakater.com/reload: cockroachdb.client.root
   name: cockroachdb-client
 spec:
@@ -141,6 +157,9 @@ spec:
       app: cockroachdb-client
   template:
     metadata:
+      annotations:
+        app.uw.systems/description: Cockroachdb instance used to store ...
+        app.uw.systems/tier: tier_1
       labels:
         app: cockroachdb-client
       name: cockroachdb-client
@@ -199,10 +218,10 @@ apiVersion: apps/v1
 kind: StatefulSet
 metadata:
   annotations:
-    app.uw.systems/description: Used to store data projections of events in multiple
-      services.
+    app.uw.systems/description: Cockroachdb instance used to store ...
     app.uw.systems/repos: https://github.com/utilitywarehouse/shared-kustomize-bases/cockroachdb
     app.uw.systems/tags.oss: "true"
+    app.uw.systems/tier: tier_1
     secret.reloader.stakater.com/reload: cockroachdb.node
   labels:
     app: cockroachdb
@@ -217,6 +236,7 @@ spec:
   template:
     metadata:
       annotations:
+        app.uw.systems/description: Cockroachdb instance used to store ...
         app.uw.systems/tier: tier_1
         uw.systems/kyverno-inject-sidecar-request: vault-sidecar-aws
       labels:
@@ -324,6 +344,9 @@ spec:
 apiVersion: policy/v1
 kind: PodDisruptionBudget
 metadata:
+  annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
   labels:
     app: cockroachdb
   name: cockroachdb-budget
@@ -336,10 +359,17 @@ spec:
 apiVersion: batch/v1
 kind: Job
 metadata:
+  annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
   name: cockroach-backup-init
 spec:
   backoffLimit: 10
   template:
+    metadata:
+      annotations:
+        app.uw.systems/description: Cockroachdb instance used to store ...
+        app.uw.systems/tier: tier_1
     spec:
       containers:
       - command:
@@ -407,11 +437,18 @@ spec:
 apiVersion: batch/v1
 kind: Job
 metadata:
+  annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
   labels:
     app: cockroachdb
   name: cockroach-init
 spec:
   template:
+    metadata:
+      annotations:
+        app.uw.systems/description: Cockroachdb instance used to store ...
+        app.uw.systems/tier: tier_1
     spec:
       containers:
       - command:
@@ -455,9 +492,85 @@ spec:
               name: cockroachdb.client.root
   ttlSecondsAfterFinished: 600
 ---
+apiVersion: batch/v1
+kind: Job
+metadata:
+  annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
+  labels:
+    app: cockroach-db-init
+    version: v1
+  name: example-database-cockroach-db-init
+spec:
+  template:
+    metadata:
+      annotations:
+        app.uw.systems/description: Cockroachdb instance used to store ...
+        app.uw.systems/tier: tier_1
+    spec:
+      containers:
+      - command:
+        - /bin/bash
+        - /opt/scripts/user-schema-bootstrap.sh
+        env:
+        - name: DB_NAME
+          value: example_database
+        - name: DB_USER
+          value: example_user
+        - name: COCKROACH_HOST
+          valueFrom:
+            configMapKeyRef:
+              key: cockroach.host
+              name: cockroach-g92fdb9h27
+        - name: COCKROACH_PORT
+          valueFrom:
+            configMapKeyRef:
+              key: cockroach.port
+              name: cockroach-g92fdb9h27
+        - name: COCKROACH_CERTS_DIR
+          value: /cockroach-certs/
+        image: cockroachdb/cockroach:v23.2.2
+        imagePullPolicy: IfNotPresent
+        name: db-init
+        resources:
+          limits:
+            cpu: 1
+            memory: 512Mi
+          requests:
+            cpu: 0
+            memory: 128Mi
+        volumeMounts:
+        - mountPath: /cockroach-certs
+          name: client-certs
+        - mountPath: /opt/scripts
+          name: cockroachdb-scripts
+          readOnly: true
+      restartPolicy: OnFailure
+      volumes:
+      - name: client-certs
+        projected:
+          defaultMode: 256
+          sources:
+          - secret:
+              items:
+              - key: ca.crt
+                path: ca.crt
+              - key: tls.crt
+                path: client.root.crt
+              - key: tls.key
+                path: client.root.key
+              name: cockroachdb.client.root
+      - configMap:
+          name: cockroachdb-scripts
+        name: cockroachdb-scripts
+---
 apiVersion: cert-manager.io/v1
 kind: Certificate
 metadata:
+  annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
   name: client
 spec:
   commonName: root
@@ -471,6 +584,9 @@ spec:
 apiVersion: cert-manager.io/v1
 kind: Certificate
 metadata:
+  annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
   name: cockroachdb-ca
 spec:
   commonName: cockroachdb-ca
@@ -486,6 +602,25 @@ spec:
 apiVersion: cert-manager.io/v1
 kind: Certificate
 metadata:
+  annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
+  name: example-database-cockroach-client
+spec:
+  commonName: example_user
+  issuerRef:
+    kind: Issuer
+    name: cockroachdb-ca-issuer
+  secretName: cockroachdb.client.example-user
+  usages:
+  - client auth
+---
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
   name: node
 spec:
   commonName: node
@@ -496,6 +631,7 @@ spec:
   - cockroachdb-0.cockroachdb.<your namespace here>.svc.cluster.local
   - cockroachdb-1.cockroachdb.<your namespace here>.svc.cluster.local
   - cockroachdb-2.cockroachdb.<your namespace here>.svc.cluster.local
+  - cockroachdb-proxy
   issuerRef:
     kind: Issuer
     name: cockroachdb-ca-issuer
@@ -507,6 +643,9 @@ spec:
 apiVersion: cert-manager.io/v1
 kind: Issuer
 metadata:
+  annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
   name: cockroachdb-ca-issuer
 spec:
   ca:
@@ -515,6 +654,9 @@ spec:
 apiVersion: cert-manager.io/v1
 kind: Issuer
 metadata:
+  annotations:
+    app.uw.systems/description: Cockroachdb instance used to store ...
+    app.uw.systems/tier: tier_1
   name: cockroachdb-selfsigned-issuer
 spec:
   selfSigned: {}
diff --git a/built-manifests/cockroachdb/manifests-cert-manager/db-init/manifests.yaml b/built-manifests/cockroachdb/manifests-cert-manager/db-init/manifests.yaml
new file mode 100644
index 0000000..21999bc
--- /dev/null
+++ b/built-manifests/cockroachdb/manifests-cert-manager/db-init/manifests.yaml
@@ -0,0 +1,78 @@
+apiVersion: batch/v1
+kind: Job
+metadata:
+  labels:
+    app: cockroach-db-init
+    version: v1
+  name: cockroach-db-init
+spec:
+  template:
+    spec:
+      containers:
+      - command:
+        - /bin/bash
+        - /opt/scripts/user-schema-bootstrap.sh
+        env:
+        - name: COCKROACH_HOST
+          valueFrom:
+            configMapKeyRef:
+              key: cockroach.host
+              name: cockroach
+        - name: COCKROACH_PORT
+          valueFrom:
+            configMapKeyRef:
+              key: cockroach.port
+              name: cockroach
+        - name: COCKROACH_CERTS_DIR
+          value: /cockroach-certs/
+        - name: DB_NAME
+          value: default
+        - name: DB_USER
+          value: default
+        image: cockroachdb/cockroach:v23.2.2
+        imagePullPolicy: IfNotPresent
+        name: db-init
+        resources:
+          limits:
+            cpu: 1
+            memory: 512Mi
+          requests:
+            cpu: 0
+            memory: 128Mi
+        volumeMounts:
+        - mountPath: /cockroach-certs
+          name: client-certs
+        - mountPath: /opt/scripts
+          name: cockroachdb-scripts
+          readOnly: true
+      restartPolicy: OnFailure
+      volumes:
+      - name: client-certs
+        projected:
+          defaultMode: 256
+          sources:
+          - secret:
+              items:
+              - key: ca.crt
+                path: ca.crt
+              - key: tls.crt
+                path: client.root.crt
+              - key: tls.key
+                path: client.root.key
+              name: cockroachdb.client.root
+      - configMap:
+          name: cockroachdb-scripts
+        name: cockroachdb-scripts
+---
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: cockroach-client
+spec:
+  commonName: userName
+  issuerRef:
+    kind: Issuer
+    name: cockroachdb-ca-issuer
+  secretName: cockroachdb.client.userName
+  usages:
+  - client auth
diff --git a/root-manifests/cockroachdb/manifests-cert-manager/manifests.yaml b/built-manifests/cockroachdb/manifests-cert-manager/manifests.yaml
index 4d1ba3e..d0e7ea6 100644
--- a/root-manifests/cockroachdb/manifests-cert-manager/manifests.yaml
+++ b/built-manifests/cockroachdb/manifests-cert-manager/manifests.yaml
@@ -27,28 +27,16 @@ data:
           WITH SCHEDULE OPTIONS ignore_existing_backups;
       COMMIT;
     EOF
-  user-schema-bootstrap.sh: |
-    #!/bin/bash
-    set -e
-
-    if [ -z "$DB_USER" ]; then
-        echo "DB_USER envvar is not set"
-        exit 1
-    fi
-
-    if [ -z "$DB_NAME" ]; then
-        echo "DB_NAME envvar not set"
-        exit 1
-    fi
-
-    SQL_CMD="/cockroach/cockroach -d system sql" > /dev/null
-
-    $SQL_CMD << EOF
-      CREATE USER IF NOT EXISTS $DB_USER;
-      CREATE DATABASE IF NOT EXISTS $DB_NAME;
-      GRANT ALL ON DATABASE $DB_NAME TO $DB_USER;
-      GRANT ALL ON TABLE $DB_NAME.* TO $DB_USER;
-    EOF
+  user-schema-bootstrap.sh: "#!/bin/bash\nset -e\n\nif [ -z \"$DB_USER\" ]; then\n
+    \   echo \"DB_USER envvar is not set\"\n    exit 1\nfi\n\nif [ -z \"$DB_NAME\"
+    ]; then\n    echo \"DB_NAME envvar not set\"\n    exit 1\nfi\n\nSQL_CMD=\"/cockroach/cockroach
+    -d system sql\" > /dev/null\n\n# try creating user, database, grant database permissions
+    to user\n$SQL_CMD << EOF\n  CREATE USER IF NOT EXISTS $DB_USER;\n  CREATE DATABASE
+    IF NOT EXISTS $DB_NAME;\n  GRANT ALL ON DATABASE $DB_NAME TO $DB_USER;\nEOF\n\n#
+    If database already exists and contains tables, grant access to them for user.
+    \n# This command will fail for freshly initialized databases.\n$SQL_CMD --execute
+    \"GRANT ALL ON TABLE $DB_NAME.* TO $DB_USER;\" 2>1 || echo \"no tables in database
+    yet\"\n"
 kind: ConfigMap
 metadata:
   name: cockroachdb-scripts
@@ -390,14 +378,11 @@ spec:
       - command:
         - /bin/bash
         - -c
-        - /cockroach/cockroach init --certs-dir=/cockroach/cockroach-certs --port=26357
-          2>&1 | grep 'initialized'
+        - /cockroach/cockroach init --certs-dir=/cockroach/cockroach-certs --host=$(COCKROACH_INIT_HOST)
+          --port=26357 2>&1 | grep 'initialized'
         env:
-        - name: COCKROACH_HOST
-          valueFrom:
-            configMapKeyRef:
-              key: cockroach.host
-              name: cockroach
+        - name: COCKROACH_INIT_HOST
+          value: cockroachdb-0.cockroachdb
         image: cockroachdb/cockroach:v23.2.2
         imagePullPolicy: IfNotPresent
         name: cluster-init

=============================================
k8s objects: 6 to add, 0 to destroy
22 changed hunks

# similar kustomize file should exist for every database/ user pair

resources:
- github.com/utilitywarehouse/shared-kustomize-bases//cockroachdb/manifests-cert-manager/db-init?ref=DENA-603

Choose a reason for hiding this comment

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

We'll need to update this ref once merged and released, right?

@MarcinGinszt MarcinGinszt merged commit 8bac405 into main Jul 22, 2024
2 checks passed
@MarcinGinszt MarcinGinszt deleted the DENA-603 branch July 22, 2024 07:08
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