-
Couldn't load subscription status.
- Fork 64
enhancements/fixes for eessi_container.sh script #232
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
Conversation
cherry-picked via commit b5bf008 ONLY picked chages to eessi_container.sh
cherry-picked via commit b5c07ee NOTE, only applied changes to eessi_container.sh
fixed bugs - determining IP address for proxy host was wrong - handling of positional parameters for executing commands with arguments was wrong
NOTE only applied changes to eessi_container.sh
cherry-picked via commit fce504f NOTE, only changed eessi_container.sh
cherry-picked via commit 23e773c NOTE only applied changes to eessi_container.sh
cherry-picked via commit bfb1b29 Note, only part for eessi_container.sh included here
eessi_container.sh
Outdated
| EESSI_HOST_STORAGE=$(mktemp -d --tmpdir eessi.XXXXXXXXXX) | ||
| echo "Using ${EESSI_HOST_STORAGE} as tmp storage (add '--resume ${EESSI_HOST_STORAGE}' to resume where this session ended)." | ||
| fi | ||
| echo "RESUME_FROM_DIR ${EESSI_HOST_STORAGE}" |
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 need for this imho, we can just grep line above?
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.
Removed.
eessi_container.sh
Outdated
| fi | ||
| [[ ${VERBOSE} -eq 1 ]] && echo "SINGULARITY_CACHEDIR=${SINGULARITY_CACHEDIR}" | ||
|
|
||
| # pull & convert image and reset CONTAINER |
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.
clarify this comment a bit?
explain that we want to ensure that same container was used as last time?
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.
Done that plus significantly improved it. Should also handle much better when we are resuming from a previous session. And also handles the case that CONTAINER was already pointing to an image file.
eessi_container.sh
Outdated
|
|
||
| # pull & convert image and reset CONTAINER | ||
| CONTAINER_URL_FMT=".*://(.*)" | ||
| if [[ ${CONTAINER} == ${CONTAINER_URL_FMT} ]]; then |
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.
should be using =~ here?
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.
Because we use wildcards only, == should work too. However, using =~ might be safer anyway. Will change it.
eessi_container.sh
Outdated
| cfg_load ${EESSI_REPOS_CFG_FILE} | ||
|
|
||
| # copy repos.cfg to job directory --> makes it easier to inspect the job | ||
| cp ${EESSI_REPOS_CFG_FILE} ${EESSI_TMPDIR}/repos_cfg/. |
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.
cp -a ${EESSI_REPOS_CFG_FILE} ${EESSI_TMPDIR}/repos_cfg/
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.
Done.
eessi_container.sh
Outdated
| [[ ${VERBOSE} -eq 1 ]] && echo "HTTP_PROXY_IPV4='${HTTP_PROXY_IPV4}'" | ||
| echo "CVMFS_HTTP_PROXY=\"${http_proxy}|http://${HTTP_PROXY_IPV4}:${PROXY_PORT}\"" \ | ||
| >> ${EESSI_TMPDIR}/repos_cfg/default.local | ||
| cat ${EESSI_TMPDIR}/repos_cfg/default.local |
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 VERBOSE, also add echo "contents of .../default.local
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.
Added and only printing when VERBOSE is set.
eessi_container.sh
Outdated
| fi | ||
| tar cf ${TGZ} -C ${EESSI_TMPDIR} . | ||
| echo "Saved contents of '${EESSI_TMPDIR}' to '${TGZ}' (to resume, add '--resume ${TGZ}')" | ||
| echo "RESUME_FROM_TGZ ${TGZ}" |
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 need for this, we can grep for --resume in line above
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.
Removed.
| echo "[EESSI/2021.12]" > cfg/repos.cfg | ||
| echo "repo_version = 2021.12" >> cfg/repos.cfg | ||
| echo "[EESSI/2023.02]" >> cfg/repos.cfg | ||
| echo "repo_version = 2023.02" >> cfg/repos.cfg |
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.
maybe use 20XY.AB instead?
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.
Changed.
|
Implemented first round of requested changes in commit 47cc1c6 |
eessi_container.sh
Outdated
| # pull container to ${EESSI_TMPDIR} if it is not there yet (i.e. when | ||
| # resuming from a previous session) | ||
| if [[ ! -x ${EESSI_TMPDIR}/${CONTAINER_IMG} ]]; then | ||
| singularity pull ${EESSI_TMPDIR}/${CONTAINER_IMG} ${CONTAINER} |
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.
There will be situations in which you do want to re-pull the container image rather than using the existing one, for example if a fix was deployed for the container image.
So we should have a separate option for this, to enforce re-pulling the image? (future work, not in this PR)
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.
Make sure that quiet option is also used here unless --verbose was used?
eessi_container.sh
Outdated
| # pull container to ${EESSI_TMPDIR} if it is not there yet (i.e. when | ||
| # resuming from a previous session) | ||
| if [[ ! -x ${EESSI_TMPDIR}/${CONTAINER_IMG} ]]; then | ||
| singularity pull ${EESSI_TMPDIR}/${CONTAINER_IMG} ${CONTAINER} |
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.
Make sure that quiet option is also used here unless --verbose was used?
9a615bf to
c001fea
Compare
c001fea to
a4cea9a
Compare
…ync-1 Final steps in Syncing with EESSI
This adds changes that are in use for the NESSI bot instances. Among the changes
-l | --list-reposto obtain a list of configured CernVM-FS repository identifiershttp(s)_proxyenv vars are used if sethttp_proxyis set, the default.local is augmented withCVMFS_HTTP_PROXY="${http_proxy}|PROXY_USING_IPv4"(thePROXY_USING_IPv4uses the IPv4 address instead of the host name inhttp_proxy; it is determined by the script)for easier parsing of--resumearguments it printsRESUME_FROM_DIR _DIR_orRESUME_FROM_TGZ _PATH_TO_TGZ_