Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Need replacement for PullApprove functionality #98

Closed
grahamwhaley opened this issue Jan 8, 2019 · 16 comments
Closed

Need replacement for PullApprove functionality #98

grahamwhaley opened this issue Jan 8, 2019 · 16 comments

Comments

@grahamwhaley
Copy link
Contributor

PullApprove are changing their terms of service, which means we likely will not be able to continue using the service as part of our Kata Containers CI PR check flows.

We need to locate and evaluate alternative options. I have placed an overview of the services we use and some potential options over on the community wiki at:
https://github.com/kata-containers/community/wiki/PullApproveAlternatives

Please add/update as appropriate so we can choose and plan to replace the service in as smooth and functional manner as possible.

@kata-containers/architecture-committee
@ttx
@ydjainopensource
@jodh-intel @chavafg

@grahamwhaley
Copy link
Contributor Author

I think we are coming to a conclusion/plan. @kata-containers/architecture-committee - may wish to discuss or chip in if we think this is not the way forwards. We can always discuss in a Monday arch call.

Here is a summary then:

@ttx and I will look into the Zuul aspects (see #99)
I will go look at the github ack setup and codeowners over on the proxy repo so we can evaluate.

@grahamwhaley
Copy link
Contributor Author

For the github group/team ack counting, see kata-containers/proxy#138 for an example of the github CODEOWNERS process in action to enforce a documentation review on a PR.

@jodh-intel
Copy link
Contributor

@grahamwhaley - aiui, we have 2 days left before pullapprove change their t+c's. Could you paste an update here on the current thinking?

@ttx
Copy link
Member

ttx commented Jan 29, 2019

@jodh-intel - @grahamwhaley and I are trying to have everything in place in time, but that leaves little time for testing and I'd rather not rush it. Maybe we could switch to a "standard" account for February to give us a few more weeks to transition ? Who has access to that account ? Would we get 14 days to "try" it ?

@jodh-intel
Copy link
Contributor

Understood but I think it would be good to outline what we've been up to on this issue. Throwing a link to the ML thread to avoid duplication:

Yeah, it does appear you get 14 days free on a standard a/c, but I don't know who has the authority to set that up and deal with T+C's, etc.

/cc @egernst.

@grahamwhaley
Copy link
Contributor Author

I've just added 'pullapprove replacement update' to todays arch call agenda, where @ttx and I can give an update.
We are tracking as above - github for acks and Zuul for SoB and WIP checking.
The github acks can (could) be seen in action over at kata-containers/proxy#140 where we got both a '2 acks needed' and the docs group added to the review group. If we agree in the arch call that that is doing the job, then I can roll that out across all the repos with docs approval required.

On the Zuul front, we think we have the solution for both SoB and WIP checking (for labels at least initially). The main thing we need now is the Zuul tenant for kata set up.

wrt account ownership for pullapprove - I think all accounts of any sort related to Kata should have their details lodged with the OSF (probably you then @ttx) and available to the arch committee :-) So, let's find the details and pass them on if we can.

@ttx
Copy link
Member

ttx commented Jan 29, 2019

Yes -- the set up of the Kata tenant in Zuul is taking some extra time (it's the first non-OpenStack tenant so they need to de-OpenStack a number of things, like the base jobs). The good news is that it will give Kata much more control on how the tests show up. The bad news is that it's unlikely to be fully set up and tested by February 1st.

I'm happy to help with setting up the "standard" account from an OSF perspective. I logged into PullApprove as @ttx -- feel free to add me as one of the account holders if that is possible...

@jodh-intel
Copy link
Contributor

/cc @sameo as I think(?) he originally setup pullapprove for CC so might have thoughts on this.

@ttx - I've just invited you to join the github org. I'm not sure how we forgot to do this sooner! You, like @jbryce, are now an "owner" so have full super-powers which you need for setting up things like github integrations / webhooks afaik. Feel the force, but please don't break anything! 😄

/cc @egernst, @gnawux

@grahamwhaley
Copy link
Contributor Author

Looks like pullapprove has expired now @ttx (thanks for noticing @jodh-intel ). Showing the text:

Please update billing information for kata-containers

at
kata-containers/tests#1120
https://pullapprove.com/kata-containers/tests/pull-request/1120/

I guess we need to make a decision

  1. are we near enough to a functioning Zuul tenant that we can cope with a short period or no-pullapprove (and start disabling it across the repos), OR
  2. Do we need to figure out how to temporarily fund or re-setup a pullapprove account to cover the interim?

@ttx
Copy link
Member

ttx commented Feb 4, 2019

It's almost there but I think we should pay for the first month to ensure a smooth transition... If nobody has a clear button to push I can try re-adding the Kata repositories, see if that gives me a place to enter billing data.

@grahamwhaley
Copy link
Contributor Author

As an update then... @ttx has enabled 1 month of billing for pullapprove, and that looks like it is linked to the kata repos... but, I'm not sure 'open PRs' have transitioned smoothly over to the billing. afaict, all open PRs are still getting the 'please update your billing' label. So, let's all keep an eye on that and:

  • see if the open PRs still get ack'd when they meet the pullapprove requirements
  • see what happens with new PRs

and debug as we go..

@jodh-intel
Copy link
Contributor

I tried the ol' "sync PR" trick... but no 🍌 😞

@grahamwhaley
Copy link
Contributor Author

An update for folks.
@ttx and I (mostly @ttx) have today been working on re-enabling pullapprove in the interim until we get Zuul tenant equivalence in place.
Due to some, shall we say, 'technical difficulties', we have had to limit the number of kata repositories that we can enable pullapprove on right now.
To that end, we current do have pullapprove for the following repos:

  • runtime
  • tests
  • agent
  • proxy
  • documentation

All other repos have pullapprove disabled. What that means for those repos right now I believe is, predominantly, the lack of:

  • 'Signed-off-by' checking
  • WIP/DNM label and subject keyword checking

I have re-sync'd all the repos that are enabled with pullapprove and hope that has updated all the pullapprove status' on the currently affected PRs.

If you see a problem with pullapprove on kata right now, then please contact @ttx and myself (either on this ticket, on slack/irc or via the mailing list), and we will do our best to sort it out.

We may have to force-merge some PRs past pullapprove right now. Let's just be sensible and ensure we do due diligence on our reviewing...

@grahamwhaley
Copy link
Contributor Author

@ttx - I think we can close this one as well now?

@ttx
Copy link
Member

ttx commented Feb 12, 2019

Yes.

@ttx ttx closed this as completed Feb 12, 2019
GabyCT pushed a commit to GabyCT/ci that referenced this issue Feb 12, 2019
@grahamwhaley
Copy link
Contributor Author

Although we have implemented solutions today, and closed this Issue, as an addend, github have added a 'draft PR' feature, that could suppliment/replace our WIP processes:
https://github.blog/2019-02-14-introducing-draft-pull-requests/
In fact, the feature is active and works today without us needing to do anything for our org or repos.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants