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

fix: ensure ES/OS cluster setup for shared usage #73

Merged
merged 10 commits into from
Jul 31, 2024

Conversation

gabor-boros
Copy link
Contributor

@gabor-boros gabor-boros commented May 3, 2024

Description

The current configuration had several smaller misconfigurations and inconsistent naming, which made it challenging to configure the cluster properly. Also, there were unnecessary configuration steps that I got rid of.

This pull request refactors the way we set up and configure both Open Search and Elasticsearch for shared usage; also

Dependencies

Testing

  1. Create a cluster
  2. Install the harmony helm-chart with ES enabled
  3. Create a new user for the instance with the create_elasticsearch_user command
  4. Install this version of the tutor-forum plugin
  5. Deploy the instance to the cluster
  6. Create a new course
  7. Go and start some discussions

Screenshots

Screenshot 2024-07-30 at 19 16 41

Notes

I have tested that the Elasticsearch cluster is spinning up correctly, the user is created, the instance is deployed with tutor-forum installed. Also, the instance seems to work fine. However, I did not test the Open Search cluster -- though it should work.

@gabor-boros gabor-boros added enhancement Relates to new features or improvements to existing features waiting for eng review PR is ready for review. Review and merge it, or suggest changes. labels May 3, 2024
@gabor-boros gabor-boros self-assigned this May 3, 2024
@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U labels May 3, 2024
@openedx-webhooks
Copy link

openedx-webhooks commented May 3, 2024

Thanks for the pull request, @gabor-boros!

What's next?

Please work through the following steps to get your changes ready for engineering review:

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @openedx/openedx-k8s-harmony-maintainers. Tag them in a comment and let them know that your changes are ready for review.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@openedx-webhooks
Copy link

Thanks for the pull request, @gabor-boros! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

1 similar comment
@openedx-webhooks
Copy link

Thanks for the pull request, @gabor-boros! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@mphilbrick211 mphilbrick211 requested a review from a team May 3, 2024 20:15
@cmltaWt0
Copy link
Contributor

@gabor-boros Just want to mention that I'm going to test OpenSearch cluster. Sorry for such delay in doing the review.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@cmltaWt0
Copy link
Contributor

cmltaWt0 commented Jun 12, 2024

Testing notes

Steps

  1. Enable OpenSearch
  2. Install Help chart
  3. Create opensearch user
  4. Create openedx-01 namespace manually
  5. Apply a secret to openedx-01 namespace
  6. Configure openedx instance
  7. Run and Init.

I'm not able to get the reindex_content_library command work yet.

./manage.py lms reindex_content_library
...
File "/openedx/venv/lib/python3.8/site-packages/elasticsearch/connection/http_urllib3.py", line 263, in perform_request
    raise SSLError("N/A", str(e), e)
elasticsearch.exceptions.SSLError: ConnectionError([SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1131)) caused by: SSLError([SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1131))

However I can see the cert in django settings:

>>> settings.ELASTICSEARCH_CA_CERT
'-----BEGIN CERTIFICATE-----\XXXXXXX
XXXXXXXXXX
XXXXXXXXXX
XXXXXXXXXX\n-----END CERTIFICATE-----\n'
>>> settings.ELASTIC_SEARCH_INDEX_PREFIX
'openedx-01-einv-'
>>> settings.ELASTIC_SEARCH_CONFIG
[{'use_ssl': True, 'host': 'harmony-search-cluster.harmony.svc.cluster.local', 'port': 9200, 'http_auth': 'openedx-01:njN2pew2NfHALTQl3zii2cbK', 'ssl_context': <ssl.SSLContext object at 0xffffaf22b940>}]

Am I right that the current PR do not implement the mounting process of the cert into forum? Maybe we have to update to testing instruction because it doesn't work for me following the instructions directly?
Forum obviously failing to verify the cert.

2024/06/12 12:24:13 Waiting for https://openedx-01:*************@harmony-search-cluster.harmony.svc.cluster.local:9200: Get "https://openedx-01:***@harmony-search-cluster.harmony.svc.cluster.local:9200": tls: failed to verify certificate: x509: certificate signed by unknown authority.

In general everything looks great, I can create a cluster, create an opensearch user, copy cert to INSTANCE_NAMESPACE and run everything. Just need to understand how to test it properly.

@gabor-boros
Copy link
Contributor Author

