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.

Sign up for GitHub

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

Jump to bottom

feat: add nutmeg release support #18

Merged
merged 1 commit into from
May 30, 2022
Merged

feat: add nutmeg release support #18

merged 1 commit into from
May 30, 2022

Conversation

mariajgrimaldi
Copy link
Contributor

@mariajgrimaldi mariajgrimaldi commented May 18, 2022 •
edited
Loading

Description

This PR adds nutmeg support:

  • Update references from Maple to Nutmeg release
  • Update AppArmor profile so it uses python > 3 (see ref)

How to test

Steps to reproduce the behavior:

  1. Install tutor, tutor-contrib-codejail, codejailservice from source
  2. Move to nutmeg branches: nutmeg -for tutor-, 14.0.0 -for codejailservice-, MJG/nutmeg -for tutor-contrib-codejail-
  3. Install plugin
  4. Build images: tutor config save && tutor images build all
  5. Init services: tutor dev init
  6. Follow test instructions here. The error will appear in the course component.

felipemontoya
felipemontoya reviewed May 23, 2022
View reviewed changes
tutorcodejail/templates/codejail/build/codejail/Dockerfile
@@ -51,9 +51,8 @@ WORKDIR /var/tmp
RUN mkdir -p common/lib/

COPY --from={{ DOCKER_IMAGE_OPENEDX }} /openedx/edx-platform/common/lib/sandbox-packages common/lib/sandbox-packages
COPY --from={{ DOCKER_IMAGE_OPENEDX }} /openedx/edx-platform/common/lib/symmath common/lib/symmath
Copy link
Member

Choose a reason for hiding this comment

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

Do we not need symmath anymore? or is it included in py38?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That path was removed in nutmeg: openedx/edx-platform@8e8d840
https://github.com/openedx/edx-platform/tree/open-release/nutmeg.master/common/lib

Copy link
Member

Choose a reason for hiding this comment

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

I see.

Now that it is located at openedx-calc

Symmath has been added to openedx-calc alongside calc in openedx/openedx-calc#38.

Do we need to allow it or install it or something else?

Copy link
Contributor Author

@mariajgrimaldi mariajgrimaldi May 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

As you said, it's now in py38.txt

@BbrSofiane BbrSofiane mentioned this pull request May 23, 2022
Closed
@mariajgrimaldi mariajgrimaldi changed the title [WIP] Mjg/nutmeg feat: add nutmeg release support May 24, 2022
mariajgrimaldi
mariajgrimaldi commented May 24, 2022
View reviewed changes
tutorcodejail/plugin.py
@@ -17,7 +17,7 @@
"VERSION": __version__,
"HOST": "codejailservice",
"DOCKER_IMAGE": f"docker.io/ednxops/codejailservice:{__version__}",
"SANDBOX_PYTHON_VERSION": "3.5.10",
Copy link
Contributor Author

@mariajgrimaldi mariajgrimaldi May 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

We changed it from 3.5 to 3.8 since the sandbox requirements didn't include py3.5 anymore.
Now, which patch version should we use here?
We first tried to use 3.8.6 but then changed it to 3.8.5 so it doesn't crash with the codejailservice pyenv (which also uses 3.8.6).
@felipemontoya @Alec4r @MoisesGSalas @jfavellar90 @johanv26

Copy link
Member

Choose a reason for hiding this comment

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

maybe you can use pyenv-alias and install the sandbox version as x.y.z_sandbox ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great suggestion! Thanks, I'll try that

MoisesGSalas
MoisesGSalas reviewed May 24, 2022
View reviewed changes
tutorcodejail/templates/codejail/apps/profiles/docker-edx-sandbox Outdated


#
# Whitelist particiclar shared objects from the system
# python installation
#
/sandbox/venv/** mr,
/opt/pyenv/versions/3.5.10/** mr,
/opt/pyenv/versions/3.[0-9].*/** mr,
Copy link
Member

Choose a reason for hiding this comment

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

Wasn't this intended for the sandbox version only? you can use {{ SANDBOX_PYTHON_VERSION }} right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right! Thanks

@mariajgrimaldi
Copy link
Contributor Author

Can you review again please? @MoisesGSalas @felipemontoya

MoisesGSalas
MoisesGSalas reviewed May 26, 2022
View reviewed changes
tutorcodejail/templates/codejail/apps/profiles/docker-edx-sandbox Outdated


#
# Whitelist particiclar shared objects from the system
# python installation
#
/sandbox/venv/** mr,
/opt/pyenv/versions/3.5.10/** mr,
/opt/pyenv/versions/{{ CODEJAIL_SANDBOX_PYTHON_VERSION }}/** mr,
Copy link
Member

Choose a reason for hiding this comment

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

This should be {{ CODEJAIL_SANDBOX_PYTHON_VERSION }}_sandbox

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I totally missed that. Thanks

tutorcodejail/templates/codejail/build/codejail/Dockerfile Outdated
@@ -20,7 +20,8 @@ ARG CODEJAILSERVICE_PYTHON_VERSION=3.8.6
RUN $PYENV_ROOT/bin/pyenv install $CODEJAILSERVICE_PYTHON_VERSION

ARG SANDBOX_PYTHON_VERSION={{ CODEJAIL_SANDBOX_PYTHON_VERSION }}
RUN $PYENV_ROOT/bin/pyenv install $SANDBOX_PYTHON_VERSION
RUN git clone https://github.com/s1341/pyenv-alias.git $PYENV_ROOT/plugins/pyenv-alias
RUN VERSION_ALIAS={{ CODEJAIL_SANDBOX_PYTHON_VERSION }}_sandbox $PYENV_ROOT/bin/pyenv install $SANDBOX_PYTHON_VERSION

RUN $PYENV_ROOT/versions/$CODEJAILSERVICE_PYTHON_VERSION/bin/python -m venv /openedx/venv
RUN $PYENV_ROOT/versions/$SANDBOX_PYTHON_VERSION/bin/python -m venv --copies /sandbox/venv
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
RUN $PYENV_ROOT/versions/$SANDBOX_PYTHON_VERSION/bin/python -m venv --copies /sandbox/venv
RUN $PYENV_ROOT/versions/"$SANDBOX_PYTHON_VERSION"_sandbox/bin/python -m venv --copies /sandbox/venv

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good, thanks. I'll build the image and let you know how it goes

@mariajgrimaldi mariajgrimaldi requested a review from NeOneSoft May 27, 2022 17:46
@DeanJayMathew
Copy link

DeanJayMathew commented May 28, 2022 •
edited
Loading

@mariajgrimaldi Has this fix been successful?

The reason I ask is because it is a blocker to resolving this Nutmeg testing issue: openedx/wg-build-test-release#159

@mariajgrimaldi
Copy link
Contributor Author

Yes @DeanJayMathew. I'm just waiting for an approval to merge and release.

@mariajgrimaldi
Copy link
Contributor Author

I've tested building a new image with the latest changes in the PR and it's working!
image
image

Using the course attached here

@mariajgrimaldi mariajgrimaldi requested a review from MaferMazu May 29, 2022 19:46
@mariajgrimaldi
feat: add support for nutmeg release
b5dff67 
* Bump version 13.x to 14.x
* Bump Python to 3.8 in sandbox environment
* Add support for python 3.8 in the apparmor profile
@mariajgrimaldi mariajgrimaldi merged commit ddde751 into main May 30, 2022
@mariajgrimaldi mariajgrimaldi deleted the MJG/nutmeg branch May 30, 2022 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MoisesGSalas MoisesGSalas MoisesGSalas approved these changes

@felipemontoya felipemontoya Awaiting requested review from felipemontoya

@Alec4r Alec4r Awaiting requested review from Alec4r

@jfavellar90 jfavellar90 Awaiting requested review from jfavellar90

@JuanDavidBuitrago JuanDavidBuitrago Awaiting requested review from JuanDavidBuitrago

@johanseto johanseto Awaiting requested review from johanseto

@NeOneSoft NeOneSoft Awaiting requested review from NeOneSoft

@MaferMazu MaferMazu Awaiting requested review from MaferMazu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mariajgrimaldi @DeanJayMathew @felipemontoya @MoisesGSalas @jfavellar90 @Alec4r