-
Notifications
You must be signed in to change notification settings - Fork 43
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
Transition from nodejs10-angular to nodejs12-angular #485
Transition from nodejs10-angular to nodejs12-angular #485
Conversation
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.
Awesome, thanks for this!
One request:
Can you drop the -angular
from the new image? I think this is highly misleading. It should be jenkins-agent-nodejs12
only. However, it should include chrome/xvfb like you have it right now.
And one comment:
I'm currently reworking how these deps are installed when the base image is a UBI8. Instead of downloading a single package from a mirror, it configures the repo properly. I think this might be worthwhile to backport to the non-UBI8 as well, but we can do this after your PR is merged.
@michaelsauter The pipeline job Error:
|
npm config set strict-ssl=false && \ | ||
yarn config set registry $nexusUrl/repository/npmjs/ -g && \ | ||
npm install -g @angular/cli@8.0.1 --unsafe-perm=true --allow-root && \ | ||
npm install -g cypress@3.3.1 --unsafe-perm=true --allow-root > /dev/null && \ |
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.
@cschweikert can we drop installing @angular/cli@8.0.1
and cypress@3.3.1
? That would be ideal because we can then get GitHub Actions to work. Further, it feels wrong for me to back versions of these tools into the agent.
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.
@angular/cli@8.0.1
: We can try to remove it. It is installed now via Jenkinsfile during provisioning:ods-quickstarters/fe-angular/Jenkinsfile
Line 13 in 2c4514d
stage("update angular cli") { cypress@3.3.1
: Having this in was nice, when a build should run e2e tests without prior installation of the cypress testing framework. This usually takes about one minute. But then again you are limited to this specific version. There are other ways of baking cypress binaries into a custom agent image. Last time I did it with aDockerfile
like this:
# This FROM tag is informational. It is overwritten by OpenShift (see oc-template.yaml file)
FROM cd/jenkins-slave-nodejs10-angular:2.x
USER root
# add cypress@4.5.0 binaries to /home/jenkins/.cache/Cypress/4.5.0/Cypress/...
# this allows to disable post-installation of cypress@4.5.0 by setting the environment variable
# CYPRESS_INSTALL_BINARY=0
# cypress@4.5.0 can use those binaries by setting environment variable to the cache folder that holds the binaries
# for the respective versions of cypress
# CYPRESS_CACHE_FOLDER=/home/jenkins/.cache/Cypress
ADD https://download.cypress.io/desktop/4.5.0?platform=linux&arch=x64 /home/jenkins/cypress/cypress-linux.zip
RUN md5sum /home/jenkins/cypress/cypress-linux.zip && \
echo "26835296a4edb1dea35795f59213c519 /home/jenkins/cypress/cypress-linux.zip" | md5sum -c && \
mkdir -p /home/jenkins/.cache/Cypress/4.5.0 && \
unzip -q /home/jenkins/cypress/cypress-linux.zip -d /home/jenkins/.cache/Cypress/4.5.0
# fix access rights
RUN chgrp -R 0 $HOME && \
chmod -R g=u $HOME && \
chmod ug=rx /home/jenkins/.cache/Cypress/4.5.0/Cypress/Cypress
USER 1001
This will allow skipping the post-installation step of cypress binaries (faster build, no issues with proxy servers, adds missing file consistancy check (MD5) for the binaries, etc.)
tl;dr
Yes, let's drop installing @angular/cli@8.0.1
and cypress@3.3.1
.
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.
Cool. Yes I'm in favour of that approach. We should have a section in the docs with "recipes" like this one.
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.
Almost 👍 but maybe you can fix the one doc issue I found before we merge? Thanks!
Transition from nodejs10-angular to nodejs12-angular.
Refers #483
Tasks:
Let me know if there's anything more that I should include in this PR.