Update build_openstack_packages to support downstream gitlab #2699
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
raukadah
requested review from
pablintino,
lewisdenny,
evallesp,
bshewale and
frenzyfriday
as code owners
There was a problem hiding this comment.
Hi @raukadah, could you please add a commit message to explain this change
raukadah
force-pushed
the
fix_downstream
branch
from
a8357c7 to
837525b
Compare
raukadah
force-pushed
the
fix_downstream
branch
from
837525b to
66f0f16
Compare
|
/hold |
raukadah
force-pushed
the
fix_downstream
branch
6 times, most recently
from
4070691 to
798e3bf
Compare
raukadah
changed the title
|
/hold cancel |
Done! |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/1c7d1238aac243e6b9a2237f2ce34e3d ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 28m 34s |
raukadah
force-pushed
the
fix_downstream
branch
from
798e3bf to
19cafd5
Compare
raukadah
changed the title
| loop: | ||
| - "{{ cifmw_bop_osp_release }}-rhel-9-trunk" | ||
| - "{{ cifmw_bop_osp_release }}-trunk-patches" | ||
| version: "{{ cifmw_bop_osp_release }}-trunk-patches" |
There was a problem hiding this comment.
you are initially checking out trunk-patches, that may not be correct in all cases such as if we want to run this in the -patches branch so you should not assume it trunk-patches in general. this was also assumed in the old code so that ok for now but we likely want to look this up based on the target branch of the MR when this is run in patches ci. for now this is enough to have parity with the older code.
|
|
||
| - name: Update the project reference in packages.yml # noqa: command-instead-of-module | ||
| vars: | ||
| _old_content: 'osp-patches: ssh://{{ _change.host | split("//") | last }}:22/{{ _change.project }}' | ||
| _old_content: 'osp-patches: git@{{ _change.host | split("//") | last }}:{{ _change.project }}.git' |
There was a problem hiding this comment.
thecnically the git protocal is just ssh by another name but ya this makes sense
| @@ -40,7 +40,8 @@ custom_preprocess={{ cifmw_bop_build_repo_dir }}/DLRN/scripts/set_nvr.sh,{{ cifm | |||
| info_files=osp.yml | |||
| versions_url=https://trunk.rdoproject.org/centos9-{{ cifmw_bop_openstack_release }}/current-podified/versions.csv | |||
| downstream_source_git_branch={{ cifmw_bop_osp_release }}-trunk-patches | |||
| downstream_distro_branch={{ cifmw_bop_osp_release }}{{ '.0' if '.' not in cifmw_bop_osp_release else '' }}-rhel-9-trunk | |||
| # downstream_distro_branch needs to be rhoso not rhos. | |||
| downstream_distro_branch={{ cifmw_bop_osp_release | replace('rhos', 'rhoso') }}{{ '.0' if '.' not in cifmw_bop_osp_release else '' }}-rhel-9-trunk | |||
There was a problem hiding this comment.
{{ '.0' if '.' not in cifmw_bop_osp_release else '' }}
so that is transformign 18 to 18.0 but it it was already 18.0.4 it would not modify it
makes sense
| @@ -145,25 +145,15 @@ | |||
| - "'host' in _change" | |||
| - "'redhat.com' in _change.host" | |||
| block: | |||
| - name: Clone the downstream project | |||
| - name: Clone the downstream project from zuul clonned repo | |||
There was a problem hiding this comment.
IMHO Tying this role even more to zuul is a strategic mistake
There was a problem hiding this comment.
i strongly disagree.
the fact that this role was ever cloning code is a huge red flag.
our ci is meant to depend on zuul providing repos with all relevant patches pre merged by the zuul executor.
the current logic for making depend on work after the fact is flawed and only exist because of the fact we do not use zuul to prepare all source repos.
i was originally going to go further and ask chandan to remove the clonign fo repos form external sources entirely and only use the zuul provided source code which is the upstream best practice for opendev zuul.
we make it a hard error by default if devstack ever clones a repo for example as it breaks that pardime.
part of the reason we write jobs to let zuul clone the relvent code is to ensure all jobs in any given build set are executing on the same code.
we do get a similar effect by using content providers but its still better to avoid the extra network load and ci time required to have each job clone form upstream/downstream repos during the build.
There was a problem hiding this comment.
Thank you Pablo for the comments. I am not getting how relying on zuul would cause issues to this role.
Based on Sean's comment I am totally agreeing with him. We should rely on zuul provided repos. If an role's user populate the repos just like zuul does and construct zuul like vars then the role will work flawlessly.
Please let us know if anything we can do to address your concern.
There was a problem hiding this comment.
Maybe my message was way too short, cause I'm not pushing to clone the repos by ourselves, I agree with both of you on that. I was just saying that, if the code is already checked out by an external tool (not the cifmw, zuul, for example) why we need to checkout? In this case, if the job is properly configured the repos should be already present in the instance, if a depends-on exists zuul already patched that checked out content, etc. With this checkout here we may loose the depends-on, for example. So, tl;dr: Why we do not just copy the content from the directory that zuul uses?
| ansible.builtin.git: | ||
| repo: '{{ _change.host }}/gerrit/{{ _change.project }}' | ||
| repo: '{{ ansible_user_dir }}/src/{{ _change.host | split("://") | last }}/{{ _change.project }}' |
There was a problem hiding this comment.
If we are using zuul as input, why not pick the source from it's metadata? It's already a git repo, so no need to checkout.
There was a problem hiding this comment.
We construct zuul_change_list like this
2025-02-04 10:14:38.069014 | controller | TASK [build_openstack_packages : Print Zuul change list var=cifmw_bop_change_list] ***
2025-02-04 10:14:38.069019 | controller | Tuesday 04 February 2025 10:14:38 -0500 (0:00:00.120) 0:05:51.056 ******
2025-02-04 10:14:38.069030 | controller | ok: [localhost] =>
2025-02-04 10:14:38.162668 | controller | cifmw_bop_change_list:
2025-02-04 10:14:38.162698 | controller | - branch: rhos-18.0-trunk-patches
2025-02-04 10:14:38.162704 | controller | change: '4'
2025-02-04 10:14:38.162709 | controller | host: <redacted>
2025-02-04 10:14:38.162714 | controller | project: <project_ID>
2025-02-04 10:14:38.162718 | controller | refspec: refs/changes/4/4/a1224a5f2a947bd3d7216b59e74f03492aa145ec
Based on above data, we perform further operations like clonning the repo and building the package using DLRN.
Since for downstream we are going to rely on zuul clonned repo (in future, we may add the same process for upstream gerrit and github). Here, We need to construct the repo name so that we can get the source of zuul metadata. I tried to modify the code on same line. I hope address the above concern.
There was a problem hiding this comment.
I was referring to, why do we craft the src directory assuming that zuul clones using that format instead of using the src_dir property that zuul uses to expose where's the content cloned?
| loop: | ||
| - "{{ cifmw_bop_osp_release }}-rhel-9-trunk" | ||
| - "{{ cifmw_bop_osp_release }}-trunk-patches" | ||
| version: "{{ cifmw_bop_osp_release }}-trunk-patches" |
There was a problem hiding this comment.
Hardcoding -trunk-patches is just asking for future maintenance burden.
raukadah
force-pushed
the
fix_downstream
branch
from
19cafd5 to
54d2518
Compare
|
New changes are detected. LGTM label has been removed. |
This pr: * Since downstream gerrit repos is moved to gitlab. It drops downstream gerrit support and adds gitlab support. * Use the zuul clonned content instead of clonning from gerrit or gitlab. As zuul already checks out the source code. Let's reclone it for dlrn purpose. * Drop branch creation task as it is no longer needed. * Use drop provided by the change itself. * Update patches branches to gitlab for gitlab related repos. Currently we have downstream openstack code on gerrit and gitlab. * For RHOS-18, downstream distgit uses rhoso-18 not rhos. It also updates the project.ini to reflect the same. Signed-off-by: Chandan Kumar (raukadah) <[email protected]> Related-Jira: https://issues.redhat.com/browse/OSPRH-13660
raukadah
force-pushed
the
fix_downstream
branch
from
54d2518 to
3f728b2
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/ec691c986b3e4bb5b68b8af49aecaa7a ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 46m 26s |
|
recheck unknown failure on cifmw-molecule-build_openstack_packages |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/65d3291ea3a64b8b8a27c69c760db55a ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 40m 13s |
This pr:
Since downstream gerrit repos is moved to gitlab. It drops downstream gerrit support and adds gitlab support.
Use the zuul clonned content instead of clonning from gerrit or gitlab. As zuul already checks out the source code. Let's reclone it for dlrn purpose.
Drop branch creation task as it is no longer needed.
Update patches branches to gitlab for gitlab related repos. Currently we have downstream openstack code on gerrit and gitlab.
For RHOS-18, downstream distgit uses rhoso-18 not rhos. It also updates the project.ini to reflect the same.
Related-Jira: https://issues.redhat.com/browse/OSPRH-13660