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

Sunset keycloak from the project #1134

Merged
merged 15 commits into from
Dec 7, 2023
Merged

Sunset keycloak from the project #1134

merged 15 commits into from
Dec 7, 2023

Conversation

Tansito
Copy link
Member

@Tansito Tansito commented Nov 30, 2023

Summary

This pull request continues the work of #1132 cleaning references to the project and logic that it remained there:

  • Removed Social Account creation
  • Removed keycloak from helm
  • Introduced a new parameter to be able to modify the mocked token
  • Updated READMEs with the new changes

@Tansito
Copy link
Member Author

Tansito commented Nov 30, 2023

I didn't test it yet in kubernetes, that's why I left it in draft. I need to solve a couple of things with @psschwei and as soon as I finish I return to this.

Copy link
Collaborator

@psschwei psschwei left a comment

Choose a reason for hiding this comment

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

Will the changes above, can confirm that the postgres pod is running, and the 5432 port is open on that pod, but continuing to get this in the gateway logs:

http://postgresql:5432 - no response
waiting for myservice

charts/quantum-serverless/values.yaml Show resolved Hide resolved
Tansito and others added 2 commits December 1, 2023 15:05
Co-authored-by: Paul Schweigert <paul@paulschweigert.com>
Co-authored-by: Paul Schweigert <paul@paulschweigert.com>
@Tansito
Copy link
Member Author

Tansito commented Dec 1, 2023

Maybe DATABASE_HOST is not correct?

@psschwei
Copy link
Collaborator

psschwei commented Dec 1, 2023

I think the host is ok:

