jenkinsci / docker-agent

Jenkins agent (base image) and inbound agent Docker images
https://hub.docker.com/r/jenkins/inbound-agent/
MIT License
282 stars 232 forks source link

Update `adoptium-install-jdk.sh` script content #811

Closed github-actions[bot] closed 5 months ago

github-actions[bot] commented 5 months ago

Update `adoptium-install-jdk.sh` script content

Update script content

1 file(s) updated with "#!/bin/sh\nset -x\n\n# Check if curl and tar are installed\nif ! command -v curl >/dev/null 2>&1 || ! command -v tar >/dev/null 2>&1 ; then\n echo \"curl and tar are required but not installed. Exiting with status 1.\" >&2\n exit 1\nfi\n\n# Check required variable\n: \"${JAVA_VERSION:?}\"\n\n# Set the OS to \"standard\" by default\nOS=\"standard\"\n\n# If a second argument is provided, use it as the OS\nif [ $# -eq 1 ]; then\n OS=\"${1}\"\nfi\n\n# Call adoptium-get-jdk-link.sh with JAVA_VERSION and OS as arguments\n# The two scripts should be in the same directory.\n# That's why we're trying to find the directory of the current script and use it to call the other script.\nSCRIPT_DIR=$(cd \"$(dirname \"$0\")\" || exit; pwd)\nif ! DOWNLOAD_URL=$(\"${SCRIPT_DIR}\"/adoptium-get-jdk-link.sh \"${JAVA_VERSION}\" \"${OS}\"); then\n echo \"Error: Failed to fetch the URL. Exiting with status 1.\" >&2\n exit 1\nfi\n\n# Use curl to download the JDK archive from the URL\nif ! curl --silent --location --output /tmp/jdk.tar.gz \"${DOWNLOAD_URL}\"; then\n echo \"Error: Failed to download the JDK archive. Exiting with status 1.\" >&2\n exit 1\nfi\n\n# Extract the archive to the /opt/ directory\nif ! tar -xzf /tmp/jdk.tar.gz -C /opt/; then\n echo \"Error: Failed to extract the JDK archive. Exiting with status 1.\" >&2\n exit 1\nfi\n\n# Get the name of the extracted directory\nEXTRACTED_DIR=$(tar -tzf /tmp/jdk.tar.gz | head -n 1 | cut -f1 -d\"/\")\n\n# Rename the extracted directory to /opt/jdk-${JAVA_VERSION}\nif ! mv \"/opt/${EXTRACTED_DIR}\" \"/opt/jdk-${JAVA_VERSION}\"; then\n echo \"Error: Failed to rename the extracted directory. Exiting with status 1.\" >&2\n exit 1\nfi\n\n# Remove the downloaded archive\nif ! rm -f /tmp/jdk.tar.gz; then\n echo \"Error: Failed to remove the downloaded archive. Exiting with status 1.\" >&2\n exit 1\nfi\n": * adoptium-install-jdk.sh

GitHub Action workflow link

Updatecli logo

Created automatically by Updatecli

Options:

Most of Updatecli configuration is done via its manifest(s).

  • If you close this pull request, Updatecli will automatically reopen it, the next time it runs.
  • If you close this pull request and delete the base branch, Updatecli will automatically recreate it, erasing all previous commits made.

Feel free to report any issues at github.com/updatecli/updatecli.
If you find this tool useful, do not hesitate to star our GitHub repository as a sign of appreciation, and/or to tell us directly on our chat!

lemeurherve commented 5 months ago

Shellcheck changes only, already tested in this repository, reviewed and approved in https://github.com/jenkins-infra/shared-tools/pull/148, merging now to avoid wasting CI resources.

dduportal commented 5 months ago

Shellcheck changes only, already tested in this repository, reviewed and approved in jenkins-infra/shared-tools#148, merging now to avoid wasting CI resources.

I disagree. It is too late know to act '(except checking the build on the main branch), but the change was NOT tested in https://github.com/jenkins-infra/shared-tools/pull/148. Did you test it in the context of this repository?

lemeurherve commented 5 months ago

Yes.

From a comment here https://github.com/jenkinsci/docker-agent/pull/811#issuecomment-2114865822

already tested in this repository

From https://github.com/jenkins-infra/shared-tools/pull/148

Tested by building all linux images locally with these scripts in the 3 existing consumer repos.

lemeurherve commented 5 months ago

I should add that while I merged before the end of the build, I waited for for it to pass the script execution step in the docker images builds before merging theses PRs.

I'll wait the complete runs for the next times.

dduportal commented 5 months ago

@lemeurherve thanks for the details! I asked for it , because, unless someone is in your brain (or mine as I also reviewed the shared-tools), then they can't get a glance of what has been done.

At least documenting what you are doing in each PR to help reviewers is a first step (which you did).

Additionally, CI checks runs are not here to slow us down or waste money: they are here to run a set of validations which serves as a reference: your machine or mine are not a reference (we can and should tests things on it when working on changes as due diligence - which you did- but this is only enough for emergency cases such as security or production hotfixes. This is not the case here hence my messages).