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

Revert "Use zuul clonned repos for upstream in build_openstack_packages role" and revers zuul changes list #2818

Merged
merged 2 commits into from
Mar 25, 2025

Conversation

amoralej
Copy link
Contributor

@amoralej amoralej commented Mar 21, 2025

This is breaking builds with downstream driver in two ways:

  • Not using --local requires the repos to have proper remote definition.
  • pre-run scripts as patch_rebaser [1] also require proper remotes.

For the first, we may handle it via change in dlrn [2], but for the second one I'm not sure about the best way.

[1] https://github.com/release-depot/patch_rebaser/
[2] https://softwarefactory-project.io/r/c/DLRN/+/33366

This reverts commit f79ca6f.

After this revert we still have problems when building multiple in-flight patches in any form:

  1. Having multiple depends-on on the same package does not work.
  2. Piling multiple reviews and adding the depends-on to the top one is
    also not working.

This PR is also fixing (2) by reversing the order of the related zuul changes to find and process the top change for the repo.

…es role"

This is breaking builds with downstream driver in two ways:

- Not using --local requires the repos to have proper remote definition.
- pre-run scripts as patch_rebaser [1] also require proper remotes.

For the first, we may handle it via change in dlrn [2], but for the
second one I'm not sure about the best way.

[1] https://github.com/release-depot/patch_rebaser/
[2] https://softwarefactory-project.io/r/c/DLRN/+/33366

This reverts commit f79ca6f.
@amoralej amoralej requested a review from a team as a code owner March 21, 2025 11:13
Copy link
Contributor

openshift-ci bot commented Mar 21, 2025

[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 abays for approval. For more information see the Code Review Process.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@amoralej
Copy link
Contributor Author

@raukadah @karelyatin check this, please. Feel free to propose alternatives

@karelyatin karelyatin enabled auto-merge (rebase) March 21, 2025 11:39
@karelyatin
Copy link
Contributor

/lgtm

After reverting [1] we still have problems when building multiple
in-flight patches in any form:

1. Having multiple depends-on on the same package does not work.
2.  Piling multiple reviews and adding the depends-on to the top one is
   also not working.

This patch is reversing the order of the related zuul changes to fix the
case (2) as it will find and process the top change for the repo.
@openshift-ci openshift-ci bot removed the lgtm label Mar 21, 2025
@amoralej amoralej changed the title Revert "Use zuul clonned repos for upstream in build_openstack_packages role" Revert "Use zuul clonned repos for upstream in build_openstack_packages role" and revers zuul changes list Mar 21, 2025
@amoralej
Copy link
Contributor Author

@raukadah
Copy link
Contributor

/lgtm

Copy link
Contributor

@amartyasinha amartyasinha left a comment

Choose a reason for hiding this comment

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

lgtm

@karelyatin karelyatin merged commit 93b065d into openstack-k8s-operators:main Mar 25, 2025
8 checks passed
raukadah added a commit to raukadah/ci-framework that referenced this pull request Mar 25, 2025
In build_openstack_packages role for building rpms from gerrit and
github, we are clonning the repo and checking out specific change.

With these tasks, we are not able to build dlrn rpms if we have multiple
prs/crs from same project.

But we want to build rpms from all changes.
In order to fix, Zuul knows how to checkout proper repos
with changes under test and from Depends-on.

DLRN always looks for clonned repos in DLRN data directory.
In order to use zuul clonned sources with DLRN, We are creating
symlink to dlrn data project directory and let's dlrn do the job for
upstream repos only.

Note: we reverted the similar pr[1] here as original change was lacking
proper condition[2] leading to breaking downstream build openstack
packages role.

By adding proper conditional for upstream, it fixes the issue.

Links:
[1]. openstack-k8s-operators#2818
[2].

Signed-off-by: Chandan Kumar (raukadah) <[email protected]>
raukadah added a commit to raukadah/ci-framework that referenced this pull request Mar 25, 2025
In build_openstack_packages role for building rpms from gerrit and
github, we are clonning the repo and checking out specific change.

With these tasks, we are not able to build dlrn rpms if we have multiple
prs/crs from same project.

But we want to build rpms from all changes.
In order to fix, Zuul knows how to checkout proper repos
with changes under test and from Depends-on.

DLRN always looks for clonned repos in DLRN data directory.
In order to use zuul clonned sources with DLRN, We are creating
symlink to dlrn data project directory and let's dlrn do the job for
upstream repos only.

Note: we reverted the similar pr[1] here as original change was lacking
proper condition[2] leading to breaking downstream build openstack
packages role.

By adding proper conditional for upstream, it fixes the issue.

Links:
[1]. openstack-k8s-operators#2818
[2]. openstack-k8s-operators@f79ca6f#diff-fedeaff036de20345e170c4f65374926975f17139c058369f2e909565054e1adR118-L134

Signed-off-by: Chandan Kumar (raukadah) <[email protected]>
raukadah added a commit to raukadah/ci-framework that referenced this pull request Mar 25, 2025
In build_openstack_packages role for building rpms from gerrit and
github, we are clonning the repo and checking out specific change.

With these tasks, we are not able to build dlrn rpms if we have multiple
prs/crs from same project.

But we want to build rpms from all changes.
In order to fix, Zuul knows how to checkout proper repos
with changes under test and from Depends-on.

DLRN always looks for clonned repos in DLRN data directory.
In order to use zuul clonned sources with DLRN, We are creating
symlink to dlrn data project directory and let's dlrn do the job for
upstream repos only.

Note: we reverted the similar pr[1] here as original change was lacking
proper condition[2] leading to breaking downstream build openstack
packages role.

By adding proper conditional for upstream, it fixes the issue.

Links:
[1]. openstack-k8s-operators#2818
[2]. openstack-k8s-operators@f79ca6f#diff-fedeaff036de20345e170c4f65374926975f17139c058369f2e909565054e1adR118-L134

Signed-off-by: Chandan Kumar (raukadah) <[email protected]>
karelyatin pushed a commit that referenced this pull request Mar 26, 2025
In build_openstack_packages role for building rpms from gerrit and
github, we are clonning the repo and checking out specific change.

With these tasks, we are not able to build dlrn rpms if we have multiple
prs/crs from same project.

But we want to build rpms from all changes.
In order to fix, Zuul knows how to checkout proper repos
with changes under test and from Depends-on.

DLRN always looks for clonned repos in DLRN data directory.
In order to use zuul clonned sources with DLRN, We are creating
symlink to dlrn data project directory and let's dlrn do the job for
upstream repos only.

Note: we reverted the similar pr[1] here as original change was lacking
proper condition[2] leading to breaking downstream build openstack
packages role.

By adding proper conditional for upstream, it fixes the issue.

Links:
[1]. #2818
[2]. f79ca6f#diff-fedeaff036de20345e170c4f65374926975f17139c058369f2e909565054e1adR118-L134

Signed-off-by: Chandan Kumar (raukadah) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants