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

Roll out CLABot to Kubernetes #1

Closed
caniszczyk opened this issue Aug 17, 2016 · 20 comments
Closed

Roll out CLABot to Kubernetes #1

caniszczyk opened this issue Aug 17, 2016 · 20 comments
Labels

Comments

@caniszczyk
Copy link
Contributor

caniszczyk commented Aug 17, 2016

This issue will track progress of rolling out the CLA Bot fully to Kubernetes (see this issue for previous discussion: cncf/demo#7) and this Google doc (https://docs.google.com/document/d/1LIwyn5PDSYu8kh-7hH8688lt1cvwATkZmhec13nhp1c/edit#).

This blocks issue: kubernetes/kubernetes#27796

@caniszczyk
Copy link
Contributor Author

Hey @emsearcy, can you add https://github.com/kubernetes/pr-bot as another repo to the CNCF CLABot so we can move the testing into the Kubernetes organization.

@dankohn
Copy link
Contributor

dankohn commented Aug 17, 2016

Cc @bgrant0607 @foxish @monadic

@foxish
Copy link

foxish commented Aug 17, 2016

Thanks @dankohn. Chris and I have communicated via email and we've granted access to the pr-bot repository in the kubernetes org.

@foxish
Copy link

foxish commented Aug 18, 2016

Looks like our first step has been successful. Thanks @caniszczyk.
Before we start on implementing the label behavior in our bot, I'd like to discuss some aspects of your system to understand how they might work after we switch to the CNCF bot.

  1. When no CLA is found, we apply the cla: no label and wait for the person to sign and comment back with I signed it!.
  2. Is it possible to figure out from the status if there are multiple commits in a PR from different authors?

@emsearcy
Copy link

(sorry about the extra notifications)

An update for @dankohn from a meeting I had with @foxish.

  • webhooks are on target to be ready by the end of the week.
  • commenting on a PR to update the status should not be needed, as our system will re-check open PRs immediately upon users signing CLAs. That said, any update to a PR will cause the webhook to fire too
  • Currently, it is not possible from within the status API message we post to get granular information about multiple different failures (authors without CLAs or commits without authorship), however the status of each commit in the PR could be used to determine this (github doesn't have a special failure for the PR itself, it just shows the status of the last commit in the series for the PR).

@emsearcy
Copy link

(meant to send this yesterday but looks like I never hit "Comment")

Example pull request with multiple authors (pass and fail) including a commit lacking a GitHub author:
https://github.com/linuxfoundation-cla/webhook-test/pull/1

(Github only uses the status of the last commit in the series, so we propagate failures through to children commits, where the message for a given commit is the status of that commit, plus a count of failed authors from earlier commits in the patch series.)

Is this workable for the mungebot you are running to set labels and post messages?

@emsearcy
Copy link

OK, the pull request web service is live now. We have monitoring set up for it—if you want to monitor it yourself you can hit a simple ping responder which is separate from the github API endpoint but in the same Go service:
https://identity.linuxfoundation.org/lfcla/ping (should return 200, contents "OK")

cncf/demo and kubernetes/pr-bot have had the polling Perl script shutdown, and have been configured with a web hook so that the new LF CLA check will immediately post statuses.

I have not yet set it up to update the statuses when the user signs the CLA, so "I signed it" is, for the moment, still needed.

@foxish
Copy link

foxish commented Aug 27, 2016

@emsearcy Sounds good! The example PR you showed looks good as well. The mungebot can handle the labels and comments. We would just need the status to be set, which appears to be happening. I'll set up the bot to do the CLA labeling and commenting later today on pr-bot. Thanks.

cc @bgrant0607 @dankohn

@foxish
Copy link

foxish commented Aug 29, 2016

@emsearcy I just tried a test deployment on pr-bot. Please see https://github.com/kubernetes/pr-bot/pull/3#issuecomment-243205552
I signed the CLA, and commented on the PR (I signed it!) but I don't see any change in the status cla/linuxfoundation. The CLA page tells me I'm authorized to contribute. Shouldn't the webhook be firing immediately and updating that status?

cc @bgrant0607

@foxish
Copy link

foxish commented Aug 30, 2016

Update: In an email discussion, we found that there was an issue with the way the CLA check is implemented currently on the CNCF side. We will continue with deployment once the issues are addressed.

@foxish
Copy link

foxish commented Sep 6, 2016

@emsearcy @caniszczyk Any updates so far on fixing the issue we found in the CNCF contributor validation?

cc @dankohn

@dankohn
Copy link
Contributor

dankohn commented Sep 10, 2016

@emsearcy @caniszczyk Could you please provide an update on the status here?

@caniszczyk
Copy link
Contributor Author

I believe @emsearcy will followup soon, he was out on vacation for the last week.

Thanks for your patience.

@emsearcy
Copy link

Thank you for your patience. I've been communicating with our developer team on this and they have completed implementing and testing a hotfix for the issue. They'll deploy it 6pm EDT today.

@foxish
Copy link

foxish commented Sep 20, 2016

@emsearcy Is the fix live at this point? Can we proceed with migrating the CLA system?

@emsearcy
Copy link

Yes, the fix for email validation behavior on GitHub-based account registration is live.

@dankohn
Copy link
Contributor

dankohn commented Oct 4, 2016

@emsearcy Could you please provide an overview on the status. What are the open issues preventing a rollout to K8s or other projects? Are there nice-to-haves we want to add in the future?

@foxish
Copy link

foxish commented Nov 19, 2016

The rollout is now complete and the kubernetes and kubernetes-incubator organizations are using the CNCF CLA. Thanks @caniszczyk, @emsearcy and @dankohn for all the help.

@bgrant0607
Copy link

Thanks all!

@mbohlool
Copy link

Can we add this check to kubernetes-client org too?

idvoretskyi added a commit to idvoretskyi/foundation that referenced this issue Nov 21, 2018
12/3/15 - added Gold membership level
* Changes to sections 4 and 5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants