Dropping SQLite support in Foreman core in favor of NullDB

Development
1

As part of the upgrade to Rails 6, we found out that the latest version of SQLite shipped in EL7 is not compatible with Rails 6. This means that if we want to continue supporting it, we would need to ship our own packages for SQLite in the Foreman SCL.

SQLite is only needed for build tasks (specifically, apipie cache and asset compilation) that require a functional Rails environment. Considering these tasks don’t actually need a working database, just the ability for the application to start, we started looking at possible alternatives.
The leading candidate is the activerecord-nulldb-adapter gem, which basically converts all DB actions into no-ops. This has proven sufficient for the build tasks, with the nice benefit of speeding them up a bit as migrations are not needed.
@ehelms has opened several PRs introducing NullDB in Foreman to make sure this approach is working:
https://github.com/theforeman/foreman/pull/7594
https://github.com/theforeman/foreman-infra/pull/1338
https://github.com/theforeman/foreman-packaging/pull/5058
https://github.com/theforeman/foreman-packaging/pull/5059

Once these are merged and we verify we are able to generate working builds with them, I would like to fully drop SQLite support from Foreman (which is currently only supported for build and development).

This will give us two advantages:

  • Drop SQLite testing from all the test matrices, freeing up jenkins worker capacity for other tasks (e.g. adding ruby 2.7 tests).
  • Drop any workarounds in our code needed for making sqlite functional, and allow us to fully take advantage of advanced PostgreSQL capabilities.

SQLite will still be used by the smart proxy for dynflow persistence, as there we don’t have the requirement of upgrading it to support Rails 6.

What does this mean for you?

  • If you are developing with sqlite, you will need to set up PG and migrate your development environment to it. This should be a fairly simple one-time effort, that will give you the added benefit of running closer to production environment in your development setup.
  • If you have a plugin with CI testing running outside Jenkins (e.g. on travis) that rely on sqlite, you will need to either convert them to use PG or migrate them to Jenkins (which already provides templates for running plugin PR and build tests).

Please let me know if you have any concerns with this proposed approach - the goal is to finish implementing it in time for 2.1 so that we can upgrade to Rails 6 in this release.

7 Likes
2

Nice. I will add to that migration to PostgreSQL can be done with a rake task:

https://theforeman.org/manuals/1.23/index.html#3.7MigratingtoPostgreSQL

For the development environment instructions are similar, setup database.yaml and call foreman-rake db:convert:prod2dev. Example YAML configuration:

# The new target database
development:
  adapter: postgresql
  host: postgres.example.com
  port: 5432
  database: foreman
  username: foreman
  password: "foreman"
  pool: 25

# The old source database
production:
  adapter: sqlite
  database: db/development.sqlite3
  pool: 5
  timeout: 5000
3

Note, that we have a CI definition that uses Github Actions and postgres. I know, that I’m a bad community member by saying this, but I can highly recommend it over the Jenkins tests.

4

Not a bad community member if you explain why you recommend it over the Jenkins tests :slight_smile: This could help other plugins make choices and help us with our infrastructure design and what is provided.

5

Testing has found there is one place where apipie does need a migrated database, which is for displaying field types for search fields. However, this isn’t used anywhere for validation and is only informational - and in any case the search parameters are sent in the url params as a string, so it does not really matter if the field is defined as integer, string or text.
This was introduced in Feature #17964: Extend apidoc with list of fields to use in a search - Foreman which only requested a list of searchable fields. I suggest removing the field type from the apipie docs, which would hopefully allow us to generate identical docs with nulldb. Alternatively, we could allow users to manually regenerate it with types if they want but only ship docs with empty types where we can’t determine the field type from the scoped search definition.

1 Like
6

Yep, it’s a little of topic so I might open a RFC for this. However, the main points are:

  • It can run tests against all Foreman versions that a plugin supports
  • it can tun tests with several plugin combinations if that’s desired
  • it respects the rubocop version defined in the plugin and does not enforce the version used in Foreman core
  • the test config is part of the plugin repo so all changes, e.g. dropping support for older versions or bumping the rubocop version can all happen as part of one PR that is tested
  • PR authors can checkout a feature branch of a dependency and have the tests run against this feature branch while they wait for a release

Oh, and I don’t like that it’s yet another approach that needs to be maintained etc.

7

This feels like a breaking change for some use cases. Why do we find so many things to break right after 2.0?

8

I see how this could be perceived as a breaking change since we published these in our docs. But given the server never validated these they were simply “incorrect” if a client was creating some sort of validation layer. That would further imply any change to a parameters type would be a breaking change as well but we are not versioning our API based upon this.

I think it’s reasonable to either drop this from the docs all together or to report them all as strings.

1 Like
9

Please do, even if all you do is copy this particular post to a thread to continue discussion. I would love to better understand if its benefits of Github Actions due to their functionality or just gaps in our Jenkins CI; and discuss further.

10

I believe this falls in the category of breaking changes that are reasonable to do in a regular release with a note in the release notes. The alternative is to continue relying on sqlite for the build process, which means we can’t upgrade to Rails 6 until we package our own newer version of sqlite.

11

I opened a pull request defaulting to string:
https://github.com/theforeman/foreman/pull/7598
This will allow any user who does use these field types to regenerate the apipie cache locally with the correct types.

12

Looks like there is agreement that empty fields are better than string for everything. opened a PR to add an upgrade warning to the manual:
https://github.com/theforeman/theforeman.org/pull/1570