openedx / tutor-contrib-aspects

The Open Analytics Reference System - Tutor plugin
Apache License 2.0
10 stars 14 forks source link

ch_report Authentication failed #893

Closed CodeWithEmad closed 2 months ago

CodeWithEmad commented 3 months ago

I set up a new aspect plugin, on one of our orgs. but ch_report can't communicate with clickhouse.

Screenshot from 2024-07-13 10-48-25

Aspect version: 1.0.2 Tutor version: 17.0.5

Update

After a bit of digging, I found out that the generated ASPECTS_CLICKHOUSE_REPORT_PASSWORD is not used in the init jobs, and changing the password to the value used in .ci/config,yml done the trick. :) I'll open a PR if no one has done anything about this.

Ian2012 commented 3 months ago

This is already fixed but I'm not sure if on 1.0.3. Will review on monday

Ian2012 commented 3 months ago

Hi @CodeWithEmad. This was fixed and tested on dev and k8s environments: https://github.com/openedx/tutor-contrib-aspects/commit/4918d068506e16644790f9b85b6eee98c6a6b671

Can you leave more information of the environment you are running?

CodeWithEmad commented 3 months ago

Thanks @Ian2012 for looking it up. https://github.com/openedx/tutor-contrib-aspects/commit/4918d068506e16644790f9b85b6eee98c6a6b671 looks more like fixing superset connection. If you're positive that ch_report's password issue has been fixed, I'll close this one.

Can you leave more information of the environment you are running?

Aspect version: 1.0.2 Tutor version: 17.0.5

Ian2012 commented 3 months ago

@bmtcril Have you experienced anything like this?

@CodeWithEmad You could try to rerun init jobs and check that the value of ASPECTS_CLICKHOUSE_REPORT_PASSWORD matches what is on the jobs

bmtcril commented 3 months ago

I haven't seen this, and I think @saraburns1 recently did an environment rebuild from scratch and it worked ok. @CodeWithEmad how are you installing the Aspects plugin? PyPI, GitHhub? Are you running Tutor dev, local, or k8s?

I honestly don't know how this could happen unless Tutor was finding the wrong config.yml and running the whole thing out of .ci

CodeWithEmad commented 3 months ago

You could try to rerun init jobs and check that the value of ASPECTS_CLICKHOUSE_REPORT_PASSWORD matches what is on the jobs

Well, I ran aspects for 3 organizations, and in all 3, the ch_report couldn't connect to click house. changing the password fixed the issue,

how are you installing the Aspects plugin? Are you running Tutor dev, local, or k8s?

pip install 'tutor-contrib-aspects==1.0.2', in k8s mode.

bmtcril commented 3 months ago

I'm working on this now, and am definitely seeing some oddities in a local k8s. The superset init job is throwing a lot of these, which we don't get on local:

2024-07-17 20:34:40,409:DEBUG:superset.models.core:Database._get_sqla_engine(). Masked URL: clickhousedb+connect://ch_report:XXXXXXXXXX@clickhouse:8123/xapi
2024-07-17 20:34:40,489:DEBUG:superset.models.core:Database._get_sqla_engine(). Masked URL: mysql://superset:XXXXXXXXXX@mysql/superset

Possibly unrelated, but also having a problem with LMS connecting to mysql so I can't even authenticate to try Superset. I'll keep digging tomorrow.

CodeWithEmad commented 3 months ago

Thanks, @bmtcril

bmtcril commented 3 months ago

Ok, I was able to reproduce it and I think I've got it. We're baking the whole openedx-assets folder into the aspects-superset image now, which will include the .ci variables for database connections, etc. We just never updated the install instructions to build the aspects and aspects-superset images when running k8s. I believe that in dev/local those directories just get mounted over.

@Ian2012 that make sense to you?

Ian2012 commented 3 months ago

I don't think it does. We are running more than 4 Aspects installations k8s on different Open edX versions and the users are created with the newly generated values per config.yml file.

Rebuilding the image is not necessary as the assets connection string is overridden. The problem should be on the ClickHouse initialization. Can you share the steps you are following to run the initialization jobs?

bmtcril commented 3 months ago

Hmm. @CodeWithEmad where did you change the password to make ch_report work?

I did this and got into a situation where Superset had the .ci password for the OpenedX_Clickhouse user, while the correct password was used in the init script on ClickHouse, so it threw the error in the initial screenshot. To the best of my knowledge my repro steps were:

I can try a couple of other things, but obviously this is a time consuming process :)

CodeWithEmad commented 3 months ago

Thank you so much and sorry for the trouble @bmtcril. I executed to the click house pod, and inside the clickhouse-client, I ran :

ALTER USER 'ch_report' IDENTIFIED BY '6lVwUJ4p6SLosW86u0tRKWn8';

I forgot how I found out the ci password was set for the ch_report user. I'll be sure to refresh my memory.

Update I haven't worked much with aspects, so some of the explanations must be obvious:

inside superset deployment, I can't see any volume mounted to this path:

# superset deployment
          volumeMounts:
            - name: docker
              mountPath: /app/docker
            - name: pythonpath
              mountPath: /app/pythonpath
            - name: security
              mountPath: /app/security

probably building the superset image will put the new OpenedX_Clickhouse.yaml file in the image.

Also, in the older versions, I remember there was a superset assets volume, but it looks like it's been removed in the newer versions.

bmtcril commented 3 months ago

Thanks @CodeWithEmad , that helps a lot. We had to move away from using configmaps for the Superset assets because they are too large. I think this supports the idea that we just need to update the install instructions to including building that image, and we should probably start a "troubleshooting" section of the docs to handle some of these errors that are likely to come up.

@Ian2012 that all sound right to you?

bmtcril commented 2 months ago

https://github.com/openedx/tutor-contrib-aspects/pull/904 should close this