we had a brief discussion on IRC regarding the community templates that we wanted to bring to discourse. Please allow me to summarize what we discussed mixed with my thoughts.
In core we have snapshot tests for templates. They render all templates with a predefined fixture set and compare them with a saved version of the rendered templates. This allows us to refactor the templates or the renderer and make sure this does not have any unwanted side effects as there would be a difference between the saved version and the new version. So this technique is a huge help when working on the rendering engine in Foreman.
One release todo is importing the templates from community templates to Foreman. This caused the snapshot generation (and the snapshot tests) to fail because the snapshot rendering engine does not handle nested snippets well (#26612). We could workaround this quite fast and will solve the root cause when there is time, but ideally, we want to notice these issues a lot earlier.
We do have tests in community-templates, but they use an entirely different renderer to render the templates. Ideally, these tests would use the same renderer as Foreman. I was thinking of adding a rake task to Foreman that given a directory renders the templates with some predefined fixtures.
There were other ideas, though: Maybe release the templates as a seperate gem and consume that in foreman, or maybe have it as a submodule of the main repo. Or move the templates to the Foreman core repo.
Commenting only on the submodule idea:
Using the community-templates repository as a submodule would solve the problem of having two sources of information. At the same time it would keep the foreman repository in a defined relation to its templates (preventing silently breaking test by random-change-somwhere-else). Updating the templates would then translate to updating the submodule. And i think, this can still be combined with running the foreman-rake test task in the community-template repository.
Setting aside that submodules are not an easy-to-use technology per se, i think that this could combine most of the benefits.
I actually like having templates as a separate repository. This allows people to quickly contribute even small changes because we do not have tight rules as in core repository (RedMine issue, Rubocop and others). The last thing I want to see is people dropping ideas of changing few lines here and there because a bot starts screaming on them. Also Foreman repo is big compared to light community-templates.
As much as I hate git submodules, I think it might be an answer here. Either submodule or a rake task that would checkout/download templates from the original location. For me, it is either this or keeping status quo - new renderer test suite was a great addition and I understand that it brings some struggle during stressful days of branching new release, however from the users perspective it is much better to see tests failing now than when users hit those issues. So it was a change for good.
Maybe this is just a process change, I think @tbrisker already mentioned he plans to sync templates more often which is indeed a good idea. I would not mind a jenkins job that could actually make an attempt to do dry run of integrating community templates into core on a weekly basis. If something breaks, we get an early notification here. I don’t know much about Jenkins configuration but AFAIR pulling templates is just a matter of few rake tasks and can be easily automated.
There were other ideas like extracting renderer into gem, I think that would be probably an overkill. I think we should be able to come to a reasonable conclusion and either way I think we are doing great. Good stuff everyone! @TimoGoebel
I just want to add, so we do not forget, that the situation with job_templates should also be addressed.
If i see it correctly, they are distributed across foreman_rex, katello and i-do-not-know…
I hope we can use the same mechanism there.
I have suggested in the past to create a gem, but what I meant is transform community-templates itself into a gem. That gem would have all the templates we have now and some boiler plate so you can easily determine the directory containing the files.
In Foreman we can then depend on this community templates gem. In develop we’d have a git URL so you always have the latest after a bundle update and in a release we’d depend on a released version.
@TimoGoebel’s approach to create a test task is great. We have users who have their own git repositories with organization specific templates and that would allow them to easily run tests in their CI. We’d be dogfooding that setup in community-templates which also provides them with an example.
I like what @ekohl says. That means no need to copy to Foreman codebase anymore. We need to update seed script to load it from gem instead of local directory. Given the number of s, I think this is a popular option. Is this something we aim for 1.23? Who will send actual PRs?
We’ll probably need some boilerplate code in the community templates PR. Can we also create a gem for that code so users can easily consume that in their custom template repo? So in the end we’ll have two gems:
community_templates - our content foreman_templates_boilerplate - the boilerplate
Core will then require community_templates, community_templates will require foreman_templates_boilerplate.
Now thinking about this more I see a possible problem with this. If we keep template snapshot test suite in core this would create unnecessary burden of breaking these tests everytime we do a change in community-templates (therefore jenkins will run against master branch gem repo).
I think that snapshot tests must be in the same repo as the source templates so we can review the changes (and snapshots) in advance prior merging a change in community-templates repo.
Which brings me to the idea which is exactly other way around - how about moving template snapshots to community repo and then run the snapshots test via foreman checkout similarly like we run plugins.
Edit: Or we can keep the idea but simply move the snapshot files to the community-templates, the test code would still be kept in Foreman core.
It feels like we should revive this discussion.
With the branching of 1.24 we again hit several issues due to changes in the community templates repo that could have been caught much sooner had we gone with one of the suggested solutions.
While all of the options could improve this, I think one option stands out as the simplest to implement, and I think this simplicity make it the options we should choose:
Lets archive the community templates repo and move to modifing the templates in the main foreman repo directly.
Simple to implement - can be done in a few minutes. No need to work on integrating gems, modifying tests etc.
Changes in core that affect template rendering will also get tested with the latest templates, and allow adjusting the templates if needed in the same PR.
Changes to snapshots can be done in the same PR as the changes to the template.
We can easily track which changes are in each Foreman release (and trace where a bug landed if necessary for backports)
No more need to sync templates during branching and release process.
PRs will require redmine issues. The concern has been raised that this may deter drive-by contributors, but I don’t think it will be a significant factor. It may also have a benefit of having more contributors take part in the foreman core after having a template PR merged. I looked at the logs for the 1.24 templates, and of 36 commits only 4 came from people who aren’t regular foreman contributors and didn’t include an issue number. In 1.23 there was only one such commit.
I think the pros greatly outweigh the con here - and I don’t think requiring a redmine issue is such a high bar that it will actually have a significant effect on contribution.
Yes, let’s archive community templates and start doing template changes in core
No, let’s keep community templates active and figure out a better way to test and integrate it
Another con is that we’d increase the load on our CI.
I’d still like to see community-templates as a proper gem that we can include in Foreman with proper CI, including a nightly/weekly build against Foreman as we do for all plugins. We have a helper rake task (that may need some love) which can help with this. We can look at creating a nightly gem out of this (we do it for Katello already).
Getting things merged in Core Foreman is hard enough. Let’s not add more.
You mean due to ~2-3 additional PRs per week? I don’t think that’s a significant difference.
This also brings to mind another pro i didn’t think of earlier - more visibility into PRs. More people look at core than community templates, and the people who do look at community templates today will have one less repo to look at.
Could you elaborate on what are the pros and cons of this option?
In this case the templates are already merged in core - and we manually sync them all the time. There is no effort needed to get the templates in core as they are already there, and we would save the effort put into syncing them and fixing snapshots every time.
Other than the templates themselves the only code in the repo is a few tests, which I believe have much lower coverage than the tests we have in core (as is evident by the bugs discovered when syncing the templates). We could also migrate these if we want to with very little effort.
I like moving the templates to core. We’ve been talking about packaging templates separately for a long time and I’m afraid, we’d just keep talking. In fact I see a benefit of more reviewers, better visibility, better connection of the change e.g. when we also need to enable something in safemode. Changes propagate to nightly also much faster. Seems like a quick win not preventing us packaging templates separately if we ever get to it.
It’s not uncommon that people open PR with templates changes against core and we are telling them to send the PR to community-templates repo. Meaning, people expect it in the main repo.
IIRC the main reason for keeping this outside of the main repo was the versioning. We kept compatibility with Foreman stable and 2 previous versions and we didn’t version templates at all. That is no longer the case for some time.