Add NRP OSPool EP image builds by brianhlin · Pull Request #307 · opensciencegrid/images

brianhlin · 2026-04-17T22:53:46Z

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

brianhlin · 2026-04-17T22:56:16Z

Oops, wrong issue name!

Copilot

Pull request overview

Adds a new osg-htc/nrp-ospool-ep container image (NRP OSPool EP) and updates the GitHub Actions container build workflow to include osg-htc/ images in addition to opensciencegrid/.

Changes:

Introduces a new osg-htc/nrp-ospool-ep image (Dockerfile + build-config) with entrypoint/init scripts for pilot/Condor behavior in k8s.
Adds wrapper scripts for singularity/apptainer to strip --pid / -p usage.
Updates .github/workflows/build-containers.yml image discovery to include osg-htc/ directories.

Reviewed changes

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

Show a summary per file

File	Description
`.github/workflows/build-containers.yml`	Expands image discovery to include `osg-htc/` roots.
`osg-htc/nrp-ospool-ep/Dockerfile`	Builds the NRP OSPool EP-derived image; adds kubectl, wrappers, and entrypoint/init wiring.
`osg-htc/nrp-ospool-ep/build-config.json`	Declares build matrix parameters for this image.
`osg-htc/nrp-ospool-ep/scripts/entrypoint.sh`	New top-level entrypoint that sources config snippets, runs the original entrypoint as `osg`, and supervises shutdown.
`osg-htc/nrp-ospool-ep/scripts/check_master.sh`	Background monitor intended to detect condor_master termination/restart and dump logs.
`osg-htc/nrp-ospool-ep/scripts/singularity_npid.sh`	Wrapper to drop `--pid/-p` and rewrite `--containall/-C` to omit pid namespace.
`osg-htc/nrp-ospool-ep/scripts/apptainer_npid.sh`	Same as above for apptainer.
`osg-htc/nrp-ospool-ep/scripts/01_token.sh`	Exports `TOKEN` from the mounted token file for osgvo-pilot.
`osg-htc/nrp-ospool-ep/scripts/01_no_condor_host.sh`	Unsets `CONDOR_HOST` before starting the pilot.
`osg-htc/nrp-ospool-ep/scripts/11_set_OSGInstitutionID.sh`	Attempts to derive and export `OSG_INSTITUTION_ID` via kubectl node label.
`osg-htc/nrp-ospool-ep/scripts/02_validate_singularity.sh`	Runtime validation for singularity execution (with optional `--nv`).
`osg-htc/nrp-ospool-ep/scripts/02_validate_apptainer.sh`	Runtime validation for apptainer execution (with optional `--nv`).
`osg-htc/nrp-ospool-ep/scripts/19_set_resources.sh`	Writes resource/slot config into the pilot config file.
`osg-htc/nrp-ospool-ep/scripts/20_advertise_glidein.sh`	Advertises `IS_GLIDEIN` into STARTD_EXPRS.
`osg-htc/nrp-ospool-ep/scripts/20_advertise_k8s_domain.sh`	Advertises k8s-related attributes (pod/domain/namespace/physical hostname).
`osg-htc/nrp-ospool-ep/scripts/21_advertise_k8s_provisioner.sh`	Advertises provisioner name/type into STARTD_EXPRS.
`osg-htc/nrp-ospool-ep/scripts/22_set_requirements.sh`	Adds matching/provisioning expressions to `START` based on requested resources and namespace matching.

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

Copilot · 2026-04-17T22:58:36Z

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


The StartLog tail path has a typo: /pilotl/log/StartLog will not exist (looks like it should be /pilot/log/StartLog). This will make the debugging output misleading right when it's needed.

Suggested change

tail -100 /pilotl/log/StartLog

tail -100 /pilot/log/StartLog

Copilot · 2026-04-17T22:58:37Z

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


Typo in error message: "restrated" should be "restarted".

Copilot · 2026-04-17T22:58:37Z

+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" should be "on its own" (or similar).

Suggested change

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

# protection in case it does not terminate on its own when condor dies or restarts

Copilot · 2026-04-17T22:58:37Z

+# 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 && \
+    chmod u+x /usr/sbin/kubectl


This downloads the latest kubectl at build time via stable.txt and doesn't verify an expected checksum/signature. That makes builds non-reproducible and introduces supply-chain risk (a compromised download endpoint would be silently trusted). Prefer pinning to an explicit kubectl version and validating the downloaded binary (e.g., sha256) during the build.

Copilot · 2026-04-17T22:58:37Z

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


The comment says "only test for apptainer functionality if singularity is present" but this script is checking for /usr/bin/apptainer. The comment is misleading; update it to refer to apptainer.

Suggested change

# only test for apptainer functionality if singularity is present

# only test for apptainer functionality if apptainer is present

Copilot · 2026-04-17T22:58:38Z

+
+
+#
+# Advertise the k8s namespace and physical hostname


The header comment says this script advertises the k8s namespace/hostname, but the body is setting NUM_CPUS/MEMORY/DISK/GPU and slot config. Please update the comment to match the actual behavior so future readers don't get misled.

Suggested change

# Advertise the k8s namespace and physical hostname

# Set resource and slot configuration in the pilot config file,

# including CPUs, memory, disk, and optional GPUs.

Copilot · 2026-04-17T22:58:38Z

+                        grep -E "${ORG_DIR_REGEX}")
          fi

            image_json=$(echo -n "${images:-dummy}" | jq -Rcs '.|split("\n") | map(select(. != ""))')


Including osg-htc/ in ORG_DIR_REGEX will cause the image list to include paths like osg-htc/nrp-ospool-ep, but the rest of this workflow still hard-codes CONTEXT="opensciencegrid/${{ matrix.name }}" (both build and push jobs). Because build-job-matrix.py sets matrix.name to the basename of the image dir, osg-htc images will be built/pushed from a non-existent opensciencegrid/<name> context. Update the matrix generation and/or workflow so the org prefix is preserved and CONTEXT is set to the actual image directory (e.g., osg-htc/nrp-ospool-ep).

Suggested change

image_json=$(echo -n "${images:-dummy}" | jq -Rcs '.|split("\n") | map(select(. != ""))')

image_json=$(echo -n "${images:-dummy}" | jq -Rcs '

split("\n")

| map(select(. != ""))

| map({

name: (split("/") | last),

context: .

})

')

Copilot · 2026-04-17T22:58:38Z

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


Typo in error message: "restrated" should be "restarted".

Copilot · 2026-04-17T22:58:39Z

+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 $@" expands $@ in the current shell and loses argument boundaries/quoting (args containing spaces, quotes, glob chars, etc.). This can break passing runtime flags through to entrypoint.osg.sh. Consider using a form that passes arguments separately (via su's --/arguments support or an intermediate bash -lc wrapper) so "$@" semantics are preserved.

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 "$@"' bash "$@" &

Copilot · 2026-04-17T22:58:39Z

+  rm -f /root/master_pids.txt
+  awk '/CondorVersion/{split($3,a,":"); split(a[2],b,")"); print b[1]}' /pilot/log/MasterLog > /root/master_pids.txt
+  npids=`cat /root/master_pids.txt | wc -l`
+
+  if [ $npids -lt 1 ]; then
+    echo "condor_master log empty, unexpected after first step" 1>&2
+    exit 1
+  fi
+
+  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


check_master.sh computes nprocs only once (before the loop) and never refreshes it, but the loop later uses nprocs to decide whether condor_master is still running. As written, the loop can miss a condor_master exit/crash. Recompute nprocs (and ideally use ps -p "$orgpid") each iteration before the nprocs check.

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 4 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

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

37a336f

Add support for pushing osg-htc project images

702cd98

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

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

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

brianhlin closed this Apr 17, 2026

brianhlin deleted the OPS-158.nrp-ospool-ep branch April 17, 2026 22:56

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add NRP OSPool EP image builds#307

Add NRP OSPool EP image builds#307
brianhlin wants to merge 34 commits intoopensciencegrid:mainfrom
brianhlin:OPS-158.nrp-ospool-ep

brianhlin commented Apr 17, 2026

Uh oh!

brianhlin commented Apr 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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

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

	# Advertise the k8s namespace and physical hostname
	# Set resource and slot configuration in the pilot config file,
	# including CPUs, memory, disk, and optional GPUs.

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

Conversation

brianhlin commented Apr 17, 2026

Uh oh!

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!

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