Skip to content
This repository has been archived by the owner on Jul 14, 2021. It is now read-only.

Add expeditor #1213

Merged
merged 5 commits into from
Apr 12, 2017
Merged

Add expeditor #1213

merged 5 commits into from
Apr 12, 2017

Conversation

tduffield
Copy link
Contributor

Description

Expeditor is the next-generation utility that we'll use for version bumping. It will include additional features that we don't currently have like the ability to not bump a version on a specific PR.

Check List

Copy link
Contributor

@schisamo schisamo left a comment

Choose a reason for hiding this comment

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

gif-keyboard-17417109445383206248

@tduffield tduffield force-pushed the tduffield/add-expeditor-config branch from 80c4c5f to 384cfff Compare March 31, 2017 21:28
@vinyar
Copy link

vinyar commented Mar 31, 2017

This is cool!

@tduffield
Copy link
Contributor Author

@vinyar I will post more about that later (its not a public repository). :) I just need to get this code in place to facilitate the migration.

@coderanger
Copy link
Contributor

@tduffield It not being public seems like a reason to not merge this yet since we (those not at Chef Software) have no idea if this is a good change or not other than taking your word for it.

@tduffield
Copy link
Contributor Author

tduffield commented Mar 31, 2017

@coderanger Expeditor is a replacement for the stuff that is currently going on behind the scenes including our version bumping, building triggering, etc. While the code that is executing the stuff is not public, this is actually more transparency than we had before.

@coderanger
Copy link
Contributor

Okay, so why does that need to in a private repo such that no one else can look at it?

@coderanger
Copy link
Contributor

If y'all want to use private tech for your own projects, that's cool, but it doesn't fly on stuff that touches the community :)

Copy link
Contributor

@coderanger coderanger left a comment

Choose a reason for hiding this comment

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

Until the code using this is public, I'm -1 on it.

@tduffield
Copy link
Contributor Author

@coderanger The reason it isn't public is because the code is specific to Chef. I'll need to walk through and make sure there isn't any information that we don't want exposed (i.e. internal URLs, etc).

@coderanger
Copy link
Contributor

Okay, that sounds like a good thing to do and then we can loop back on this 👍

@coderanger
Copy link
Contributor

To try and explain my resistance here a bit, I'm increasingly concerned about how much hidden stuff there is in the CI and build process for Chef as a project. I am eternally thankful that Chef Software as a company is willing to devote resources towards managing builds and testing for the FOSS side of the equation, but I think we should ensure that even if Chef Software was unable to continue that for any reason, the community could take over (within reason, I suspect AIX and Solaris builds would hit the cutting room floor pretty quickly for example). So previously we had this stuff basically one-off'd in each repo but in a way that I could go plug in to a new Jenkins tomorrow if I had to. By shifting this logic to an external, private gem we're in a much more precarious position as a project. I agree it's functionally equivalent and nothing here will reduce code quality or harm users in any way, but there is a deeper existential concern on my mind. Slippery slope is a shitty argument and you would be 100% right to call me out as assuming the worst in a situation that doesn't call for it, but still, I worry.

@coderanger
Copy link
Contributor

And as a secondary concern, our CI process is getting immensely complex. This would probably be more on the simplification side than anything but changes in gnarly code get my attention more than other places since any changes will be the basis for future progress. See also: the gemfile management system which kind of ... got away from us.

@tduffield
Copy link
Contributor Author

@coderanger so I had a chance to talk with some folks internally. While there is nothing "special" per-say about the Chef Expeditor code, it is heavily customized for Chef Software. As such, it's not something that we necessarily want to support as an open source project. Additionally, there is more than just version bumping occurring in Expeditor, and we've identified some surfaces that we'd rather not publish out to the world.

As a middle ground, we've given the core maintainers group read access to the repository. Any maintainer will be able to see the code and how it operates. Is that sufficient?

@coderanger
Copy link
Contributor

@tduffield That's a little less bad, but if I don't have the legal right to use that code (which would be the case if I had read access to it under a proprietary license) then we're still in the same position of the project being explicitly proprietary instead of "accidentally" proprietary. And I don't think that is okay, the precedent of us having not gotten around to fully documenting/opening some of our backend-y tasks is very different from purposefully making Chef's release process require code that is private to Chef Software.

@tduffield
Copy link
Contributor Author

