Modulesync 5.4.0 #418

h-haaks · 2022-11-07T00:39:57Z

No description provided.

h-haaks · 2022-11-07T01:03:38Z

manifests/plugin.pp

@@ -43,7 +43,7 @@
  $source = undef,
  $ensure = present,
  $environment = [],
-  String $user = $logstash::logstash_user,
+  String $user = 'root',


Package installs logstash as root and logstash-plugin command needs access to /usr/share/logstash/Gemfile

this is a bit tricky. will this break existing installations or did it never work?

We don't use any plugins and I really don't know logstash that well.
But I do not see how this could ever have worked.
The default $logstash::logstash_user is not created by the module, its created by the package.
The package installs logstash at /usr/share/logstash with root as owner.
$logstash_user is only used to run the service.

Even if someone has a profile class that changes the ownership of /usr/share/logstash and they don't override the $user param, new plugins will be installed with root as owner but I do think logstash running as a less privileged user will still have read access to the installed plugin.

Might be a good idea to mention this in the release notes though.

@bastelfreak have a look at #413

@thebeanogamer do you have any input here?

h-haaks · 2022-11-07T01:04:57Z

manifests/service.pp

@@ -16,7 +16,6 @@
  }

  $default_startup_options = {
-    'JAVACMD'             => '/usr/bin/java',


Removed this to use the jdk installed by the package.

h-haaks · 2022-11-07T01:05:34Z

manifests/service.pp

-    '-XX:+UseCMSInitiatingOccupancyOnly',
-    '-XX:+UseConcMarkSweepGC',
-    '-XX:+UseParNewGC',
-  ]


This was never used. The same default are in the template ..

h-haaks · 2022-11-07T01:07:01Z

spec/acceptance/class_logstash_spec.rb

@@ -4,7 +4,7 @@

 shared_examples 'a logstash installer' do
  it "installs logstash version #{LS_VERSION}" do
-    expect(shell('/usr/share/logstash/bin/logstash --version').stdout).to eq("logstash #{LS_VERSION}\n")
+    expect(shell('/usr/share/logstash/bin/logstash --version').stdout).to contain("logstash #{LS_VERSION}")


logstash --version now prints the jdk used as well as the version.

This test is actually a bit tricky to have in shared_examles since it is used in examples that install the latest version available in the yum repo, and it will fail whenever elastic releases a new patch release ...
I had to update the default LS_VERSION in helpers.rb to make the test pass.

h-haaks · 2022-11-07T01:08:17Z

spec/acceptance/class_logstash_spec.rb

@@ -209,8 +210,6 @@
          '-XX:+DisableExplicitGC',
          '-XX:+HeapDumpOnOutOfMemoryError',
          '-XX:+UseCMSInitiatingOccupancyOnly',
-          '-XX:+UseConcMarkSweepGC',
-          '-XX:+UseParNewGC',


Unsupported java options.

h-haaks · 2022-11-07T01:09:53Z

spec/acceptance/class_plugin_spec.rb

+    remove_logstash
+    install_logstash('status => "disabled", restart_on_change => false')
+  end
+


Added this to be able to run this spec file alone

h-haaks · 2022-11-07T01:11:03Z

spec/acceptance/define_patternfile_spec.rb

@@ -5,7 +5,7 @@
 describe 'class patternfile' do
  def apply_pattern(pattern_number, extra_logstash_class_args = nil)
    manifest = <<-END
-      #{install_logstash_from_local_file_manifest(extra_logstash_class_args)}
+      #{install_logstash_manifest(extra_logstash_class_args)}


Install from repo so that the tests could be run separatly

h-haaks · 2022-11-07T06:51:15Z

Fixes #391, #400, #402, #408

h-haaks · 2022-11-07T07:05:41Z

@bastelfreak could you please have a look at this.

ekohl · 2022-12-14T10:47:48Z

spec/defines/define_plugin_spec.rb

@@ -7,17 +9,19 @@
        facts
      end

-      let(:pre_condition) { %q(
+      let(:pre_condition) do
+        '


For multi line strings I'd suggest heredoc

<<~PUPPET include elastic_stack::repo include logstash PUPPET

ekohl · 2022-12-14T10:48:59Z

spec/support/acceptance/helpers.rb

+PUPPET_VERSION = ENV['PUPPET_VERSION'] || '4.10.7'
+
+PE_VERSION = ENV['BEAKER_PE_VER'] || ENV['PE_VERSION'] || '3.8.3'
+PE_DIR = ENV['BEAKER_PE_DIR']


This should be removed IMHO. If it's still used, it should be refactored to be dropped.

ekohl · 2022-12-14T10:49:08Z

spec/support/acceptance/helpers.rb

@@ -0,0 +1,239 @@
+# frozen_string_literal: true
+
+require 'beaker-rspec'


This should be part of our generic helper.

ekohl · 2022-12-14T10:50:26Z

spec/support/acceptance/helpers.rb

+                  false
+                end
+
+def agent_version_for_puppet_version(puppet_version)


This should be removed. We don't test on Puppet 4 anymore so if it's still called it's redundant.

ekohl · 2022-12-14T10:50:57Z

spec/support/acceptance/helpers.rb

+def pe_package_url
+  distro, distro_version = ENV['BEAKER_set'].split('-')
+  case distro
+  when 'debian'
+    os = 'debian'
+    arch = 'amd64'
+  when 'centos'
+    os = 'el'
+    arch = 'x86_64'
+  when 'ubuntu'
+    os = 'ubuntu'
+    arch = 'amd64'
+  end
+  url_root = "https://s3.amazonaws.com/pe-builds/released/#{PE_VERSION}"
+  "#{url_root}/puppet-enterprise-#{PE_VERSION}-#{os}-#{distro_version}-#{arch}.tar.gz"
+end
+
+def pe_package_filename
+  File.basename(pe_package_url)
+end
+
+def puppet_enterprise?
+  ENV['BEAKER_IS_PE'] == 'true' || ENV['IS_PE'] == 'true'
+end


We don't support Puppet Enterprise in our CI so this should be dropped.

ekohl · 2022-12-14T10:52:58Z

I'll also note that our current modulesync config is at 5.3.0 so perhaps you could also include that. If you need help with that, please let me know.

h-haaks · 2022-12-22T07:01:02Z

@ekohl I have fixed your comments now and I had a look at https://github.com/voxpupuli/modulesync_config to see if I could fix that as well.
I think I need some help with this ..

h-haaks · 2022-12-22T07:25:25Z

Hmm, it seems like puppet-logstash is missing in https://github.com/voxpupuli/modulesync_config/blob/master/managed_modules.yml

h-haaks · 2022-12-22T11:13:05Z

@ekohl, @bastelfreak Could you please have another look at this?
I think it would be smart to get this merged before the 5.4.0 release of modulesync.

h-haaks · 2022-12-22T11:31:34Z

Should README.md have been updated according to the elastic -> voxpupuli transition and logstash version constraints adjusted to supported versions?

kenyon · 2022-12-22T16:11:34Z

@h-haaks yes (in a separate pull request).

ekohl · 2022-12-22T17:22:55Z

I think it would be smart to get this merged before the 5.4.0 release of modulesync.

The differences between 5.3.0 and 5.4.0 are trivial so I'd include that here.

ekohl

Generally I'm not too happy about making changes in the code itself in a modulesync PR, but if it gets the module moving forward I'm happy to approve.

ekohl · 2022-12-22T17:25:07Z

spec/support/acceptance/helpers.rb

+IS_PRERELEASE = if LS_VERSION =~ %r{(alpha|beta|rc)}
+                  true
+                else
+                  false
+                end


Suggested change

IS_PRERELEASE = if LS_VERSION =~ %r{(alpha|beta|rc)}

true

else

false

end

IS_PRERELEASE = LS_VERSION.match?(%r{(alpha|beta|rc)})

h-haaks · 2022-12-23T07:10:19Z

The differences between 5.3.0 and 5.4.0 are trivial so I'd include that here.

Is it ok to run msync from the master branch before the 5.4.0 tag is created?

h-haaks · 2022-12-23T07:20:46Z

Generally I'm not too happy about making changes in the code itself in a modulesync PR, but if it gets the module moving forward I'm happy to approve.

I do agree. All of this is just parts of transitioning this module into voxpupuli, I think. The modulesync commits are just part of that.
I'll gladly change the title to something like 'Get module to build with voxpupuli'.

ekohl · 2022-12-23T20:25:18Z

Is it ok to run msync from the master branch before the 5.4.0 tag is created?

I think @bastelfreak forgot to push a tag because he did create voxpupuli/modulesync_config@2f1495f so just go for it.

h-haaks · 2022-12-29T13:09:11Z

@ekohl I don't have write access to the repository. Could you please merge this PR?

ekohl · 2022-12-29T13:37:54Z

Thanks for taking this over the finish.

h-haaks force-pushed the fix-modulesync branch 2 times, most recently from 78b5c51 to f48ef6c Compare November 7, 2022 00:53

h-haaks commented Nov 7, 2022

View reviewed changes

h-haaks force-pushed the fix-modulesync branch 2 times, most recently from c0b87b7 to c936be5 Compare November 7, 2022 02:01

h-haaks mentioned this pull request Dec 14, 2022

modulesync 5.2.0 #408

Closed

ekohl changed the title ~~Fix modulesync~~ Modulesync 5.2.0 Dec 14, 2022

ekohl added the modulesync label Dec 14, 2022

ekohl reviewed Dec 14, 2022

View reviewed changes

bastelfreak and others added 12 commits December 21, 2022 13:43

modulesync 5.2.0

5bae7c1

rubocop: safe autocorrect

78a8798

rubocop: Fix Style/FrozenStringLiteralComment

c049d5a

Add Puppet 6/7 support

30c0cbd

Drop Puppet 4/5 support

d9cd804

Drop EoL CentOS 6 support

621828a

Delete legacy nodesets

8f7f857

drop useless acceptance test

0a7695d

rubocop: fix remaining violations

8b80167

add old spec_helper_acceptance.rb as helpers.rb

bf7b606

WIP: 6785c72 Fix errors related to modulesync

35f8039

Alligning os support with puppet-elastic_stack

6022ddc

h-haaks force-pushed the fix-modulesync branch from c936be5 to 6022ddc Compare December 21, 2022 12:50

h-haaks added 2 commits December 21, 2022 14:31

fix broken test after rebase

d0169c2

Cleanup unused helper code

0230b27

h-haaks force-pushed the fix-modulesync branch from f3b7422 to 0230b27 Compare December 21, 2022 22:47

h-haaks mentioned this pull request Dec 22, 2022

Add puppet-logstash voxpupuli/modulesync_config#804

Merged

modulesync 5.3.0

84afaba

h-haaks changed the title ~~Modulesync 5.2.0~~ Modulesync 5.3.0 Dec 22, 2022

h-haaks mentioned this pull request Dec 22, 2022

Convert module to PDK and cleanup tests #402

Closed

kenyon mentioned this pull request Dec 22, 2022

updated libraries and version of puppet #419

Closed

ekohl approved these changes Dec 22, 2022

View reviewed changes

modulesync 5.4.0

da86a8a

h-haaks changed the title ~~Modulesync 5.3.0~~ Modulesync 5.4.0 Dec 29, 2022

ekohl approved these changes Dec 29, 2022

View reviewed changes

ekohl merged commit 804e85a into voxpupuli:master Dec 29, 2022

h-haaks deleted the fix-modulesync branch December 29, 2022 13:51

This was referenced Dec 29, 2022

Cleanup failing tests #401

Closed

Finalize voxpupuli transition #420

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modulesync 5.4.0 #418

Modulesync 5.4.0 #418

h-haaks commented Nov 7, 2022

h-haaks Nov 7, 2022

bastelfreak Nov 9, 2022

h-haaks Nov 9, 2022

h-haaks Nov 9, 2022

h-haaks Dec 14, 2022

h-haaks Nov 7, 2022

h-haaks Nov 7, 2022

h-haaks Nov 7, 2022

h-haaks Nov 7, 2022

h-haaks Nov 7, 2022

h-haaks Nov 7, 2022

h-haaks Nov 7, 2022

h-haaks commented Nov 7, 2022 •

edited

Loading

h-haaks commented Nov 7, 2022

ekohl Dec 14, 2022

ekohl Dec 14, 2022

ekohl Dec 14, 2022

ekohl Dec 14, 2022

ekohl Dec 14, 2022

ekohl commented Dec 14, 2022

h-haaks commented Dec 22, 2022

h-haaks commented Dec 22, 2022

h-haaks commented Dec 22, 2022

h-haaks commented Dec 22, 2022

kenyon commented Dec 22, 2022

ekohl commented Dec 22, 2022

ekohl left a comment

ekohl Dec 22, 2022

h-haaks commented Dec 23, 2022

h-haaks commented Dec 23, 2022

ekohl commented Dec 23, 2022

h-haaks commented Dec 29, 2022

ekohl commented Dec 29, 2022

		@@ -0,0 +1,239 @@
		# frozen_string_literal: true

		require 'beaker-rspec'

Modulesync 5.4.0 #418

Modulesync 5.4.0 #418

Conversation

h-haaks commented Nov 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h-haaks commented Nov 7, 2022 • edited Loading

h-haaks commented Nov 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekohl commented Dec 14, 2022

h-haaks commented Dec 22, 2022

h-haaks commented Dec 22, 2022

h-haaks commented Dec 22, 2022

h-haaks commented Dec 22, 2022

kenyon commented Dec 22, 2022

ekohl commented Dec 22, 2022

ekohl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h-haaks commented Dec 23, 2022

h-haaks commented Dec 23, 2022

ekohl commented Dec 23, 2022

h-haaks commented Dec 29, 2022

ekohl commented Dec 29, 2022

h-haaks commented Nov 7, 2022 •

edited

Loading