Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wip ci #382

Closed
wants to merge 16 commits into from
Closed

wip ci #382

wants to merge 16 commits into from

Conversation

h0tw1r3
Copy link
Contributor

@h0tw1r3 h0tw1r3 commented Jan 27, 2024

aside from syntax, there are no puppet code changes.

h0tw1r3 and others added 6 commits December 19, 2023 14:02
For now we want to have running CI.  We do not care about the module
being fully documented.
* Fix acceptance tests to work with Litmus
* Fix acceptance test installation path of puppet modules
* update/add dependencies to fixtures
* Fix CI failures related to legacy facts
* More CI fixes with latest posgresql module
* Remove local facts overrides
  They break adding facts from a context.
* Match the ensure value of the manifest
  This was changed in 68d8c64 to fix
  puppet lint issues, but as the test suite was not run it passed through.
* Match owner / mode of the manifest
  This was changed in 010bf13 to match
  best practice, but as the test suite was not run it passed through.
* Add path to the custom facts
  The systemd module use this to run `systemctl daemon-reload`.  Set an
  arbitrary value to avoid:
  > Validation of Exec[systemd-postgresql.service-systemctl-daemon-reload] failed: 'systemctl' is not qualified and no path was specified. Please qualify the command or specify a path.
* patched cas module_ci workflow to disable the dependency checker
@@ -78,11 +78,11 @@
"requirements": [
{
"name": "puppet",
"version_requirement": ">= 4.10.0 < 9.0.0"
"version_requirement": ">= 7.0.0 < 9.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This drops a significant amount of major versions. Maybe that can be done in a dedicated PR so it's more present in the changelog?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe that was in the squashed #344
Is this a blocker? I can't see how it would matter. No reasonable way to test on agents that old.

Copy link
Collaborator

Choose a reason for hiding this comment

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

if it matters depends on the CHANGELOG.md quality Puppet wants to have. For Vox Pupuli it's important to document those breaking changes. I suggest to make this change in a dedicated PR and rebase your good-times branch against it.

spec/default_facts.yml Outdated Show resolved Hide resolved
.sync.yml Outdated Show resolved Hide resolved
.sync.yml Show resolved Hide resolved
}

describe 'puppetdb::database::read_grant' do
let(:facts) { on_supported_os.take(1).first[1] }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you limit the tests to one OS instead of all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because that's the way it was :) The change was a simple refactor to remove the let(:facts) blocks all over the spec tests.
Many of the classes, which in some cases are just resource wrappers, there is no purpose to testing all platforms.

@@ -32,7 +32,10 @@ class { 'postgresql::server':
end

describe 'manifest' do
it { is_expected.to compile.with_all_deps }
it {
pending('platform support to be removed') if facts[:os]['name'] == 'Ubuntu' && (Puppet::Util::Package.versioncmp(facts[:os]['release']['major'], '18.04') < 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

what about dropping Ubuntu 16.04 in another PR stead of marking it as pending?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree another PR is needed, and one is open.
I prefer all tests pass on this PR, which should open up merging the backlog.

* centos/oracle 7 docker hanging, cgroupsv1?
* scientific 7 is just broken, no testing for you
* release of postgres module is deploying expired gpg keys for all EL
  versions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants