Implement update lockfiles and manage pure-rpm containers

[amoralej]

This PR introduces two improvements in build.sh:

• Implements a clone-free update-lockfiles command.
• Manages containers with only rpms installation (not sources management)
• It also replaces the existing resource consuming update-sources test by a lighweight update-lockfiles based one.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign rebtoor for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[SeanMooney]

not sure if https://gist.github.com/SeanMooney/9b4fa52de29e2f795443e7eb60cc3994 helps

i tried to define the 3 things tha tupdate-souce can do and give them names

https://gist.github.com/SeanMooney/9b4fa52de29e2f795443e7eb60cc3994#command-contracts
and then breifly expored optimising the git interaction
https://gist.github.com/SeanMooney/9b4fa52de29e2f795443e7eb60cc3994#ci-transport-design
and compared the data transfered
https://gist.github.com/SeanMooney/9b4fa52de29e2f795443e7eb60cc3994#measured-transfer-impact
using watcher and cybrog which have relitive small git repos at ~30M for the .git folder
vs novas 371M

[amoralej]

not sure if https://gist.github.com/SeanMooney/9b4fa52de29e2f795443e7eb60cc3994 helps

i tried to define the 3 things tha tupdate-souce can do and give them names

https://gist.github.com/SeanMooney/9b4fa52de29e2f795443e7eb60cc3994#command-contracts and then breifly expored optimising the git interaction https://gist.github.com/SeanMooney/9b4fa52de29e2f795443e7eb60cc3994#ci-transport-design and compared the data transfered https://gist.github.com/SeanMooney/9b4fa52de29e2f795443e7eb60cc3994#measured-transfer-impact using watcher and cybrog which have relitive small git repos at ~30M for the .git folder vs novas 371M

Yep, good to descompose the use use cases. This patch implements update-lockfiles which is just intended to cover regeneration of lockfiles without any source update (neither upper-constraints.txt nor services sources in sources.txt).

For the update-sources I still need to re-read your doc and find if we really need to decompose in two or we can live with one and predefined behavior (if anything is under src/ ignore sources.txt for that repo). In any case i want to also optimize update-sources to avoid full clones of repos. There are some caveats, i.e. we need git history for pbr to find the versions, but that's only required for build, so it should be fine to just pull selectively for update-sources as you are proposing.

I'd like to go case by case and would appreciate reviews in this particular one while we keep working for the others.

[rebtoor]

I'm generally ok with this but I'd like some clarifications before adding my +2 :)

[rebtoor]

regenerate_requirements_lock can return 1 (missing constraints or
missing lockfile), but the return value is not checked here. If one
project fails, the loop continues and the function returns 0, so the
caller never knows something went wrong.

We can consider to track failures somehow:

  local rc=0
  for img in "${targets[@]}"; do
      ...
      regenerate_requirements_lock "${project}" "${stream}" || rc=1
  done
  return $rc

[amoralej]

We run the script with set -e so in case the regenerate_requirements_lock returns 1, the command fails immediately with rc 1.

=== Regenerating requirements.lock files ===
ERROR: No /home/amoralej/VMwareENG/cyborg/s2i-openstack-containers-upstream/containers/glance/upper-constraints.txt.master found for glance.
       Run 'update-sources' first to fetch constraints.
update-lockfiles: exit 1 (0.16 seconds) /home/amoralej/VMwareENG/cyborg/s2i-openstack-containers-upstream> bash /home/amoralej/VMwareENG/cyborg/s2i-openstack-containers-upstream/build.sh update-lockfiles glance pid=760828
  update-lockfiles: FAIL code 1 (0.29=setup[0.11]+cmd[0.01,0.00,0.16] seconds)
  evaluation failed :( (0.31 seconds)
amoralej@fedora:~/VMwareENG/cyborg/s2i-openstack-containers-upstream$ echo $?
1

Which I think is an acceptable behavior at this point. We may refine to keep looping after error and returning 1 at the end, but that may left the messages hidden imo.

[rebtoor]

Was removing the push intentional? If so, there is no longer any
post-merge verification that lockfiles are consistent on main after
a direct push or merge.

[rebtoor]

The paths trigger includes build.sh, so a PR that only modifies
build.sh (without touching containers/**) will trigger this
workflow. However, detect-changed-services will return an empty
list and both steps will be skipped due to the if: guard, making
the job report green without verifying anything.

Consider either:

• removing build.sh from the paths trigger, or
• running lockfile regeneration for all services when no specific
  changed services are detected but build.sh changed.

[amoralej]

I'm finally only slightly changing the existing test and keeping on push.

[rebtoor]

Multiple packages appear twice with conflicting versions in this lockfile
(and all other service buildrequirements lockfiles):

  flit-core==3.12.0
  flit-core==4.0.2
  ...
  setuptools==82.0.1
  setuptools==84.0.0

Affected files: cinder, cyborg, glance, manila, watcher.

I suspect this is caused by the new --no-annotate + sed '/^#/d'
cleanup stripping the "# The following packages are considered to be
unsafe" section header that previously separated the "safe" and "unsafe"
package lists in pybuild-deps output. Without that header,
pybuild-deps may be emitting some packages in both sections and now
both entries survive the cleanup.

Could you check whether pybuild-deps compile --no-annotate is actually
producing this dual output, and if so, deduplicate (e.g. sort -u -t= -k1,1
keeping the latest version)?