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

Try the k8s prow bot to configure per component owners with merge rights #7063

Closed
bogdandrutu opened this issue Jan 6, 2022 · 30 comments
Closed
Assignees
Labels
ci-cd CI, CD, testing, build issues closed as inactive Stale

Comments

@bogdandrutu
Copy link
Member

I think we should try for at least few months the k8s bot that offers support for OWNERS.

I believe that we don't need the full functionality of the praw bot, but definitely we need the support for OWNERS files and automation to merge PRs based on OWNERS approvers/reviewers.

@codeboten @jpkrohling @tigrannajaryan what do you think?

@bogdandrutu bogdandrutu changed the title Try to k8s praw bot to configure per component owners with merge rights Try the k8s praw bot to configure per component owners with merge rights Jan 6, 2022
@codeboten
Copy link
Contributor

I looked briefly at praw, would be happy to try it out and see how it can help our workflows.

@tigrannajaryan
Copy link
Member

I don't mind trying. It may be worth listing exactly what we expect the bot to do, which subset of functionality we will use.

@bogdandrutu
Copy link
Member Author

@tigrannajaryan I definitely want to be able to do have the same experience as https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#code-review-using-owners-files. We don't need to use praw jobs for our CI/CD.

@anuraaga anuraaga changed the title Try the k8s praw bot to configure per component owners with merge rights Try the k8s prow bot to configure per component owners with merge rights Jan 7, 2022
@alolita alolita added the ci-cd CI, CD, testing, build issues label Jan 7, 2022
@alolita
Copy link
Member

alolita commented Jan 7, 2022

Hi @tigrannajaryan @codeboten @jpkrohling - I discussed enabling this bot for contrib component owners w Bogdan.
Are you good with us enabling this on contrib?

I'd like to setup and evaluate this tool to see if it helps improve velocity on reviews, ownership responsibilities, and help enable automated merges. I would like to see this run for a trial run for a couple of months.

@tigrannajaryan
Copy link
Member

Works for me.

@codeboten
Copy link
Contributor

Yup, happy to try it

@jpkrohling
Copy link
Member

Me too, but don't we need a machine to host this tool? Who's going to maintain our prow instance?

@bogdandrutu
Copy link
Member Author

@jpkrohling I think you are correct, @dashpole I think we may need a GCP machine for this (not sure works in a different cloud) correct? @dashpole would you be able to help us with maintain/getting a resource?

@dashpole
Copy link
Contributor

Prow works on any kubernetes cluster, so it isn't GCP-specific. If we aren't using prow to run tests, I don't think we need any other integrations (log storage can integrate with gcs or azure, but I don't think we need it). @SergeyKanzhelev might know what a process is for getting a kubernetes cluster to run prow would be.

If it is just as a brief experiment, I can spin something up for a week so we can try it. In the past when i've maintained gcp-owned resources for OSS projects, gcp ownership has made it hard for other contributors to use. If we plan to use prow longer-term, we should get a community-owned project.

@jpkrohling
Copy link
Member

jpkrohling commented Jan 10, 2022

It shouldn't be hard to get a Kubernetes cluster sponsored by the CNCF. My main concerns right now are around maintaining that cluster. The engineering costs might be just too high for the benefits... Aren't there other tools that would be more suited for this, if all we are looking for is to automate/customize assigning issues and PRs to folks? I think mergify had a good solution in the past, but I haven't used it in a while.

@bryan-aguilar
Copy link
Contributor

Hi all, I am new to the AWS team and haven't had a chance to introduce myself in any SIG meetings yet. @alolita pointed me to this issue and I volunteered to take a look at it. I haven't had a chance to dig into it too deep yet but @jpkrohling is right and we would need a sponsored cluster.

I also had initial questions about maintainability requirements in the future. I would be happy to look into it this week and present whatever I find in a future SIG meeting. I think it would also be a good idea to see if there are any other solutions that would be better suited as @jpkrohling brought up.

Let me know your thoughts, and I look forward to working with all of you.

@jpkrohling
Copy link
Member

I think it would also be a good idea to see if there are any other solutions that would be better suited

Would you be interested in doing some research on the topic and presenting your findings during the SIG meeting?

@bryan-aguilar
Copy link
Contributor

bryan-aguilar commented Jan 10, 2022

I think it would also be a good idea to see if there are any other solutions that would be better suited

Would you be interested in doing some research on the topic and presenting your findings during the SIG meeting?

Yup, not a problem.

@bogdandrutu
Copy link
Member Author

bogdandrutu commented Jan 11, 2022

@jpkrohling I am not attached to prow bot, but I think they have the best workflow, so we should have the same workflow and capabilities.

The engineering costs might be just too high for the benefits... Aren't there other tools that would be more suited for this, if all we are looking for is to automate/customize assigning issues and PRs to folks? I think mergify had a good solution in the past, but I haven't used it in a while.

Not sure what you think will be to maintain, I think once we setup something (if possible to use GKE or other vendor k8s distribution) we don't have too many things to maintain, once in a while update the bot... Maybe I am simplifying things too much.. As I mentioned if we get the same capabilities easier, always happy to go with that.

@SergeyKanzhelev
Copy link
Member

I would be interested to see how experiment will go. It is a big disappointment that the built-in CODEOWNERS doesn't provide enough features to effectively represent the components maintainership.

@anuraaga
Copy link
Contributor

anuraaga commented Jan 12, 2022

FYI this GitHub-based approach looks like it could be a simple way to get the chat-ops functionality without external infra

https://github.com/jpmcb/prow-github-actions

@SergeyKanzhelev
Copy link
Member

Do you know if it supports ONWERS file and PRs spanning across multiple components of owners files? I quickly scanned it and didn't find this support. The need for many chatops capabilities like applying labels may be solved by extending the triagers group (less spam from those comments :-))

@bryan-aguilar
Copy link
Contributor

Do you know if it supports ONWERS file and PRs spanning across multiple components of owners files? I quickly scanned it and didn't find this support. The need for many chatops capabilities like applying labels may be solved by extending the triagers group (less spam from those comments :-))

I also stumbled across this tool and no it doesn't which is one of the main drawbacks.

@bryan-aguilar
Copy link
Contributor

Notes from 1/12/22 collector sig meeting:

Initial research doc

  • Prow is powerful but looking for volunteers to maintain CI is a big ask.
  • Explore mergify deeper and perform short term test.
  • Evaluate after ~30d to see if mergify increases productivity in PR workflow.

@bryan-aguilar
Copy link
Contributor

Shortly after the SIG meeting I discovered https://www.pullapprove.com/ . Another SaaS product but which is more focused on the PR assignment workflow. I believe that this provides stronger rule based functionality than mergify and should be experimented with first. I will work on a proof of concept that can be presented to the larger group for feedback/approval.

@anuraaga
Copy link
Contributor

Thanks for looking into this everyone. Just wondering if we could start by listing out the features that we want? I see the table in the attached doc but it's still not quite clear what the actual features needed are - I guess it's just having a list of reviewers that get auto assigned, and detect comments with / commands, check the originator against that list, and execute them, but just a guess. Presumably CODEOWNERS then is only the SIG maintainers team, one line, and the component owners are defined in a separate file.

FWIW, I believe it would be a very small amount of code, to write a GitHub action for this, the only logic seems to be

  1. Determine list of owners from file for autoassigning. This is already done with @dyladan's action and could be copy-pasted https://github.com/dyladan/component-owners

  2. Determine if a PR comment's author is in the list of owners. If so, execute the desired command based on the text in the comment.

The gh CLI makes it trivial to execute actions such as assigning reviewers or merging PRs. All of the required logic seems to be determine list of owners and/or check against list of owners. While all of this could be packaged into a small sized JS action, I believe it should be possible with just a bit of yq + gh CLI (yq is preinstalled on GitHub runners https://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu2004-Readme.md#tools)

I think OSS projects tend to have long term issues when using SaaS services, at the lowest level just sharing the admin key isn't trivial, and unless there is a really strong reason to do so, we would probably want to avoid it in favor of using GitHub itself.

@bogdandrutu
Copy link
Member Author

@anuraaga we discussed this in the SIG meeting. Essentially we want the features that OWENRS in k8s gives https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md. Here is a nice description of the workflow https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#the-code-review-process

@bogdandrutu
Copy link
Member Author

bogdandrutu commented Jan 20, 2022

@anuraaga as a summary, if you give me something that I can do at least Phase 0,1,2 and Phase 3 as optional from https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#the-code-review-process I think we can enable it :D

@anuraaga
Copy link
Contributor

Thanks @bogdandrutu that makes the requirements very clear. We will look into a solution, hopefully without any extra infrastructure than GitHub

@bryan-aguilar
Copy link
Contributor

Hi all, just wanted to provide an update that I am still looking into this. Thank you @bogdandrutu for the explicit requirements. I already did a first pass at examining what a PullApprove workflow would look like but the doc should be touched up before sharing. Next, I'll need to do some research based on @anuraaga comments to see the viability of creating our own workflow using GitHub actions. I am just trying to finish up some other tasks before refocusing on this issue.

@dyladan
Copy link
Member

dyladan commented Jan 21, 2022

@bryan-aguilar would be great if you or someone from this SIG could present what you come up with to the maintainer meeting. I know myself and i'm sure the other maintainers would love to be able to spread ownership around contrib a little more easily.

@bogdandrutu
Copy link
Member Author

@alolita you told me that you have updates on this... can you please update this issue? Would like to enable this to make the repo more self service

@aanm
Copy link

aanm commented Oct 28, 2022

@bryan-aguilar out of curiosity, do you have an update on this?

@github-actions
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

@github-actions github-actions bot added the Stale label Dec 28, 2022
@github-actions
Copy link
Contributor

This issue has been closed as inactive because it has been stale for 120 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-cd CI, CD, testing, build issues closed as inactive Stale
Projects
None yet
Development

No branches or pull requests