@coderanger Nothing that Expeditor does is required to release Chef or ChefDK. Expeditor is a chat ops utility that Chef Software uses to automate otherwise manual tasks including version bumping. If Expeditor were to disappear or go offline, you would still be able to ship Chef or ChefDK, you would just have to perform tasks like version bumping manually.

@tduffield
Copy link
Contributor Author

@coderanger as to your point about fully documenting our backend-y tasks, that is what https://github.com/chef/chef-rfc/blob/master/rfc086-chef-oss-project-policies.md was also intended to cover. In that document we outline a great deal about how Chef and ChefDK are built and released.

@coderanger
Copy link
Contributor

Okay, so let's zoom out a bit. Chef Software Inc and Chef the open-source project exist in kind of a delicate dance. Chef Software owns the trademark on "Chef" (and a few related things), owns copyright on some but not all of the Chef code, and sponsors development time on Chef using some of its staff. In the abstract, the two should be at least somewhat separable, the project should be able to exist without the company and vice versa. In reality things are more intertwined than that, but as a major project contributor that operates outside the company, I try to ensure this kind of project continuity plan is at least mildly protected since there may come a time when Chef Software doesn't have the resources to support the project to the degree it does today (I'll leave out the "acquired by Oracle who then paints a bullseye on the FOSS community" scenario though that's one I worry about too, just less so). As a matter of how things have evolved over the years, Chef Software has also provided some services to the project that are more capital or maintenance intensive, like producing and hosting installers (which, again, super grateful for and hope will continue). At least initially this build farm being private was simply a matter of Chef Software owning the physical devices and paying for the time of all the people involved in release engineering, but this wasn't part of the day-to-day development workflow (with Travis running our day-to-day CI stuffs) and anyone could build the same installers on their own hardware if they needed to run a release (indeed, many of the early installers were hand-built on someone's laptop) so even if Chef Software was unable to build or host the installers for us, the work to replicate the 99% use case (read: Debuntu and EL) was some omnibus build commands and an S3 bucket. Doable in an afternoon. As the build system has gotten more complex over time, the ratio of "private hardware" to "private code" has been shifting, slowly for sure, but still shifting. The reason this sticks out at me is this is the first time (that I've seen) that we've purposefully made this process more proprietary rather than less, the other times this has happened has been a "by default" situation because of the physical hardware being owned by Chef Software. Aside from the baseline concern that this is not how an open-source-first company should run things IMO (though of course that's just an opinion and y'all can run your company however you want), this presents a new risk to the project's continuity plan (what little plan there is). If this project is complex enough that you don't feel comfortable releasing it, that probably means it isn't trivial to replace. As such, I don't think it is appropriate to introduce it to the workflow of the open source project side of the dance.

If we wanted to take this as an opportunity to make things more safe rather than less, how about you move all the secret-y strings in to environment variables and then open the repo under a FOSS license with a clear note that no user support will be provided and it is only open for project continuity purposes. Alternatively we could do something like the old Chef Server split, make a minimalist FOSS version of whatever scripting system Expeditor uses (looks like some form of named hook scripts from the contents of the PR) and make that available. This might be a 10 line shell script, or it might not, I don't actually know what Expeditor does so I am assuming it does a lot more than that (again, because if it was a simple project I am assuming you wouldn't have a desire to keep it proprietary).

Hope that explains my viewpoint in a little more depth. And again, this is not intended to be a critique of Chef Software's involvement in Chef as a project, just trying to make sure the project stays in a healthy state.

@thommay
Copy link
Contributor

thommay commented Apr 6, 2017

I pretty much entirely disagree with your summary of the situation around CI, actually.

There's almost no proprietary code in the stack; there's a Jenkins instance that sits on a bunch of expensive hardware that we pay to host, and it runs some very trivial shell scripts (virtually all of which is released either in chef/chef, here, or in omnibus). We then pay for an Artifactory license to host the artefacts (again. because it's convenient), and give Fastly some large amount of money to CDN those artefacts. The end.

I don't see what can really be changed here besides some waffly crap about donating hardware and money to a foundation that would be de-facto controlled by Chef because all of the time, money and resources would come from Chef, and honestly that's a waste of everyone's time.

The reason Expeditor is not open source is because it's utterly uninteresting and provides no-one with any benefit if we do so, and because we don't want to commit the resources to supporting it for other people, which we must do for it to be open source. We can drop it and run, but that's not how Chef operates, and I don't think you mean to encourage that behaviour.

If Chef ceases to be, the velocity of changes and releases drops to a point where the version bumping and notification of CI, which is 100% of what this bot does, becomes sufficiently negligible that a human can do it.

@coderanger
Copy link
Contributor

@thommay The current minimal state of "behind the scenes magic" is exactly why I called this out as concerning to me. If Expeditor is minimal enough to not really move the needle then so be it, but I'm not a big fan of that as an overall plan. Maybe document what it does to enough of a degree that it could be replaced easily? That seems like a good thing anyway if this is going to be part of our of project. That would make it a lot easier to judge the full impact of this change on the project.

@thommay
Copy link
Contributor

thommay commented Apr 6, 2017

https://github.com/chef/chef-rfc/blob/master/rfc086-chef-oss-project-policies.md#auto-bumping-patch-versions

We wrote an RFC documenting exactly that, and Tom linked to it above.

@coderanger
Copy link
Contributor

That doesn't explain anything about what that config file is or why it appears there is a second bundle install going on. I have no way to judge if anything in this PR is a bug or not. I can certainly make guesses, something like that shell script snippet is run from Jenkins during each build as well as some kind of Slack notification but it's a little weird to me to have code in this project's build flow that I can't really change because I don't know where it gets called. Even just explaining what Expeditor does in here (more specifically than "it bumps versions") would be something for the community to go on.

As a concrete example, if I make a change to the platforms in the gemfile should I update the install script?

Expeditor is the next-generation utility that we'll use for version
bumping. It will include additional features that we don't currently
have like the ability to not bump a version on a specific PR.

Signed-off-by: Tom Duffield <tom@chef.io>
@tduffield tduffield force-pushed the tduffield/add-expeditor-config branch from 384cfff to 4cc31a3 Compare April 6, 2017 19:10
Signed-off-by: Tom Duffield <tom@chef.io>
@tduffield tduffield force-pushed the tduffield/add-expeditor-config branch from 4cc31a3 to 2fd5b54 Compare April 6, 2017 19:12
@tduffield
Copy link
Contributor Author

@coderanger does 2fd5b54 supply the necessary context?

@coderanger
Copy link
Contributor

👍 That helps. And would I be correct in saying that if you ignore all the notification features, the effective change to an imaginary Jenkins config would be s/rake ci_version_bump/bash .expeditor/update_version.sh/? Like that seems fine and is effectively no different than we have today? I can see both sides of this (I think) that I'm pointing at a relatively minor change and yelling "slippery slope", but at the same time "take our word for it, this is fine" coming from the company is hard to see as comforting even when I have great personal trust for the individual people actually saying it. But again, I am probably the only person worried about this and in the end it's not actually a big change so 💯

Signed-off-by: Tom Duffield <tom@chef.io>
@tduffield
Copy link
Contributor Author

@coderanger yes, what Expeditor is doing is effectively no different than what we're doing today. I also moved stuff entirely into the single file so that things are extra clear about what is part of Chef and what Expeditor is doing.

@tduffield
Copy link
Contributor Author

@coderanger I also explicitly declared defaults and added some more context to the config.yml file for you.

Signed-off-by: Tom Duffield <tom@chef.io>
@tduffield tduffield force-pushed the tduffield/add-expeditor-config branch from b7b5b14 to b56f7a9 Compare April 6, 2017 19:50
@coderanger
Copy link
Contributor

👍 Just, again, for clarity: the actual version bump itself is a feature of expeditor so this won't use the bump tasks in Rake anymore?

@tduffield
Copy link
Contributor Author

@coderanger correct. After we formally move to expeditor, we'd remove that code.

@coderanger
Copy link
Contributor

Good enough for me :)

Please note: the VERSION file is not technically in use yet. I just
needed to add it to ensure everything was in place.

Signed-off-by: Tom Duffield <tom@chef.io>
Copy link
Contributor

@thommay thommay left a comment

Choose a reason for hiding this comment

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

gif-keyboard-10800535248897067643

@tduffield tduffield mentioned this pull request Apr 7, 2017
4 tasks
@tduffield tduffield merged commit 60df368 into master Apr 12, 2017
@tduffield tduffield deleted the tduffield/add-expeditor-config branch April 12, 2017 20:37
@chef-boneyard chef-boneyard locked and limited conversation to collaborators Feb 14, 2018
@tas50 tas50 added Expeditor: Skip Changelog Used to skip built_in:update_changelog. and removed Expeditor: Exclude From Changelog labels Jan 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Expeditor: Skip Changelog Used to skip built_in:update_changelog.
Development

Successfully merging this pull request may close these issues.

6 participants