voxpupuli / puppet-rundeck

Module for managing the installatation and configuration of the rundeck orchestration tool
https://forge.puppet.com/puppet/rundeck
MIT License
39 stars 128 forks source link

Module quality update #520

Closed Joris29 closed 11 months ago

Joris29 commented 1 year ago

Pull Request (PR) description

This PR will make several changes to follow the best practices of puppet more and remove deprecated parameters.

Like removing params.pp Removing projects_storage_type parameter ...

This will be a breaking change for some use cases.

This Pull Request (PR) fixes the following issues

Fixes #197 Fixes #373 Makes #398 obsolete Fixes #406 Fixes #426 Fixes #445 Makes #447 obsolete Fixes #451 Fixes #452 Fixes #457 Fixes #472 Fixes #479 Fixes #484 Makes #487 obsolete Makes #521 obsolete

Joris29 commented 11 months ago

@kenyon Hello it's been a while, in my attempt to remove the params.pp i also took the opportunity to rework the module entirely.

The unit tests still need to updated and maybe some other things not related to the code. The code has been tested on a RHEL 8 machine.

My main questions is how to progress towards a new release is this something i need to do or how would this be done?

Joris29 commented 11 months ago

Good to know nothing has to be in spec/fixtures

I will try to move some things from common but what about big hashes like the admin policy and the os family specific things?

kenyon commented 11 months ago

OS-specific things should still go in the OS data file in hiera. I think big hashes in init.pp are fine, this allows them to be documented in REFERENCE.md.

Joris29 commented 11 months ago

I moved the parameter values to init.pp but i'm having some difficulties with framework_config.

This should be merged with the defaults from rundeck because if you overwrite the framework_config paramter then you lose all defaults if you don't set them explicitly like rdeck.base and i would like to avoid this so what's the best approach for this?

kenyon commented 11 months ago

I moved the parameter values to init.pp but i'm having some difficulties with framework_config.

This should be merged with the defaults from rundeck because if you overwrite the framework_config paramter then you lose all defaults if you don't set them explicitly like rdeck.base and i would like to avoid this so what's the best approach for this?

One thing you can do is make $framework_config only override the defaults. Set the defaults in the code, then merge (+) the hashes.

Joris29 commented 11 months ago

@kenyon I updated the specs and references even further for me everything checks out one check fails but it has nothing to do with the actual unit tests. The only thing still on my todo is to update the readme as this includes wrong example for now. Feel free to add more todo's if needed i also updated which issues/PR's will be fixed or become obsolete i might have forgot some but this can resolved later i guess?

Joris29 commented 11 months ago

@kenyon Update the PR for ubuntu and the unit tests all check out.

Joris29 commented 11 months ago

Yeah maybe it's best to squash the commits as there are quite a lot and some commits are not really clear on what they do exactly.

For the migration part i think this will kind off be part of the change logs because all the breaking changes will be in there. There is also not much to it except renaming/removing parameters or restructuring some data like the auth_config or key_storage_config but examples for both are added to the readme.

Joris29 commented 11 months ago

Should i do the squash now or will you do it when merging?

kenyon commented 11 months ago

Release PR #523 includes this change.

Fabian1976 commented 1 month ago

I'm sorry to say that this pull request actually reduced the quality of the module.

This parameter:

dataSource.dbCreate = update

Found on this line, is now hard-coded in the module and prevents new installation of rundeck and a postgresql backend.

In the version before this change (v8.0.1), it was a proper parameter:

dataSource {
  dbCreate = "<%= @database_config['dbCreate'] %>"

Because of that fixed parameter, new installs of rundeck with Postgresql generate this error:

[2022-09-29T21:22:05,759] ERROR boot.SpringApplication - Application run failed
liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for change set core/ExecReportJcExecIdToExecutionId.groovy::1653344096108-52::gschueler (generated):
     Reason: liquibase.exception.DatabaseException: ERROR: column "jc_exec_id" does not exist
  Position: 44 [Failed SQL: (0) update base_report set execution_id = cast(JC_EXEC_ID as bigint)]

Manually removing this parameter results in rundeck starting just fine.

I will also log an issue for this.