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

USHIFT-549 USHIFT-679: Python script for rebase job #1223

Merged
merged 1 commit into from
Jan 18, 2023

Conversation

pmtk
Copy link
Member

@pmtk pmtk commented Dec 27, 2022

Wraps rebase.sh with extra goodies for better interactions with PRs.
Checks rebase.sh exit code, so case of error the logs is committed along of any other staging changes, PR's title will get *FAILURE* prefix and PR's description will have big (h1) message to check committed rebase.sh log.
If PR exists for the branch (which name is amd64's release tag), PR desc will be updated.
PR description now includes: amd64 and arm64 release tags, and (deduced) prow job link.

Obsoletes create_pr.py

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 27, 2022
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 27, 2022
@pmtk pmtk force-pushed the rebase-python-wrapper branch 2 times, most recently from 45b8824 to 2a1b8d3 Compare January 13, 2023 13:50
@pmtk pmtk changed the title WIP rebase.py USHIFT-549 USHIFT-679: Python script for rebase job Jan 13, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 13, 2023
@pmtk pmtk force-pushed the rebase-python-wrapper branch 3 times, most recently from 0cfecc6 to 4ddcaec Compare January 16, 2023 13:04
@pmtk
Copy link
Member Author

pmtk commented Jan 16, 2023

/retest

Copy link
Contributor

@dhellmann dhellmann left a comment

Choose a reason for hiding this comment

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

A couple of small suggestions, but nothing that should hold this up.

result = subprocess.run(args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, universal_newlines=True)
logging.info(f"Output:\n\n{result.stdout}\n------------------------------\n")
logging.info(f"Script returned code: {result.returncode}")
rr = namedtuple("RebaseResult", ["success", "output"])
Copy link
Contributor

Choose a reason for hiding this comment

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

This type really should be defined outside of the function so it's clear it's used elsewhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

matching_branches = ", ".join([r.name for r in matching_remote_refs])
logging.warning(f"Found more than one branch matching '{branch_name}' on remote: {matching_branches}. Taking first one")
_extra_msgs.append(f"Found more than one branch matching '{branch_name}' on remote: {matching_branches}.")
return matching_remote_refs[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

Are the duplicate branches returned in a deterministic order?

Copy link
Member Author

@pmtk pmtk Jan 17, 2023

Choose a reason for hiding this comment

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

Refs must match exactly remote/branch-name and you can't have two branches named the same (or can you?) so I don't think this would ever happen but wanted to check all possibilities just in case.



def generate_pr_description(branch_name, amd_tag, arm_tag, prow_job_url, rebase_script_succeded):
base = (f"amd64: {amd_tag}\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can add the changelog contents from #1255 here, too, when that work is done.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good idea

@@ -688,7 +688,7 @@ rebase_to() {
exit 1
fi

rebase_branch="rebase-${ver_stream}+amd64-${amd64_date}+arm64-${arm64_date}"
rebase_branch="rebase-${ver_stream}_amd64-${amd64_date}_arm64-${arm64_date}"
Copy link
Member Author

Choose a reason for hiding this comment

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

I noticed that + are not working good with prow's github query as they're used to join filters together, example:

is:pr+repo:openshift/microshift+author:microshift-rebase-script+head:rebase-4.12.0-0.nightly+amd64-2023-01-10-062211+arm64-2023-01-12-221036

@pmtk pmtk changed the title USHIFT-549 USHIFT-679: Python script for rebase job USHIFT-549 USHIFT-679: Python wrapper for rebase job Jan 17, 2023
@pmtk pmtk changed the title USHIFT-549 USHIFT-679: Python wrapper for rebase job USHIFT-549 USHIFT-679: Python script for rebase job Jan 17, 2023
Copy link
Contributor

@dhellmann dhellmann left a comment

Choose a reason for hiding this comment

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

/lgtm

Nice!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 18, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 18, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhellmann, pmtk

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

The pull request process is described 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

@ggiguash
Copy link
Contributor

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 18, 2023

@pmtk: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit ff29348 into openshift:main Jan 18, 2023
@pmtk pmtk deleted the rebase-python-wrapper branch January 30, 2023 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants