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

ch_report Authentication failed #893

Closed
CodeWithEmad opened this issue Jul 13, 2024 · 14 comments · Fixed by #904
Closed

ch_report Authentication failed #893

CodeWithEmad opened this issue Jul 13, 2024 · 14 comments · Fixed by #904

Comments

@CodeWithEmad
Copy link
Member

CodeWithEmad commented Jul 13, 2024

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
Copy link
Contributor

Ian2012 commented Jul 14, 2024

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

@Ian2012
Copy link
Contributor

Ian2012 commented Jul 15, 2024

Hi @CodeWithEmad. This was fixed and tested on dev and k8s environments: 4918d06

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

@CodeWithEmad
Copy link
Member Author

Thanks @Ian2012 for looking it up. 4918d06 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
Copy link
Contributor

Ian2012 commented Jul 15, 2024

@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
Copy link
Contributor

bmtcril commented Jul 16, 2024

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
Copy link
Member Author

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
Copy link
Contributor

bmtcril commented Jul 17, 2024

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
Copy link
Member Author

Thanks, @bmtcril

@bmtcril
Copy link
Contributor

bmtcril commented Jul 18, 2024

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
Copy link
Contributor

Ian2012 commented Jul 18, 2024

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
Copy link
Contributor

bmtcril commented Jul 18, 2024

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:

  • Destroy my entire environment, delete data, env, all cached images, all Docker volumes, openedx k8s cluster
  • Recreate the k8s cluster
  • Install tutor-contrib-aspects @ main along on a recent tutor nightly
  • tutor images build openedx --no-cache
  • tutor images build mfe until it works, as usual
  • tutor k8s launch
  • Import a course, create a superuser
  • Log into LMS, log into Superset
    • I ran into another issue here with embedded dashboards looking for the wrong URL (local.edly.io:8088), haven't had time to look into it yet, maybe unrelated
  • Superset throws errors about ch_report
  • Check files on Superset image, see that openedx-assets/assets/databases/OpenedX_Clickhouse.yaml has the .ci password
  • tutor k8s stop
  • tutor images build aspects aspects-superset
  • tutor local do init -l aspects
  • Things work, aside from the embedded dashboards

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

@CodeWithEmad
Copy link
Member Author

CodeWithEmad commented Jul 18, 2024

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:

  • there's a configmap called superset-scripts, which runs python /app/pythonpath/create_assets.py
  • It led me to this path: /app/openedx-assets/assets/databases inside the pod
  • the content of the file was:
superset@superset-687f67c876-rmm99:/app/openedx-assets/assets/databases$ cat OpenedX_Clickhouse.yaml 
_file_name: OpenedX_Clickhouse.yaml
allow_ctas: false
allow_cvas: false
allow_dml: false
allow_file_upload: false
allow_run_async: false
cache_timeout: null
database_name: OpenedX Clickhouse
expose_in_sqllab: true
extra:
  allows_virtual_table_explore: true
  engine_params: {}
  metadata_params: {}
  schemas_allowed_for_file_upload: []
sqlalchemy_uri: 'clickhousedb+connect://ch_report:6lVwUJ4p6SLosW86u0tRKWn8@clickhouse:8123/xapi'
uuid: 21174b6c-4d40-4958-8161-d6c3cf5e77b6

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
Copy link
Contributor

bmtcril commented Jul 19, 2024

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
Copy link
Contributor

bmtcril commented Jul 22, 2024

#904 should close this

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 a pull request may close this issue.

3 participants