@cmltaWt0 Are you using this version of the forum (which is listed in the dependencies)? If yes, the forum pod should get the ELASTICSEARCH_CA_PATH as a docker build parameter from the config.yaml of Tutor and connect to the ES server accordingly. If that's not the case, please let me know as I may forgot something from the test instructions.

@cmltaWt0
Copy link
Contributor

cmltaWt0 commented Jun 12, 2024

@cmltaWt0 Are you using this version of the forum (which is listed in the dependencies)? If yes, the forum pod should get the ELASTICSEARCH_CA_PATH as a docker build parameter from the config.yaml of Tutor and connect to the ES server accordingly. If that's not the case, please let me know as I may forgot something from the test instructions.

Yes. Using your branch:

❯ git -C tutor-forum branch
* gabor/wait-for-search-server-with-ca

And I have the volume definition in the forum deployment spec:

spec:
      volumes:
        - name: search-cluster-certs
          secret:
            secretName: search-cluster-certificates
            defaultMode: 420
      containers:
        - name: forum
          image: docker.io/overhangio/openedx-forum:17.0.0
          ports:
            - containerPort: 4567
              protocol: TCP
          env:
            - name: SEARCH_SERVER
              value: >-
                https://openedx-01:njN2pew2NfHALTQl3zii2cbK@harmony-search-cluster.harmony.svc.cluster.local:9200
            - name: MONGODB_AUTH
            - name: MONGODB_HOST
              value: mongodb
            - name: MONGODB_PORT
              value: '27017'
            - name: MONGODB_DATABASE
              value: cs_comments_service
            - name: MONGOID_AUTH_SOURCE
              value: admin
            - name: MONGOID_AUTH_MECH
            - name: MONGOID_USE_SSL
              value: 'false'
            - name: ELASTICSEARCH_INDEX_PREFIX
              value: openedx-01-einv-
            - name: ELASTICSEARCH_CA_PATH
              value: /etc/ssl/certs/search-cluster.pem
          resources: {}
          volumeMounts:
            - name: search-cluster-certs
              mountPath: /etc/ssl/certs/search-cluster.pem
              subPath: ca.crt
          terminationMessagePath: /dev/termination-log
          terminationMessagePolicy: File
          imagePullPolicy: IfNotPresent

So we should be fine - ca.crt is mounted to /etc/ssl/certs/search-cluster.pem.
Need to investigate the cause of the error msg.

UPD: OpenSearch has the same ca.crt too. So the idea is that OpenSearch works differently.
I will test it from scratch with Elasticsearch, not with OpenSearch.
Am I correct in understanding that no additional actions are required beyond those described in the instructions?

@cmltaWt0
Copy link
Contributor

cmltaWt0 commented Jun 25, 2024

@gabor-boros I've tested the flow from a scratch for ElasticSearch and it works perfectly!
It's good for me to merge this but add a note to README that work with OpenSearch is not confirmed yet. I'll re-test the OS separately and try to understand why it doesn't work for me.

P.S. Also we need to rebase the source branch on top of main.

@mphilbrick211
Copy link

@gabor-boros I've tested the flow from a scratch for ElasticSearch and it works perfectly! It's good for me to merge this but add a note to README that work with OpenSearch is not confirmed yet. I'll re-test the OS separately and try to understand why it doesn't work for me.

P.S. Also we need to rebase the source branch on top of main.

@gabor-boros - friendly follow-up on this!

@gabor-boros
Copy link
Contributor Author

@mphilbrick211 We are still waiting on a dependent tutor-forum PR. This PR is not forgotten

@mphilbrick211 mphilbrick211 removed the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Jul 25, 2024
@gabor-boros gabor-boros force-pushed the gabor/refactor-search-usage branch 2 times, most recently from 008d257 to fc1935a Compare July 26, 2024 10:46
@gabor-boros
Copy link
Contributor Author

@felipemontoya / @cmltaWt0 I would like to have another review/approve from you as I pushed some changes:

Copy link
Contributor

@cmltaWt0 cmltaWt0 left a comment

Choose a reason for hiding this comment

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

Looks great. Just a one typo.

@gabor-boros gabor-boros merged commit 8767df2 into main Jul 31, 2024
1 of 2 checks passed
@gabor-boros gabor-boros deleted the gabor/refactor-search-usage branch July 31, 2024 09:21
@openedx-webhooks
Copy link

@gabor-boros 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Relates to new features or improvements to existing features open-source-contribution PR author is not from Axim or 2U
Projects
Status: Done
5 participants