$ testpod
Running psschwei/nmap:latest pod on default namespace
If you don't see a command prompt, try pressing enter.
root@paultest:/# nmap -Pn -p 5432 postgresql
Starting Nmap 7.70 ( https://nmap.org ) at 2023-12-01 20:25 UTC
Nmap scan report for postgresql (10.96.109.241)
Host is up (0.000044s latency).
rDNS record for 10.96.109.241: postgresql.default.svc.cluster.local

PORT     STATE SERVICE
5432/tcp open  postgresql

Nmap done: 1 IP address (1 host up) scanned in 0.25 seconds

For whatever reason, the db seems to be non-responsive

@akihikokuroda
Copy link
Collaborator

With these changes, all pods come up.

diff --git a/charts/quantum-serverless/charts/gateway/templates/deployment.yaml b/charts/quantum-serverless/charts/gateway/templates/deployment.yaml
index b8c16268..5636c5d9 100644
--- a/charts/quantum-serverless/charts/gateway/templates/deployment.yaml
+++ b/charts/quantum-serverless/charts/gateway/templates/deployment.yaml
@@ -30,7 +30,7 @@ spec:
       initContainers:
         - name: waitpostresql
           image: actions/pg_isready
-          command: ['sh', '-c', 'until pg_isready -U ${DATABASE_USER} -d "dbname=${DATABASE_NAME}" -h ${DATABASE_HOST} -p ${DATABASE_PORT}; do echo waiting for myservice; sleep 2; done']
+          command: ['sh', '-c', 'until pg_isready -U ${DATABASE_USER} -d ${DATABASE_NAME} -h ${DATABASE_HOST} -p ${DATABASE_PORT}; do echo waiting for myservice; sleep 2; done']
           volumeMounts:
           - name: gateway-pv-storage
             mountPath: /usr/src/app/media/
@@ -167,7 +167,7 @@ spec:
             - name: DATABASE_PASSWORD
               valueFrom:
                 secretKeyRef:
-                  name: {{ .Values.secrets.servicePsql.name }}
+                  name: {{ .Values.secrets.servicePsql.databaseSecretName }}
                   key: {{ .Values.secrets.servicePsql.key.databasePassword }}
       {{- with .Values.nodeSelector }}
       nodeSelector:
@@ -280,7 +280,7 @@ spec:
             - name: DATABASE_PASSWORD
               valueFrom:
                 secretKeyRef:
-                  name: {{ .Values.secrets.servicePsql.name }}
+                  name: {{ .Values.secrets.servicePsql.databaseSecretName }}
                   key: {{ .Values.secrets.servicePsql.key.databasePassword }}
             - name: RAY_KUBERAY_NAMESPACE
               value: {{ .Release.Namespace }}
diff --git a/charts/quantum-serverless/charts/gateway/values.yaml b/charts/quantum-serverless/charts/gateway/values.yaml
index f792862f..84e0fc74 100644
--- a/charts/quantum-serverless/charts/gateway/values.yaml
+++ b/charts/quantum-serverless/charts/gateway/values.yaml
@@ -57,9 +57,10 @@ secrets:
   servicePsql:
     create: true
     name: service-psql-binding
+    databaseSecretName: postgresql
     key:
       databaseName: database-name
-      databasePassword: database-password
+      databasePassword: password
       host: database-host
       port: database-port
       userName: user-name
diff --git a/charts/quantum-serverless/values.yaml b/charts/quantum-serverless/values.yaml
index eabcc48a..708a57bf 100644
--- a/charts/quantum-serverless/values.yaml
+++ b/charts/quantum-serverless/values.yaml
@@ -73,18 +73,18 @@ gateway:
       name: service-psql-binding
       key:
         databaseName: database-name
-        databasePassword: database-password
+        databasePassword: password
         host: database-host
         port: database-port
         userName: user-name
       value:
         databaseName: serverlessdb
         databasePassword: serverlesspassword
-        host: "http://postgresql"
+        host: "postgresql"
         port: 5432
         userName: serverlessuser
     superuser:
-      create: false
+      create: true
       name: gateway-superuser
       key:
         name: name

@psschwei
Copy link
Collaborator

psschwei commented Dec 4, 2023

Aki's patch worked for me (was able to run the "running program" notebook successfully as well)

@Tansito
Copy link
Member Author

Tansito commented Dec 5, 2023

@akihikokuroda , @psschwei for me it didn't work but I think my problem is another one. Related to your changes I see some problems:

  • Probably this change is not needed or may be needed to update too for the scheduler. So if the scheduler is working we can see to avoid the change:
-          command: ['sh', '-c', 'until pg_isready -U ${DATABASE_USER} -d "dbname=${DATABASE_NAME}" -h ${DATABASE_HOST} -p ${DATABASE_PORT}; do echo waiting for myservice; sleep 2; done']
+          command: ['sh', '-c', 'until pg_isready -U ${DATABASE_USER} -d ${DATABASE_NAME} -h ${DATABASE_HOST} -p ${DATABASE_PORT}; do echo waiting for myservice; sleep 2; done']
  • Why do you think this change is needed? Because I see that the secret is configured as postgresql but we are not creating that secret. And it would have implications in production too:
-                  name: {{ .Values.secrets.servicePsql.name }}
+                  name: {{ .Values.secrets.servicePsql.databaseSecretName }}
  • Similar to my previous comment. We are not setting password as password for the database anywhere. How does this work?
-      databasePassword: database-password
+      databasePassword: password

Sorry I'm just trying to figure out my problem in the computer to test too all of this.

@akihikokuroda
Copy link
Collaborator

-          command: ['sh', '-c', 'until pg_isready -U ${DATABASE_USER} -d "dbname=${DATABASE_NAME}" -h ${DATABASE_HOST} -p ${DATABASE_PORT}; do echo waiting for myservice; sleep 2; done']
+          command: ['sh', '-c', 'until pg_isready -U ${DATABASE_USER} -d ${DATABASE_NAME} -h ${DATABASE_HOST} -p ${DATABASE_PORT}; do echo waiting for myservice; sleep 2; done']

I'm not sure why but I tried pg_isready in the gateway container and found this works.

-                  name: {{ .Values.secrets.servicePsql.name }}
+                  name: {{ .Values.secrets.servicePsql.databaseSecretName }}

The default password is set in postgrespl secret by the helm install,

-      databasePassword: database-password
+      databasePassword: password

and the key of the password is password in the postgrespl secret.

@psschwei
Copy link
Collaborator

psschwei commented Dec 5, 2023

One trick on applying the Aki's changes: copy them into a file called patch.diff and then git apply --ignore-space-change --ignore-whitespace patch.diff

@Tansito
Copy link
Member Author

Tansito commented Dec 5, 2023

Oooh, ok I understood @akihikokuroda , thank you. I'm going to analyze that well and propose you a change to try no to affect to much to what we had right now in production.

One trick on applying the Aki's changes: copy them into a file called patch.diff and then git apply --ignore-space-change --ignore-whitespace patch.diff

Yes yes, my problem is being with the cluster. I couldn't make it work yet in my new computer 😢

@psschwei
Copy link
Collaborator

psschwei commented Dec 5, 2023

my problem is being with the cluster. I couldn't make it work yet in my new computer

Is it still the DB check not showing as ready, or something else?

@Tansito
Copy link
Member Author

Tansito commented Dec 5, 2023

Is it still the DB check not showing as ready, or something else?

This is my output when I try to install it.

NAME                                                        READY   STATUS                       RESTARTS   AGE
...
kuberay-operator-6594cb6695-ffw85                           1/1     Running                      0          45s
postgresql-0                                                1/1     Running                      0          45s
gateway-76685f54bd-th95q                                    0/1     CreateContainerConfigError   0          45s
scheduler-5d94cdfdb9-xp2gj                                  0/1     CreateContainerConfigError   0          45s

And the error that I get from the gateway for example is:

Events:
  Type     Reason            Age                From               Message
  ----     ------            ----               ----               -------
  Warning  FailedScheduling  58s                default-scheduler  0/1 nodes are available: pod has unbound immediate PersistentVolumeClaims. preemption: 0/1 nodes are available: 1 No preemption victims found for incoming pod..

So the database I understand that it is being created but I'm just taking a look.

@psschwei
Copy link
Collaborator

psschwei commented Dec 5, 2023

pod has unbound immediate PersistentVolumeClaims

looks like you're missing the PV to go with the PVC...

@Tansito
Copy link
Member Author

Tansito commented Dec 5, 2023

It's giving me random errors to be honest:

Error: an error occurred while uninstalling the release. original install error: Get "https://127.0.0.1:6443/api/v1/namespaces/quantum-serverless/persistentvolumeclaims/gateway-claim": http2: client connection lost: warning: Hook pre-delete quantum-serverless/templates/predelete.yaml failed: 1 error occurred:
        * Internal error occurred: resource quota evaluation timed out


helm.go:84: [debug] 1 error occurred:
        * Internal error occurred: resource quota evaluation timed out

@akihikokuroda
Copy link
Collaborator

FYI: after chart install my system has:

(env-quantum-serverless) (base) akihikokuroda@Akihikos-MacBook-Pro quantum-serverless % kubectl get pv
NAME                                       CAPACITY   ACCESS MODES   RECLAIM POLICY   STATUS   CLAIM                       STORAGECLASS   REASON   AGE
gateway-volume                             2Gi        RWX            Retain           Bound    default/gateway-claim       manual                  12m
pvc-2f31a2f0-eddd-42aa-b50e-3ecad4b334a9   8Gi        RWO            Delete           Bound    default/data-postgresql-0   local-path              12m
(env-quantum-serverless) (base) akihikokuroda@Akihikos-MacBook-Pro quantum-serverless % kubectl get pvc
NAME                STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS   AGE
gateway-claim       Bound    gateway-volume                             2Gi        RWX            manual         13m
data-postgresql-0   Bound    pvc-2f31a2f0-eddd-42aa-b50e-3ecad4b334a9   8Gi        RWO            local-path     13m

@Tansito
Copy link
Member Author

Tansito commented Dec 6, 2023

I took the idea from @akihikokuroda and I simplified it a little bit reusing our secret instead to use the secret from default. This way I think it affect less to our current configuration and we add less noise. Let me know what you think Aki, @psschwei: 0a6936f

@Tansito Tansito marked this pull request as ready for review December 6, 2023 15:44
@akihikokuroda
Copy link
Collaborator

I'm getting

psycopg2.OperationalError: connection to server at "postgresql" (10.43.238.230), port 5432 failed: FATAL:  password authentication failed for user "serverlessuser"

I may be messing up something.

@akihikokuroda
Copy link
Collaborator

I reinstalled clean but I'm still getting the error.

@Tansito
Copy link
Member Author

Tansito commented Dec 6, 2023

oh, you're right, I think I removed something important. Let me take a look 🙏

@Tansito
Copy link
Member Author

Tansito commented Dec 6, 2023

Ok, I discovered what it was happening. The image is not getting the correct value for the password:

I have no name!@postgresql-0:/$ echo $POSTGRES_USER
serverlessuser
I have no name!@postgresql-0:/$ echo $POSTGRES_PASSWORD
IbVSGP1w00

I'm going to return to the old configuration I think 😅

@Tansito
Copy link
Member Author

Tansito commented Dec 6, 2023

Now it's working for me @akihikokuroda. I changed it to plain text seeing that reusing secrets seems not work. Not important anyway thinking that this is for local development anyway.

Copy link
Collaborator

@akihikokuroda akihikokuroda left a comment

Choose a reason for hiding this comment

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

It works for me. Thanks!

@Tansito
Copy link
Member Author

Tansito commented Dec 7, 2023

I want to wait for @psschwei to see if it works for him too in kind before merge it. The tests passed but just to be sure.

Copy link
Collaborator

@psschwei psschwei left a comment

Choose a reason for hiding this comment

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

works for me too

@Tansito Tansito merged commit 481d5d8 into main Dec 7, 2023
21 checks passed
@Tansito Tansito deleted the finish-to-remove-keycloak branch December 7, 2023 15:15
@IceKhan13 IceKhan13 mentioned this pull request Dec 12, 2023
3 tasks
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