OSPOOL-158: Add NRP OSPool EP image builds by brianhlin · Pull Request #308 · opensciencegrid/images

brianhlin · 2026-04-17T22:58:11Z

I bet this will fail, at the very least because the Harbor user for pushing images probably doesn't have permissions to push to the osg-htc project.

…fixed condor config

…elease

…egrid/osgvo-docker-pilot:23-el9-releasee

…ease

…' into OPS-158.nrp-ospool-ep

Copilot

Pull request overview

Adds a new osg-htc/nrp-ospool-ep container image (and supporting init/entrypoint scripts) intended for NRP OSPool EP usage, and updates the GitHub Actions container build workflow to detect images under osg-htc/ in addition to opensciencegrid/.

Changes:

Introduces a new nrp-ospool-ep image (Dockerfile + build-config) with custom entrypoint behavior and HTCondor/pilot configuration scripts.
Adds wrapper scripts for singularity/apptainer to strip --pid/-p and adjusts runtime validation scripts.
Updates .github/workflows/build-containers.yml to include osg-htc/... image directories when building the image list.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
`osg-htc/nrp-ospool-ep/Dockerfile`	Defines the new EP-derived image and wires in runtime/init scripts.
`osg-htc/nrp-ospool-ep/build-config.json`	Declares build matrix inputs (OS/series/repos) for the new image.
`osg-htc/nrp-ospool-ep/scripts/entrypoint.sh`	New entrypoint wrapper to run image-config hooks and supervise the pilot process.
`osg-htc/nrp-ospool-ep/scripts/check_master.sh`	Adds condor_master monitoring/diagnostics for termination/restart handling.
`osg-htc/nrp-ospool-ep/scripts/singularity_npid.sh`	Wrapper to drop `--pid/-p` and rewrite related flags.
`osg-htc/nrp-ospool-ep/scripts/apptainer_npid.sh`	Wrapper to drop `--pid/-p` and rewrite related flags.
`osg-htc/nrp-ospool-ep/scripts/01_token.sh`	Exposes the pilot token as an env var for osgvo-pilot.
`osg-htc/nrp-ospool-ep/scripts/01_no_condor_host.sh`	Unsets `CONDOR_HOST` to satisfy osgvo-pilot expectations.
`osg-htc/nrp-ospool-ep/scripts/02_validate_singularity.sh`	Adds a basic singularity execution validation at init time.
`osg-htc/nrp-ospool-ep/scripts/02_validate_apptainer.sh`	Adds a basic apptainer execution validation at init time.
`osg-htc/nrp-ospool-ep/scripts/11_set_OSGInstitutionID.sh`	Attempts to derive `OSG_INSTITUTION_ID` from k8s node labels.
`osg-htc/nrp-ospool-ep/scripts/19_set_resources.sh`	Sets advertised CPU/memory/disk/GPU slot/resource config.
`osg-htc/nrp-ospool-ep/scripts/20_advertise_glidein.sh`	Advertises glidein attribute(s) into the pilot config.
`osg-htc/nrp-ospool-ep/scripts/20_advertise_k8s_domain.sh`	Advertises k8s pod/domain/namespace/physical host into the pilot config.
`osg-htc/nrp-ospool-ep/scripts/21_advertise_k8s_provisioner.sh`	Advertises k8s provisioner name/type into the pilot config.
`osg-htc/nrp-ospool-ep/scripts/22_set_requirements.sh`	Adds matching/provisioning requirements expressions into the pilot config.
`.github/workflows/build-containers.yml`	Expands image discovery to include `osg-htc/...` directories.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-17T23:02:20Z

+  if [ $npids -gt 1 ]; then
+    echo "condor_master restrated" 1>&2
+    break
+  fi
+
+  if [ $nprocs -ne 1 ]; then
+    echo "condor_master not running" 1>&2
+    break
+  fi


check_master.sh computes nprocs once (line 25) but then uses it inside the monitoring loop (line 49) without recomputing it after each sleep. This means the loop may never detect a restarted/dead condor_master based on process state. Recompute the process check inside the loop (and consider using ps -p "$orgpid" or similar to avoid grep-based matching).

Copilot · 2026-04-17T23:02:21Z

+  echo "condor_master restrated at first step" 1>&2
+  exit 1


Typo in error message: "restrated" should be "restarted" (appears in multiple messages). Fixing this will make logs easier to search and understand.

Copilot · 2026-04-17T23:02:21Z

+trap 'echo signal received!; kill $(jobs -p); wait' SIGINT SIGTERM
+
+export HOME=/pilot
+su osg -p -c "/usr/local/sbin/entrypoint.osg.sh $@" &


su ... -c "/usr/local/sbin/entrypoint.osg.sh $@" builds a shell command by interpolating the container's arguments into a string. This is fragile (breaks on spaces/quoting) and can allow shell metacharacters in args to be interpreted by the shell invoked by su -c. Prefer passing arguments without re-parsing (e.g., use su's ability to pass additional args to the shell, or switch to runuser/gosu/setpriv so you can exec the target with a proper "$@" argument vector).

Suggested change

su osg -p -c "/usr/local/sbin/entrypoint.osg.sh $@" &

su osg -p -s /bin/bash -c 'exec /usr/local/sbin/entrypoint.osg.sh "$@"' -- "$@" &

Copilot · 2026-04-17T23:02:22Z

+# else do nothing, let Condor figure it out
+
+if [ -f "/usr/bin/apptainer" ]; then
+  # only test for apptainer functionality if singularity is present


In 02_validate_apptainer.sh, the comment says "only test for apptainer functionality if singularity is present" but the script checks for /usr/bin/apptainer. Update the comment to avoid confusion during debugging.

Suggested change

# only test for apptainer functionality if singularity is present

# only test for apptainer functionality if apptainer is present

Copilot · 2026-04-17T23:02:22Z

+env ACCEPT_JOBS_FOR_HOURS=24
+
+# keep default idle time low, as we may over-provision certain kinds of resources
+# but others may be waiting
+env ACCEPT_IDLE_MINUTES=20


Dockerfile uses the instruction env (lowercase) to set environment variables. Dockerfile instructions are case-insensitive in some tooling, but env is not a valid Dockerfile directive in the standard parser; this will fail builds on common builders. Use ENV for these variables so the image builds reliably.

Suggested change

env ACCEPT_JOBS_FOR_HOURS=24

# keep default idle time low, as we may over-provision certain kinds of resources

# but others may be waiting

env ACCEPT_IDLE_MINUTES=20

ENV ACCEPT_JOBS_FOR_HOURS=24

# keep default idle time low, as we may over-provision certain kinds of resources

# but others may be waiting

ENV ACCEPT_IDLE_MINUTES=20

Copilot · 2026-04-17T23:02:22Z

+tail -100 /pilot/log/MasterLog
+
+echo "=== tail /pilot/log/StartLog"
+tail -100 /pilotl/log/StartLog


The diagnostics section tails tail -100 /pilotl/log/StartLog but the rest of the script uses /pilot/log/.... The /pilotl path looks like a typo and will cause the StartLog tail to fail, reducing the usefulness of the debug output.

Suggested change

tail -100 /pilotl/log/StartLog

tail -100 /pilot/log/StartLog

Copilot · 2026-04-17T23:02:22Z

+su osg -p -c "/usr/local/sbin/entrypoint.osg.sh $@" &
+myproc=$!
+
+# protection in case it does not terminate oby itself when condor dies or restarts


Comment typo: "does not terminate oby itself" should be "does not terminate by itself".

Suggested change

# protection in case it does not terminate oby itself when condor dies or restarts

# protection in case it does not terminate by itself when condor dies or restarts

Copilot · 2026-04-17T23:02:23Z

+# unless it is already set
+#
+if [ "x${OSG_INSTITUTION_ID}" == "x" ]; then
+   OSG_INSTITUTION_ID=`/usr/sbin/kubectl get node ${PHYSICAL_HOSTNAME} -L nautilus.io/OSGInstitutionID | tail -1 | awk '{print $6}'`


kubectl get node ... | awk '{print $6}' relies on a fixed column number in human-readable output, which is brittle (kubectl output columns can change with versions/flags). Prefer a structured output format (e.g., -o jsonpath=... for the label value) so OSGInstitutionID extraction is stable.

Suggested change

OSG_INSTITUTION_ID=`/usr/sbin/kubectl get node ${PHYSICAL_HOSTNAME} -L nautilus.io/OSGInstitutionID | tail -1 | awk '{print $6}'`

OSG_INSTITUTION_ID=`/usr/sbin/kubectl get node ${PHYSICAL_HOSTNAME} -o jsonpath="{.metadata.labels['nautilus\.io/OSGInstitutionID']}"`

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-17T23:35:17Z

+#!/bin/bash
+
+#
+# osgvo pilot does not like is CONDOR_HOST is set


Comment typo: "does not like is CONDOR_HOST" should be "does not like if CONDOR_HOST".

Suggested change

# osgvo pilot does not like is CONDOR_HOST is set

# osgvo pilot does not like if CONDOR_HOST is set

Copilot · 2026-04-17T23:35:18Z

+su osg -p -c "/usr/local/sbin/entrypoint.osg.sh $@" &
+myproc=$!
+
+# protection in case it does not terminate oby itself when condor dies or restarts


Typo in comment: "oby" → "on its own" / "by itself".

Suggested change

# protection in case it does not terminate oby itself when condor dies or restarts

# protection in case it does not terminate by itself when condor dies or restarts

Copilot · 2026-04-17T23:35:18Z

+export TOKEN=`cat /etc/condor/tokens.d/prp-wn.token`
+


If /etc/condor/tokens.d/prp-wn.token is missing/unreadable, this will silently export an empty TOKEN and continue, which can lead to confusing pilot failures later. Add an explicit existence/readability check and fail fast with a clear error (and prefer $(...) over backticks).

Suggested change

export TOKEN=`cat /etc/condor/tokens.d/prp-wn.token`

token_file=/etc/condor/tokens.d/prp-wn.token

if [ ! -r "$token_file" ]; then

echo "ERROR: required token file '$token_file' is missing or unreadable" >&2

exit 1

fi

export TOKEN=$(cat "$token_file")

Copilot · 2026-04-17T23:35:18Z

@@ -54,7 +54,7 @@ jobs:
               images=$(git diff --name-only \
                                 "$BASE" \
                                 "$GITHUB_SHA" |
-                        egrep "^$ORG_DIR/" |
+                        grep -E "$ORG_DIR_REGEX" |
                        cut -d/ -f -2 |


The workflow now includes osg-htc/... images in the build list, but the push job still uses the same Harbor registry (hub.opensciencegrid.org) and robot credentials for all contexts. If context is used as the registry repo name, this will attempt to push into an osg-htc namespace/project that the current robot account may not be authorized for; consider adding per-org registry/credentials routing or excluding osg-htc/* from the push steps until permissions are in place.

Copilot · 2026-04-17T23:35:18Z

+    echo "condor_master restrated" 1>&2
+    break


Typo in error message: "restrated" → "restarted".

Copilot · 2026-04-17T23:35:18Z

+RUN update-alternatives --install /usr/bin/python python /usr/bin/python3 10
+
+# Add kubectl, to be able to interact with the k8s cluster
+RUN curl -L "https://dl.k8s.io/release/$(curl -L -s https://dl.k8s.io/release/stable.txt)/bin/linux/amd64/kubectl" -o /usr/sbin/kubectl && \


This downloads kubectl from the internet at build time using the moving stable.txt pointer and without checksum/signature verification. Pin the kubectl version (and verify the SHA256, or install via the distro package manager where possible) to reduce supply-chain risk and make builds reproducible.

Suggested change

RUN curl -L "https://dl.k8s.io/release/$(curl -L -s https://dl.k8s.io/release/stable.txt)/bin/linux/amd64/kubectl" -o /usr/sbin/kubectl && \

ARG KUBECTL_VERSION=v1.30.2

ARG KUBECTL_SHA256=4310270c87a4f80637ef6fa9af9d1f703d8d43b7efd2d7661f7902dc0e05251d

RUN curl -fL "https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/linux/amd64/kubectl" -o /usr/sbin/kubectl && \

echo "${KUBECTL_SHA256} /usr/sbin/kubectl" | sha256sum -c - && \

Copilot · 2026-04-17T23:35:19Z

+  echo "condor_master restrated at first step" 1>&2
+  exit 1


Typo in error message: "restrated" → "restarted".

sfiligoi added 30 commits May 12, 2022 11:20

Copy files from prp-osgvo-pilot/image

2c87eaf

Integrate wn/image/image-config.d/02_additional_requirements.sh with …

2a606c7

…fixed condor config

Add scripts/21_advertise_k8s_provisioner.sh

2b208d3

Add FORCE_K8SNAMESPACE_MATCHING

1a434ec

Add scripts/19_set_resources.sh

48843d3

Fix typo

99c17c6

Add scripts/01_token.sh

e6a3206

Fix script location

044de80

Fix typo in 22_set_requirements.sh

f67e725

Add scripts/01_no_condor_host.sh

8b07089

Add scripts/check_master.sh

dc73c23

Fix typo

9a6638f

Add 02_validate_singularity.sh

9d90d37

Fix return code

f87182a

Wrap apptainer, too

bc2f7a2

Test also --nv in singularity

2168067

Merge branch 'main' into ospool_pool_base

e779a8f

Test also --nv in apptainer

410761f

Change entrypoint location as per upstream

5c77a66

Switch from osgvo-docker-pilot:3.6-release to osgvo-docker-pilot:23-r…

30e0480

…elease

Move from opensciencegrid/osgvo-docker-pilot:23-release to openscienc…

e9efb8b

…egrid/osgvo-docker-pilot:23-el9-releasee

Fix typo

2881c0d

Fix python vs python3

ee5a863

Add IS_GLIDEIN classad

3286dfa

Add kubectl in the image

0247b70

Add 11_set_OSGInstitutionID.sh init script that sets OSG_INSTITUTION_ID

7fe8e93

Add OpenCL support

7b2a55f

Update base image to hub.opensciencegrid.org/osg-htc/ospool-ep:24-rel…

b073ed5

…ease

Replace exact matching with approximate matching

bbea903

Fix typo

ad44c50

sfiligoi and others added 3 commits February 24, 2025 17:44

Make disk range larger

2c7cc9f

Merge remote-tracking branch 'prp-htcondor-portal/ospool_pool_base_24…

1b00676

…' into OPS-158.nrp-ospool-ep

OSPOOL-158: add OSG 25 NRP ospool-ep builds

03de498

brianhlin requested a review from a team as a code owner April 17, 2026 22:58

Copilot AI review requested due to automatic review settings April 17, 2026 22:58

Copilot started reviewing on behalf of brianhlin April 17, 2026 22:58 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

brianhlin force-pushed the OSPOOL-158.nrp-ospool-ep branch from a29deba to 34374da Compare April 17, 2026 23:11

Add support for pushing osg-htc project images

f09cdf3

brianhlin force-pushed the OSPOOL-158.nrp-ospool-ep branch from 34374da to f09cdf3 Compare April 17, 2026 23:16

brianhlin requested a review from Copilot April 17, 2026 23:30

Copilot started reviewing on behalf of brianhlin April 17, 2026 23:30 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

	su osg -p -c "/usr/local/sbin/entrypoint.osg.sh $@" &
	su osg -p -s /bin/bash -c 'exec /usr/local/sbin/entrypoint.osg.sh "$@"' -- "$@" &

	# only test for apptainer functionality if singularity is present
	# only test for apptainer functionality if apptainer is present

	# protection in case it does not terminate oby itself when condor dies or restarts
	# protection in case it does not terminate by itself when condor dies or restarts

	OSG_INSTITUTION_ID=`/usr/sbin/kubectl get node ${PHYSICAL_HOSTNAME} -L nautilus.io/OSGInstitutionID \| tail -1 \| awk '{print $6}'`
	OSG_INSTITUTION_ID=`/usr/sbin/kubectl get node ${PHYSICAL_HOSTNAME} -o jsonpath="{.metadata.labels['nautilus\.io/OSGInstitutionID']}"`

	# osgvo pilot does not like is CONDOR_HOST is set
	# osgvo pilot does not like if CONDOR_HOST is set

-export TOKEN=`cat /etc/condor/tokens.d/prp-wn.token`
+token_file=/etc/condor/tokens.d/prp-wn.token
+if [ ! -r "$token_file" ]; then
+    echo "ERROR: required token file '$token_file' is missing or unreadable" >&2
+    exit 1
+fi
+export TOKEN=$(cat "$token_file")

-RUN curl -L "https://dl.k8s.io/release/$(curl -L -s https://dl.k8s.io/release/stable.txt)/bin/linux/amd64/kubectl" -o /usr/sbin/kubectl && \
+ARG KUBECTL_VERSION=v1.30.2
+ARG KUBECTL_SHA256=4310270c87a4f80637ef6fa9af9d1f703d8d43b7efd2d7661f7902dc0e05251d
+RUN curl -fL "https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/linux/amd64/kubectl" -o /usr/sbin/kubectl && \
+    echo "${KUBECTL_SHA256}  /usr/sbin/kubectl" | sha256sum -c - && \

Conversation

brianhlin commented Apr 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants