Skip to content
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

BCI repo publisher: also trigger a rebuild of devel:BCI:$VERSION #3023

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dcermak
Copy link
Contributor

@dcermak dcermak commented Oct 24, 2023

currently untested

from osc.core import makeurl
from osc.connection import http_GET

_SLE_VERSION_T = Literal['15-SP3', '15-SP4', '15-SP5', '15-SP6']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this really need to be typed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it doesn't have to, but I found it quite useful, because I was wondering what the values of this variable are. Initially I thought this was a package version only to realize later that it's actually 15-SP$N.

@codecov-commenter
Copy link

codecov-commenter commented Oct 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.46%. Comparing base (c46db9b) to head (de1d827).
Report is 210 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3023   +/-   ##
=======================================
  Coverage   28.46%   28.46%           
=======================================
  Files          85       85           
  Lines       14722    14722           
=======================================
  Hits         4191     4191           
  Misses      10531    10531           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

req.raise_for_status()

for pkg in self.fetch_package_names_in_project(build_prj):
url = makeurl(self.apiurl, ['trigger', 'rebuild'], {'project': build_prj, 'package': pkg})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, no, please no fanout of api posts against the build service. we don't need DoS here. "'package" can take a list, so if project wide rebuild is not working (which it should) then please do one call with the "package" being a list of packages to rebuild.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly, a list doesn't work.

curl -H "Authorization: Token $TOKEN" -X POST "https://api.opensuse.org/trigger/rebuild?project=devel:BCI:SLE-15-SP5&package=rmt-helm,sle15-kernel-module-devel,rust-oldstable-image,rust-stable-image"
<status code="unknown_package">
  <summary>Package not found: devel:BCI:SLE-15-SP5/rmt-helm,sle15-kernel-module-devel,rust-oldstable-image,rust-stable-image</summary>
</status>

And specifying the parameter multiple times rebuilds just one package:

curl -H "Authorization: Token $TOKEN" -X POST "https://api.opensuse.org/trigger/rebuild?project=devel:BCI:SLE-15-SP5&package=rmt-helm&package=sle15-kernel-module-devel&package=rust-oldstable-image&package=rust-stable-image"
<status code="ok">
  <summary>Ok</summary>
</status>

So sadly, we need to DoS OBS 🤷

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dirk is most likely confusing it with the route used by rebuildpac. The rebuild token is a very special route

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. I didn't realize that this uses /trigger instead of /build . The letter has a project wide rebuild option.

I wonder if we somehow can give the bot access to that route.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

openSUSE/open-build-service#2094 is the history of this route

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and openSUSE/open-build-service#15119 is a new attempt at getting a fix in.

@dcermak dcermak marked this pull request as ready for review October 25, 2023 12:14
gocd/bci.gocd.yaml.erb Outdated Show resolved Hide resolved
@dirkmueller dirkmueller changed the title BCI repo publisher: also trigger a rebuild of SUSE:SLE-$VERSION:Update:BCI BCI repo publisher: also trigger a rebuild of devel:BCI:$VERSION Oct 26, 2023
@@ -152,8 +174,16 @@ def run(self, version, token=None):
self.logger.info('No positive result from openQA (yet)')
return

if rebuild_token:
for pkg in self.fetch_package_names_in_project(_DEVEL_PRJ_APIURL, devel_prj):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jftr OBS team will fix this that we can use unscoped tokens to trigger project wide rebuilds

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So shall we wait for them to implement this before moving this forward?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implementation is ready at openSUSE/open-build-service#15119

Copy link
Member

@dirkmueller dirkmueller Oct 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So shall we wait for them to implement this before moving this forward?

no, I think we can do this optimisation to use only one project wide trigger after merging this. however I'd really like to get one repo published first before adding more risk of regressions..

@Vogtinator
Copy link
Member

This will need a rebase as #3026 got merged.

Technically the docstring for is_repo_published is incomplete because of that as well, probably makes sense to fix that in this PR as well

@dcermak
Copy link
Contributor Author

dcermak commented Oct 26, 2023

This will need a rebase as #3026 got merged.

Technically the docstring for is_repo_published is incomplete because of that as well, probably makes sense to fix that in this PR as well

Both done

and correct the docstring of is_repo_published
The current version is iterating twice over the package list, but that is not
necessary. The list can be assembled in a single pass, making it more obvious
what is happening.
@dirkmueller
Copy link
Member

dirkmueller commented Dec 21, 2023

openSUSE/open-build-service#15261 is now merged, I guess we could switch to it and test this (and get it over the hump)?

@@ -157,8 +182,16 @@ def run(self, version, token=None):
return
openqa_passed_packages.append(pkg)

if rebuild_token:
for pkg in self.fetch_package_names_in_project(_DEVEL_PRJ_APIURL, devel_prj):
url = makeurl(_DEVEL_PRJ_APIURL, ['trigger', 'rebuild'], {'project': devel_prj, 'package': pkg})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could now be changed based on openSUSE/open-build-service#15261 (which is deployed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants