-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feanil/ubuntu 24.04 #35713
feanil/ubuntu 24.04 #35713
Conversation
a8e9207
to
b4a8466
Compare
@dianakhuang with both xmslec and lxml upgraded it looks like we've got everything passing. Is there any further testing you want to do before we update the testing and merge this? |
@feanil I think I'm okay with it as long as you're okay with me panic reverting it if it does break our deploys. |
@dianakhuang any way to test prior to it going out? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI & requirement changes look good.
Could you add more context in your commit / PR description? There are a lot of pieces to this upgrade story which I don't have straight in my head but maybe you do. Reading through https://openedx.atlassian.net/wiki/spaces/COMM/pages/4546199561/Mini-RCA+doc+ubuntu-latest+issues+on+edx-platform, I have somewhat of a sense what happened, but it's not clear to me what caused 2U to revert before and what/whether anything is different now.
.github/workflows/unit-tests.yml
Outdated
# We only need to test older versions of Mongo with modules that directly interface with Mongo (that is: xmodule.modulestore) | ||
# This code is left here as an example for future refernce in case we need to reduce the number of shards we're | ||
# testing but still have good confidence with older versions of mongo. We use Mongo 4.4 in the example. | ||
# | ||
# exclude: | ||
# - mongo-version: "4.4" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment needs updating
b4a8466
to
4cb4448
Compare
4cb4448
to
78f53ff
Compare
@kdmccormick this is ready for another review. |
Rather than doing the install ourselves and needing to keep the debion source URL up-to-date, use a community action to handle installing mongo.
libxmlsec and lxml need to be updated in lockstep and the version we had wouldn't work with Ubuntu 24.04 so unpinning lxml along with libxmlsec to see if that resolves the build and dependency issues.
Run `make upgrade-package package='lxml[html_clean]'` to update lxml and then `make compile-requirements`
78f53ff
to
5cb2984
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TY
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
2U Release Notice: This PR has been deployed to the edX production environment. |
Hey, we are getting |
@bradenmacdonald I was seeing that error on ubuntu 24.04 until I install both the newest version of lxml and xmlsec python packages and then I didn't see any issues. What version of ubuntu are you running for grove? And is the |
@feanil Ah, looks like we're on 20.04 because we pinned to an old tutor version to fix a mysql charset bug.
Yes, I assume if we upgrade to the latest Tutor nightly which is using 22.04 it may fix the issue. I will check. Or should we be using 24.04? |
22.04 should fix it, but as of the latest edx-platform version, 24.04 should work as well. Keep in mind that there are some changes to make codejail work correctly on the newer versions if you're running that service on the same boxes. |
I'll double check, but for me the latest nightly having ubuntu 22.04 doesn't work. |
Given that multiple people are seeing issues with it, perhaps it makes sense to revert this for now and I can try to re-produce the error locally rather than have everyone run into it. |
Revert PR is here: #35826 |
@bradenmacdonald @cmltaWt0 can either of you provide a stack trace of where this failure is happening? I'm trying to re-produce it on my local tutor-nightly and am unable to do it so far. |
FYI, after some digging, it looks like this might be an issue with uwsgi, xmlsec/python-xmlsec#320 so an issue on the tutor side not on the edx-platform side. I'm gonna see if I can fix it there quickly rather than rollback edx-platform but if others are opposed to this idea, please say so. |
Can you try image build with --no-cache option and see if it is still reproducing? In some cases, maybe due to cache, I had seen the build fail on some machines but when they would re-build with --no-cache, it would fix the issue. Second, the stack trace and the error location would certainly be helpful. During Ubuntu upgrade in Tutor, xmlsec error would come up during translation related commands execution. |
FYI, the following change in tutor fixed the issue for me: overhangio/tutor#1157 Given that this is an upstream issue, I'm not going to revert the change from edx-platform and hopefully we can fix forward. @DawoudSheraz I am building after a |
On Oct 9th the version of ubuntu that
ubuntu-latest
pointed to in github action workflows changed from Ubuntu 22.04 to Ubuntu 24.04 which resulted in failures of the edx-platform CI jobs due to a couple of different issues. A timeline of the series of events and errors can be found here.Issue 1: The mongo install/mirror URL was OS specific and did not get correctly updated to be resilient to the version of the OS changing.
Resolution: 5f2e853 updates the workflow to use a re-usable action which should prevent this issue from occuring in the future.
Issue 2: The version of libxmlsec and lxml that were constrained in edx-platform were not compatible with newer versions of the underlying libssl and libxmlsec C libraries that were available on the newer Ubuntu 24.04 images.
Resolution: The constraints were put in place due to incremental incompatabilities between the LXML and libxmlsec libraries but both have published new versions that are compatible with eachother and more resilient to the underlying C library versions. By un-pinning and upgrading, we've resolved all known errors with the ubuntu upgrade.
Upgrading to
ubuntu-24.04
instead ofubuntu-latest
Given the complexities of edx-platform depedencies and the sheer number of them, I think it makes sense to explicitly test on a named version of ubuntu that doesn't slip out from under us, instead of a more dynamic pointer. Enough edx-platform libraries rely on the underlying systems C libraries that we'll want to explicitly roll that forward.
This does not mean that we should roll back the rest of the openedx ecosystem as we have not seen any issues that indicate any other repos have this level of deep dependencies on the underlying OS.