-
Notifications
You must be signed in to change notification settings - Fork 22
RHAIENG-1069: Build rstudio RPM from source, instead of using theirs #1693
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
base: main
Are you sure you want to change the base?
RHAIENG-1069: Build rstudio RPM from source, instead of using theirs #1693
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| # RUN subscription-manager repos --enable=rhel-9-for-x86_64-appstream-rpms | ||
| # RUN subscription-manager repos --list-enabled | ||
|
|
||
| #Use CentOS while we are still waiting on final AIPCC image |
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.
It's on the way, need to just merge
- RHAIENG-948: chore(rstudio): add conditional
subscription-manager refreshopendatahub-io/notebooks#2671 - RHOAIENG-38630: chore(rstudio): update AIPCC base images to latest 3.1 images #1694
and we'll have this working on AIPCC
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.
+1 on these two PRs. I am running my test against 3.1 now, will be awhile.
podman build --no-cache -f rstudio/rhel9-python-3.12/Dockerfile.konflux.cpu --platform linux/x86_64 --build-arg BASE_IMAGE=quay.io/aipcc/base-images/cpu:3.1-1762903336 -t vath-rpm-rhel9-python-3.12 .
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.
cpu:3.1-1762903336 was successfully built locally.
| # Install R packages | ||
| # https://cran.r-project.org/web/packages | ||
| # COPY ${RSTUDIO_SOURCE_CODE}/install_packages.R ./ | ||
| # RUN R -f ./install_packages.R && \ |
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.
Can you uncomment this part as well an see how it goes? This script pre-installs R Packages
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.
I will uncomment them once this PR is ready because it take long time to test this. But I will do.
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.
It's something we don't do currently, and since the task is to build and GA what we already have, I believe we should not be preinstalling R packages in 3.2/2.25.1
notebooks/rstudio/rhel9-python-3.12/Dockerfile.cpu
Lines 161 to 168 in f0983f5
| # # Install R packages | |
| # # https://cran.r-project.org/web/packages | |
| # COPY ${RSTUDIO_SOURCE_CODE}/install_packages.R ./ | |
| # RUN /bin/bash <<'EOF' | |
| # set -Eeuxo pipefail | |
| # R -f ./install_packages.R | |
| # rm ./install_packages.R | |
| # EOF |
To preinstall packages, first, we should not preinstall package in RStudio-minimal workbench, we should create a RStudio-datascience workbench with packages. And second, there is followup task
- https://issues.redhat.com/browse/RHAIENG-514 (Option to install R core package in R-Studio Build)
- and https://issues.redhat.com/browse/RHOAIENG-366 (Preinstall needed packages on R Studio workbench)
- and the long list from @sarabanderby https://docs.google.com/document/d/1k8-TusRRjjQ0l9GVd5RpsrJWR_vbkv46uKDUXAU_Xsw/edit?tab=t.0
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.
If no objection from @atheo89, I'll remove these installation section.
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.
No objections by my side, definitely this can be done in a follow up work.
I remember but it is hard to find that ticket, we had reported to AIPCC to include also some libs that are needed for these preinstalled Rpackages, and I thought that was useful to check if some of them are missing. But no problem from my side
|
/build-konflux |
f62a29f to
506bdd8
Compare
…r prebuilt package. During an early build step called rpm-builder, the source code is downloaded and all build instructions provided by RStudio are followed to produce an .rpm file. Later, the RStudio RPM is copied and installed into the AIPCC image.
506bdd8 to
6077139
Compare
|
Another push, included Dockerfile.konflux.cuda |
|
@ysok: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Description
RHAIENG-1069: Build the RStudio RPM from source instead of using the prebuilt RPM provided by RStudio. During an early build step called rpm-builder, the source code is downloaded and all build instructions provided by RStudio are followed to produce an .rpm file. Later, the RStudio RPM is copied and installed into the AIPCC image.
How Has This Been Tested?
Tested locally using podman
podman build -f rstudio/rhel9-python-3.12/Dockerfile.konflux.cpu --platform linux/x86_64 --build-arg BASE_IMAGE=quay.io/aipcc/base-images/cpu:3.0 -t test-rstudio-rhel9-python-3.12 .Self checklist (all need to be checked):
make test(gmakeon macOS) before asking for reviewDockerfile.konfluxfiles should be done inodh/notebooksand automatically synced torhds/notebooks. For Konflux-specific changes, modifyDockerfile.konfluxfiles directly inrhds/notebooksas these require special attention in the downstream repository and flow to the upcoming RHOAI release.Merge criteria: