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

Configure the git resolver with the GitHub API token #1333

Merged

Conversation

abayer
Copy link
Contributor

@abayer abayer commented Jan 31, 2023

Changes

I'm not 100% sure if this is right - specifically, this is trying to write to the git-resolver-config ConfigMap in the tekton-pipelines-resolvers namespace, but I put it in the same directory as the rest of the pipelines overlays. @afrittoli - I think this should work correctly, but I defer to you. =)

/kind misc

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

See the contribution guide
for more details.

@abayer abayer added the kind/misc Categorizes issue or PR as a miscellaneuous one. label Jan 31, 2023
@tekton-robot tekton-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 31, 2023
@abayer
Copy link
Contributor Author

abayer commented Jan 31, 2023

/assign @afrittoli

@abayer
Copy link
Contributor Author

abayer commented Feb 2, 2023

/test check-pr-has-kind-label

@tekton-robot
Copy link
Contributor

@abayer: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test pull-dogfooding-mario-test
  • /test pull-tekton-plumbing-check-testgrid-config

In response to this:

/test check-pr-has-kind-label

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.

@abayer
Copy link
Contributor Author

abayer commented Feb 2, 2023

/assign @dibyom @lbernick @vdemeester

abayer added a commit to abayer/tektoncd-pipeline that referenced this pull request Feb 2, 2023
fixes tektoncd#6091

(this can be merged before tektoncd/plumbing#1333, but it'd definitely be beter for that Plumbing PR to merge first)

This switches how we resolve the `publish-release` `Task` in `release-pipeline` from an anonymous git clone to using the GitHub API. The full clone approach is almost always timing out, in part thanks to tektoncd#6025, but even if it finished successfully every time, it'd still be adding at least a minute of extra time to the pipeline execution for no particularly good reason. Using the GitHub API-based resolution instead means no clone is needed, with the resolver just making a couple very specific API calls to get the contents of the specified file. So yeah, much faster!

Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
@lbernick
Copy link
Member

lbernick commented Feb 2, 2023

@abayer is there a way to test this/have you tested this? (maybe at least build w/ kustomize and see what the diff is compared to the dogfooding cluster?)

Copy link
Member

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

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

@abayer
Copy link
Contributor Author

abayer commented Feb 2, 2023

@abayer is there a way to test this/have you tested this? (maybe at least build w/ kustomize and see what the diff is compared to the dogfooding cluster?)

I'm asking @afrittoli about this in the productivity WG in a few minutes. =)

Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
@abayer abayer force-pushed the configure-git-resolver-api-token branch from 4f7f3d1 to 89dcc40 Compare February 2, 2023 17:58
@abayer
Copy link
Contributor Author

abayer commented Feb 2, 2023

kustomization has now been updated!

@abayer
Copy link
Contributor Author

abayer commented Feb 3, 2023

And I've verified (after copying https://storage.googleapis.com/tekton-releases/pipeline/previous/v0.44.0/release.yaml into tekton/cd/pipeline/base) that the output of kubectl kustomize tekton/cd/pipeline/overlays/dogfooding does contain the expected git-resolver-config configmap, so yeah, this is good to merge.

Copy link
Member

@jerop jerop left a comment

Choose a reason for hiding this comment

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

thanks @abayer

@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerop

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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 3, 2023
@lbernick
Copy link
Member

lbernick commented Feb 3, 2023

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 3, 2023
@tekton-robot tekton-robot merged commit 365f810 into tektoncd:main Feb 3, 2023
tekton-robot pushed a commit to tektoncd/pipeline that referenced this pull request Feb 3, 2023
fixes #6091

(this can be merged before tektoncd/plumbing#1333, but it'd definitely be beter for that Plumbing PR to merge first)

This switches how we resolve the `publish-release` `Task` in `release-pipeline` from an anonymous git clone to using the GitHub API. The full clone approach is almost always timing out, in part thanks to #6025, but even if it finished successfully every time, it'd still be adding at least a minute of extra time to the pipeline execution for no particularly good reason. Using the GitHub API-based resolution instead means no clone is needed, with the resolver just making a couple very specific API calls to get the contents of the specified file. So yeah, much faster!

Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
JeromeJu pushed a commit to JeromeJu/pipeline that referenced this pull request Feb 6, 2023
fixes tektoncd#6091

(this can be merged before tektoncd/plumbing#1333, but it'd definitely be beter for that Plumbing PR to merge first)

This switches how we resolve the `publish-release` `Task` in `release-pipeline` from an anonymous git clone to using the GitHub API. The full clone approach is almost always timing out, in part thanks to tektoncd#6025, but even if it finished successfully every time, it'd still be adding at least a minute of extra time to the pipeline execution for no particularly good reason. Using the GitHub API-based resolution instead means no clone is needed, with the resolver just making a couple very specific API calls to get the contents of the specified file. So yeah, much faster!

Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
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. kind/misc Categorizes issue or PR as a miscellaneuous one. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants