Skip to content

Conversation

pmtk
Copy link
Member

@pmtk pmtk commented Apr 15, 2024

When ci-operator clones the repository it does not include any credentials to talk back to the repository.
To be able to execute git push we need to add a special remote that will use token that is either supplied (personal) or generated (Installation Access Token, IAT).

@openshift-ci-robot
Copy link

@pmtk: No Jira issue with key USHFT-2969 exists in the tracker at https://issues.redhat.com/.
Once a valid jira issue is referenced in the title of this pull request, request a refresh with /jira refresh.

In response to this:

When ci-operator clones the repository it does not include any credentials to talk back to the repository.
To be able to execute git push we need to add a special remote that will use token that is either supplied (personal) or generated (Installation Access Token, IAT).

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 openshift-eng/jira-lifecycle-plugin repository.

@pmtk
Copy link
Member Author

pmtk commented Apr 15, 2024

/assign @dhellmann

@openshift-ci openshift-ci bot requested review from dhellmann and ggiguash April 15, 2024 09:01
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 15, 2024
env = {}
env.update(os.environ)

print(f'git remote remote {REMOTE}')
Copy link
Contributor

@ggiguash ggiguash Apr 15, 2024

Choose a reason for hiding this comment

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

Suggested change
print(f'git remote remote {REMOTE}')
print(f'git remote remove {REMOTE}')

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, thx!

@@ -404,6 +407,28 @@ def tag_exists(release_name):
return False


def add_token_remote():
"""
Returns the Git remote for the given repository using
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Returns the Git remote for the given repository using
Adds the Git remote to the given repository using

@@ -486,14 +511,18 @@ def publish_release(new_release, take_action):
# Create draft release with message that includes download URLs and history
try:
github_release_create(release_name, notes)
except urllib.error.URLError as e:
print(f"Failed to create the release {release_name}: {e}")
print(f"Response: {str(e.fp.readlines())}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add response printout the to catch-all handler?

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 think it's a different type so it doesn't have these fields

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain when this exception is thrown?

Copy link
Member Author

@pmtk pmtk Apr 15, 2024

Choose a reason for hiding this comment

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

For me it when I tested and the release was already present. Original log would only say: Failed to create the release 4.16.0-ec.1-202401101824.p0: HTTP Error 422: Unprocessable Entity and the hint that there already was a release was in the response body of the HTTP request.
But I suspect this exception could happen any other time that HTTP request wasn't successful (i.e. 200) and without response's body we have not enough info to work on it.

@pmtk pmtk force-pushed the release-notes-job/use-token-for-git-commands branch 2 times, most recently from 926b1e2 to 6cec94b Compare April 15, 2024 10:09
@pmtk
Copy link
Member Author

pmtk commented Apr 15, 2024

/retest

@pmtk pmtk force-pushed the release-notes-job/use-token-for-git-commands branch from 6cec94b to 4854bfb Compare April 16, 2024 07:36
pmtk added 3 commits April 16, 2024 09:39
When ci-operator clones the repository it does not
include any credentials to talk back to the repository.
To be able to execute `git push` we need to add a special
remote that will use token that is either supplied (Personal)
or generated (installation access token).
@pmtk pmtk force-pushed the release-notes-job/use-token-for-git-commands branch from 4854bfb to 8874b1a Compare April 16, 2024 07:39
@pmtk
Copy link
Member Author

pmtk commented Apr 16, 2024

/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.

/lgtm

)
if result.returncode != 0:
err = result.stderr.replace(GITHUB_TOKEN, "") if result.stderr else "stderr is empty"
raise Exception(f"Command `git remote add` failed: {err}")
Copy link
Contributor

Choose a reason for hiding this comment

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

You could use CalledProcessError, like on line 459, but I wouldn't hold this up over it.

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 it, but didn't quite fit here - I didn't want to put cmd in there and redact two things

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 16, 2024
Copy link
Contributor

openshift-ci bot commented Apr 16, 2024

[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

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 871297a and 2 for PR HEAD 8874b1a in total

Copy link
Contributor

openshift-ci bot commented Apr 16, 2024

@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-bot openshift-merge-bot bot merged commit 9af9ccd into openshift:main Apr 16, 2024
@pmtk pmtk deleted the release-notes-job/use-token-for-git-commands branch April 24, 2024 08:09
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