Skip to content
This repository was archived by the owner on Jul 10, 2018. It is now read-only.

0025: Compute Resources as plugins#28

Open
GregSutcliffe wants to merge 2 commits intotheforeman:masterfrom
GregSutcliffe:0025-crs-as-plugins
Open

0025: Compute Resources as plugins#28
GregSutcliffe wants to merge 2 commits intotheforeman:masterfrom
GregSutcliffe:0025-crs-as-plugins

Conversation

@GregSutcliffe
Copy link
Member

Initial draft. This discusses the implementation plan for moving compute resources to plugins. Feedback wanted!

compute resource, we should:

* Identify at least one maintainer for the new plugin
* Create a new repo `theforeman/foreman-<resource>`
Copy link
Member

Choose a reason for hiding this comment

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

I vote for foreman-compute-<resource> to have a tag in there for easier listing and overall better visibility.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree in principle, but it could be awkward to rename existing packages and migrate cleanly. We've done it before though. @domcleal @mmoll is this sensible?

compute resource, we should:

* Identify at least one maintainer for the new plugin
* Create a new repo `theforeman/foreman-<resource>`
Copy link
Member

Choose a reason for hiding this comment

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

Bit of a nitpick, but underscores vs dashes has caused issues in the past given Ruby convention tend towards using underscores for gem names.

Copy link
Member

Choose a reason for hiding this comment

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

We should also setup Jenkins jobs etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

+1 for both, I'll update for that.

* Identify at least one maintainer for the new plugin
* Create a new repo `theforeman/foreman-<resource>`
* Move the appropriate code from Foreman Core to the plugin
* Release the plugin
Copy link
Member

Choose a reason for hiding this comment

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

This being gem/RPM/deb release?

Copy link
Member Author

Choose a reason for hiding this comment

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

Open question really - at what point is it ready to delete the core code? My feeling is at least one packaged (rpm/deb) release, but perhaps it could be done sooner.

One option is to version the namespace in a pluign (e.g. Foreman::Libvirt::V2).
This would allow the new plugin to co-exist with the core provider until
feature parity is reached. This does leave the question of whether to rename
the namespace once the core provider is deleted, however.

Choose a reason for hiding this comment

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

Any ideas of how to more easily (or automatically) handle upgrading users?

Copy link
Member

Choose a reason for hiding this comment

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

I think, we can just use a simple db migration for this that changes the type on the ComputeResource model.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jlsherrill that's why I leave it as an open question. Using a separate namespace does cause issues when the core provider is dropped.

I'm not sure where that db migration would go @timogoebel. It doesn't make sense to have a migration as part of the core PR that deletes the core provider, because that migration would run for everyone regardless of whether they use it. It could be in the plugin, but then the user is broken until they install the first version of the new plugin. Tricky...

Copy link
Member

Choose a reason for hiding this comment

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

It could be in the plugin, but then the user is broken until they install the first version of the new plugin. Tricky...

I don't think, this is a real concern. We tell the user, that they have to take action (it's just installing the appropriate os packages) when we drop support in Core and then the db migration in the plugin will make the cr work.
I do agree, that we need to think of a way to handle the case that a user upgrades to a Foreman version without core support for an existing compute resource. But maybe a message is enough that tells the user to install package tfm-rubygem-forman-compute-ec2 to make everything work again.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that could work. This is all assuming we go with namespacing the plugin, of course. If the plugin uses the same names as the core code, then no migration is needed.

Copy link
Member

Choose a reason for hiding this comment

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

Core should be updated to detect a plugin is no longer working, we should have a way of disabling already installed plugins, so they don't shutdown the entire system.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ohadlevy I don't disagree, but that feels like a separate discussion, since it applies to all plugins.

Copy link
Member

Choose a reason for hiding this comment

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

I will find it much easier to discuss these points if we clarify to all plugins vs just to compute.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ohadlevy Are you suggesting we widen this scope of this RFC for this purpose, or pause this while we discuss the point elsewhere? I don't want to derail this completely :)

Copy link
Member

Choose a reason for hiding this comment

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

@GregSutcliffe TBH: I dont know, I dont have a simple answer in terms of do i want to do it to all compute resources.

My suggestion, is pick all the CR that are either un-maintained, or there is a maintainer who wants to extract it to plugins and do that first, and based on that decide for the remaining ones.

# Detailed design
[design]: #detailed-design

It may not be feasible to move *all* resources to plugins right away. For each
Copy link

Choose a reason for hiding this comment

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

At least it should be tried to make ths move in one release cycle.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd love to - However, I think we may struggle to form teams around every provider at the same time. What issues do you see coming up if that doesn't happen?

Copy link

Choose a reason for hiding this comment

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

IMHO the extraction should be made by core devs very quickly for each provider and once that's done, the plugins are on their own. I even do expect that e.g. the GCE plugin won't have a maintainer for the first time being.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mmm, that's not a bad idea. I had planned to look for interested parties first - but if there's support for a bulk-dump-to-plugins, then that's probably even better. We'll need to find resources for it - any volunteers? :P

Copy link
Member

Choose a reason for hiding this comment

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

I'll do vmware. :-)

Copy link
Member

Choose a reason for hiding this comment

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

+1 for @timogoebel && vmware

we already have several examples of this in use (Xen, Azure, DigitalOcean,
etc). This has brought benefits:

* able to release independatly of Core
Copy link
Member

Choose a reason for hiding this comment

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

while its true, many of the dependencies go back to fog, so unless fog is broken down fully to different providers, one plugin requirements might conflict with another plugin, making the support matrix harder to follow

Copy link
Member Author

Choose a reason for hiding this comment

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

Fog is already breaking down into multiple providers anyway (fog-openstack, fog-google, fog-azure, etc) and I'm sure they'd welcome PRs to continue that work.

So, my assumption is that we can ship a version of Fog-core in foreman-compute (which should prevent incompatibility issues), and plugins can depend on the appropriate Fog subpackage. I agree that it's not perfect, but it feels like it grants CRs more flexibility than we currently have.

* able to release independatly of Core
* make use of the modular Fog gems that are now available
* separate logging of issues
* dedicated maintainers who know the compute resource well
Copy link
Member

@ohadlevy ohadlevy Oct 27, 2016

Choose a reason for hiding this comment

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

is this a valid assumption? do we have enough people who know compute resources enough? one major risk when moving code out of core to plugins, is code quality, and hacks done inside a plugin vs a proper solution in core.

Copy link
Member

Choose a reason for hiding this comment

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

That's true. But at least for VMWare I think a proper maintainer is missing in core. I can't tell about other compute resources. If the plugin API in core is missing features, we can add them so that no hacks are necessary. I'd rather have a plugin in core that receives attention than a compute resource in core that is stable but doesn't support all required features. Extracting a compute resource to a plugin can actually also lead to besser code quality when the chance is used to add tests or refactor things.

Copy link
Member Author

Choose a reason for hiding this comment

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

If no-one is maintaining the code, then it'll be of poor quality regardless of whether it's in core or in a plugin. We know that's true today for some of our CRs.

We've seen historically that people are more willing to get involved if the codebase is not big & scary (regardless of whether the actual code is any less complex). On that basis we should get more contributors to a CR plugin than to the same code in core. It's all perception :)

Copy link
Member

Choose a reason for hiding this comment

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

i think that one clear outcome is that non-maintained code must be removed to a plugin (e.g. google).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but how do we define it? GCE is easy, what about AWS (which I would move)?

* dedicated maintainers who know the compute resource well

In addition, it allows the core project to mark a particular resource as
unmaintained when necessary (as happened with Xen).
Copy link
Member

Choose a reason for hiding this comment

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

but that's always an option? that to me sounds like a reason to move a code into a separate gem in the first place.

Copy link
Member Author

Choose a reason for hiding this comment

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

To your first point - yes, but where would you make that mark? For plugins, we have http://projects.theforeman.org/projects/foreman/wiki/List_of_Plugins where you can clearly see which are unmaintained (and even easier if #30 goes ahead).

To your second point - I think you have a typo. I parse that as you agreeing with me, this is a reason to move the code to a plugin (gem) :)

Copy link
Member

Choose a reason for hiding this comment

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

I think we agree, but on different parts, as mentioned earlier, non maintained code path should be removed from core (regardless if its compute or not).

regarding maintained code (e.g. vmware/ec2 etc) I think we should move it to a plugin only if there is a good reason, I personally don't think that a VMWare maintainer will do better job if its in the repo or in another repo.
This does open the discussion of breaking down maintainer responsibilities, e.g. someone is maintains the vmware code, regardless of where it is, and can make the decision to move it to a plugin if he wishes to.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I agree with you - but since I find the benefits of moving a plugin a strong argument in-and-of-itself, I would move it to a plugin anyway (and this RFC is currently written such that not all CRs will move at once). Your comment puts you in disagreement with @mmoll though, who suggests all CRs should move in one release cycle (https://github.com/theforeman/rfcs/pull/28/files#r85011715). Discuss! :)

Copy link

Choose a reason for hiding this comment

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

Not extracting all plugins is kind of against the title of this RFC, but I also don't have strong arguments against it. Personally I do have the feeling that this further strengthens the situation of having a set of "first class" compute resources in core vs. "second class / potentially unmaintaind" ones in plugins.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mmoll when you say it "strengthens the situation", do you feel have 1st/2nd class CRs is a good or bad thing? I feel we're always going to have some that are more tested than others, regardless of where the code lives...

Copy link

Choose a reason for hiding this comment

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

As said somewhere above, I even expect that e.g. GCE is unmaintained after extraction, so the situation will occour anyway. It's more about having the same start situation for all.

to plugins, we gain the ability to properly support the maintainers or mark
them as curerntly in need of a maintainer. It's also easier for new
contributors to work on a specific resource they know, without having to learn
the whole core codebase.
Copy link
Member

Choose a reason for hiding this comment

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

again, I'm not sure how easy it is to write a compute resource plugin without knowing the core part of compute resources?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but you have much less to work with (and potentially break - see #28 (comment) about big vs small codebases). It's not about actual complexity.

Also, if the CR Plugin API is improved (as per #28 (comment)) and well documented, then it should be much much easier to work with.

* Release the plugin
* Send a PR to Core to delete the unused code (and maybe Fog dependencies)

At this point, the plugin can now iterate on it's own release cycle.
Copy link
Member

Choose a reason for hiding this comment

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

I think this brings the discussion around plugin testing and versioning within release branches, e.g. how does core knows it breaks a plugin, or how would a plugin maintainer release a different version of its plugin to different foreman versions.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, but that's a subject for another discussion, since it affects all plugins.

Copy link
Member

Choose a reason for hiding this comment

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

so it probably makes sense to discuss that first.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm happy to have a discussion about it, but it's a parallel discussion, not a serial one, I think. I think we all agree plugins need a better test structure, and so long as it improves somehow then this discussion is valid. The method by which it improves isn't relevant to this RFC.

[alternatives]: #alternatives

The alternative is to leave the compute resources in core, which for the
motivation above is undesirable.
Copy link
Member

Choose a reason for hiding this comment

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

one idea, is to move all compute resource code to external service (think dynflow as an example) which does all compute related tasks, this could be a clean separation from one large foreman monolith, and maybe easier in terms of defining a very clear plugin api.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure I'm in favour of that, but I'm struggling to define why. I'll think on it, but we'll see what others say :)

Copy link
Member

Choose a reason for hiding this comment

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

to me, one of the risk factor is the amount of code that ruby loads, it can make server faster and lighter on resources, and can scale differently, it can also be enabled to run on a proxy instead of core.

in theory, this can also be used in other uses cases (think about other projects similar to ours - manageiq or others).

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm concerned about the timescales this brings. A new dedicated service is a big task, and increases the barrier to contributing to CRs in the short term (while it is written). That conflicts with the goal of this RFC (increasing contribution to old, unhandled parts of our codebase). I think my vote goes to plugins in the short term, and perhaps such a service in the long term - after all, the CRs would already be plugins then, and could be adapted to the new service.

Another worry is that we're adding another abstraction on top of an abstraction (Fog). I think your idea has some merit, but I'd want to discuss it along with options like dropping Fog, and so forth. The code path to debugging issues with your CR is already torturous enough :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants