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.

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

Merged
merged 6 commits into from
Nov 16, 2020
Merged

Transition from nodejs10-angular to nodejs12-angular #485

merged 6 commits into from
Nov 16, 2020

Conversation

felipecruz91
Copy link
Contributor

@felipecruz91 felipecruz91 commented Nov 13, 2020

Transition from nodejs10-angular to nodejs12-angular.

Refers #483

Tasks:

  • remove the nodejs10-angular agent image
  • add the required dependencies (chrome, xvfb) to the nodejs12 agent images
  • switch all quickstarters to use nodejs12

Let me know if there's anything more that I should include in this PR.

Copy link
Member

@michaelsauter michaelsauter left a 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.

@felipecruz91
Copy link
Contributor Author

felipecruz91 commented Nov 13, 2020

@michaelsauter The pipeline job jenkins-agent-nodejs12 will fail because it uses a fake nexus URL when building the Docker image: --build-arg nexusUrl=https://nexus.example.com.

Error:

npm ERR! network request to https://nexus.example.com/repository/npmjs/@angular%2fcli failed, reason: getaddrinfo ENOTFOUND nexus.example.com

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 && \
Copy link
Member

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.

Copy link
Member

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:
    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 a Dockerfile 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.

Copy link
Member

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.

Copy link
Member

@michaelsauter michaelsauter left a 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!

docs/modules/jenkins-agents/partials/update-to-3x.adoc Outdated Show resolved Hide resolved
@michaelsauter michaelsauter merged commit 85d57aa into opendevstack:master Nov 16, 2020
@michaelsauter michaelsauter added breaking change a change that is NOT backwards compatible documentation Improvements or additions to documentation enhancement New feature or request labels Nov 16, 2020
michaelsauter added a commit to BIX-Digital/ods-core that referenced this pull request Nov 16, 2020
@felipecruz91 felipecruz91 deleted the feature/transition-nodejs12-angular branch November 16, 2020 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change a change that is NOT backwards compatible documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants