Copilot commented on code in PR #13597:
URL: https://github.com/apache/cloudstack/pull/13597#discussion_r3570062357
##########
tools/docker/supervisord.conf:
##########
@@ -16,7 +16,7 @@ redirect_stderr=true
user=root
[program:cloudstack-ui]
-command=/bin/bash -c "npm run serve"
+command=/bin/bash -c "NODE_OPTIONS=--openssl-legacy-provider npm run serve"
directory=/root/ui
Review Comment:
Avoid embedding NODE_OPTIONS inside the shell command; supervisord supports
an explicit `environment=` directive, which is easier to read and override and
avoids quoting edge cases.
##########
tools/docker/Dockerfile:
##########
@@ -34,7 +34,8 @@ RUN apt-get -y update && apt-get install -y \
ipmitool \
iproute2 \
maven \
- openjdk-11-jdk \
+ openjdk-17-jre-headless \
+ openjdk-17-jdk \
Review Comment:
`openjdk-17-jdk` already pulls in the required JRE, so installing both
`openjdk-17-jre-headless` and `openjdk-17-jdk` is redundant and increases image
size / package surface.
##########
tools/docker/Dockerfile:
##########
@@ -61,12 +62,10 @@ RUN find /var/lib/mysql -type f -exec touch {} \; && \
mysql -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH
caching_sha2_password by ''" --connect-expired-password; \
mvn -Pdeveloper -pl developer -Ddeploydb; \
mvn -Pdeveloper -pl developer -Ddeploydb-simulator; \
- MARVIN_FILE=`find /root/tools/marvin/dist/ -name "Marvin*.tar.gz"`; \
- rm -rf /usr/bin/x86_64-linux-gnu-gcc && \
- ln -s /usr/bin/gcc-10 /usr/bin/x86_64-linux-gnu-gcc; \
- pip3 install $MARVIN_FILE
+ MARVIN_FILE=`find /root/tools/marvin/dist/ -name "[mM]arvin*.tar.gz"`; \
+ pip3 install $MARVIN_FILE --break-system-packages
-RUN curl -sL https://deb.nodesource.com/setup_14.x | sudo -E bash -; \
+RUN curl -sL https://deb.nodesource.com/setup_24.x | sudo -E bash -; \
apt-get install -y nodejs; \
cd ui && npm rebuild node-sass && npm install
Review Comment:
`npm rebuild node-sass` appears to target a dependency that is not present
in `ui/package.json` / `ui/package-lock.json`, so it is unnecessary and may
break the image build. It should be removed (or replaced with rebuilding the
actual native deps, if any).
##########
.github/workflows/docker-cloudstack-simulator.yml:
##########
@@ -21,6 +21,7 @@ on:
push:
branches:
- main
+ - docker-image-build
tags:
- 4.*
- 5.*
Review Comment:
Adding an extra branch to `on.push.branches` means any push to that branch
will also build *and push* an image to the registry using org secrets. If this
is meant for testing the workflow, prefer `workflow_dispatch` (manual runs)
instead of auto-publishing from a non-release branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
