-
Notifications
You must be signed in to change notification settings - Fork 167
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
Run and deploy nodejs-github-bot #404
Comments
I think as soon as we have an accepted deployment strategy we should be fine. I can spawn one or two vms (does it/should it support being run from multiple locations?) once that's done. As for deployment, I'd be cool with auto-deploying on master if the repo started following a more strict merge strategy, similar to nodejs/node (signoffs, tests, commit messages, etc). |
Do you have similar processes running, or have anything special in mind? ATM it's pretty much
Looking at the minimal traffic it receives/generates atm, there's not a need for multiple instances. To keep it simple at first, I suggest we keep it as one instance.
Cool! In regards to tests we absolutely have to improve when things get settled. In regards to deployment, I'll create a smoke test just to ensure we don't try to merge and deploy code which doesn't even start. Not sure we should aim to be just as strict as in nodejs/node at this stage being 3-4 devs involved. Would n't a green Travis build and at least one LGTM from another dev be good enough? |
Not sure I really agree here. The website has a similar lax policy, autodeploys, and arguably can effect a wider ranger of people in worse ways. The bot only needs basic repo access afaik (stauses), so I am not too worried. I think the org inviting thing should probably be a separate bot due to security reasons. |
@nodejs/build should we take this to the next build WG meeting perhaps? |
Great suggestion @Fishrock123, I have tagged this with |
@Fishrock123 I'd like to counter-suggest reviewing for the nodejs.org repo too then. I don't feel it would slow down much to have one other person say "hey, this looks good -- I'm ok with you pushing this change that can affect a lot of people". If its about collaborators not being used to git I'm open to find a middle ground. |
I prefer redundancy is all. Especially if we are to collect raw logs from github that will be the foundation for audits. If we don't support running two slaves (races et al) lets at least have it in mind and try to put it on a roadmap.
(as follow up to my above comment) Regarding implementing code-reviews, I'm fine with having a lower bar for entry until collaborators become more acquainted; I guess similar in a way to how we'd have a quicker merge for documentation typos @ the node repo. What I'd like to at least raise concerns about is the "laissez fair" approach for pushing changes. Every time we have jenkins security issues we're on lockdown for unknown amounts of time and worst case need to redeploy. I don't want to add other components to a build infrastructure that has an unknown/lax state of trust. I know "Reviewed-by" doesn't mean much if someone really wants to compromise a repo (especially seeing how we'd autodeploy on push), but it's a start and makes someone else than $committer partially responsible for what can happen. Finishing off, It's not super clear to me what we would discuss at an agenda this point but I guess security/trust would be at least one topic. |
@jbergstroem we definitly use LGTMs in nodejs.org, but there's no strict requirement to have tests at all times or format commit messages. So far this has worked perfectly for that project, still it's far from the process in core. P.S. the release blog posts can be merged right away by the author to get them out asap, although they're still PRs which might get comments later on. |
Another option would be: check for tags and only build ones that are signed by one of specific gpg keys. In hindsight, I don't really see a problem using LGTMs on the bot. Not sure about adding reviewed-by metadata since we usually merge by the github button but those all link back to the PRs. |
I had a chance to chat with @mikeal & @Fishrock123 at NodeConf this weekend about this topic and got some positive responses so I would like to try to help move this forward. I just listened to the Build WG 2016-06-07 #434 meeting to get the update. It sounds like the participants wanted a bit more background. BackgroundThere were originally 2 bots being created in parallel (on accident): 1 by myself with a focus on creating tasks to manage the Github/YoutTube Communities & 1 by @phillipj which focused on doing build status and tagging. I moved my work to @phillipj 's project to combine efforts. I also opened an issue on the bot's repo to start a discussion about deployment, access, & permission since my scripts will need elevated permissions to the Github Org and YouTube/Google+. What elevated permissions?Github: This is only needed to add people to the org. Working demo video: nodejs/github-bot#29 What are the next steps needed?
Optional:
I have paused work on development while I figure out of this can/will move forward since my tasks are depend on this. Perhaps a high-level "actionable item" for the WG is just to vote on whether the WG feels it is within their domain (or is even interested) in taking on this responsibility. Looking at the WG's charter, I'm not 100% sure it is- so I hope I'm not conveying pressure. /cc @nodejs/tsc I recognize this is additional load and scope placed on the Build WG which you MAY not have time (or interest?) to commit to it. I'm happy to dedicate time & help out however possible. |
@jbergstroem could you elaborate on the need for elevated Jenkins access that you mentioned in WG meeting? In the testing we've done so far when updating PRs w/Jenkins build status, the bot hasn't had the need to even authenticate to Jenkins, as Jenkins pushes status updates to the bot via HTTP/curl. IIRC those status updates doesn't contain anything secret either. Not trying to say the bot isn't subject to security issues, as it already has elevated GitHub access to several of our repos. But we might not need to give it elevated Jenkins access. |
@phillipj sorry for the late reply
As-is, nope. Elevated access to jenkins is not required; anonymous would even work fine but if we want to query builds for release or perhaps "sync", as in figure out current state seeing how jenkins will likely bug and send us half of the curl requests we expect it too, we will have a problem. I guess it was unfair to speculate on future access, but I feel it might be unavoidable.
We could perhaps redeploy when elevated access is required? |
Sounds good. It would be great if we could identify showstoppers for deployment as-is, and tackle challenges as they come. |
@nodejs/build any thoughts/preferences on deploying this? E.g. ordinary scripts or docker? |
We use ansible for most stuff, so that's preferable. |
This adds an ansible setup for the github-bot. Primarly installs necessary binaries, checks out the application code from github.com, before starting the systemd service. Refs: nodejs#404 PR-URL: nodejs#469 Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Removing wg-agenda -- nothing to discuss. |
Made a separate issue for auto deployment nodejs/github-bot#69, closing this for now. Feel free to open more focused issues here or in the github-bot repo if there's something in this issue which still needs to be addressed. |
Starting this issue to get the ball rolling in regards to running and deploying the nodejs-github-bot like other parts of the Node.js infrastructure. The ideal solution would for the bot to be deployed automatically whenever something is merged to master.
We've tried to document the bot's requirements and startup procedure as is in the README.md.
What do you need for this to happen?
Refs nodejs/github-bot#23
The text was updated successfully, but these errors were encountered: