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

adding remote_write support #144

Merged
merged 12 commits into from
Jan 31, 2018
Merged

adding remote_write support #144

merged 12 commits into from
Jan 31, 2018

Conversation

gangsta
Copy link

@gangsta gangsta commented Jan 10, 2018

Hi,

Adding remote_write support for prometheus 2.X
https://prometheus.io/docs/prometheus/latest/configuration/configuration/#%3Cremote_write%3E

BR
Karen

@tuxmea
Copy link
Member

tuxmea commented Jan 10, 2018

Looks good to me. Can you add spec tests for the new parameter?

@gangsta
Copy link
Author

gangsta commented Jan 10, 2018

Hi @tuxmea ,

Isn't it defined here ?
And in pull request here

BR
Karen

@tuxmea
Copy link
Member

tuxmea commented Jan 10, 2018

Yes. But I am missing a test for the parameter, like: https://github.com/voxpupuli/puppet-prometheus/blob/master/spec/classes/prometheus_spec.rb#L203

@gangsta
Copy link
Author

gangsta commented Jan 14, 2018

Here I need some help , Cause read parameter is also missing tests, maybe someone can help me to write this tests?

@juniorsysadmin juniorsysadmin added enhancement New feature or request needs-tests labels Jan 15, 2018
@tuxmea
Copy link
Member

tuxmea commented Jan 16, 2018

Busy today. I can provide a test within tomorrow.

@gangsta
Copy link
Author

gangsta commented Jan 16, 2018

Thank you !

@gangsta
Copy link
Author

gangsta commented Jan 16, 2018

Hi @tuxmea ,

Adding Readme support for Prometheus 2.0.0 see #143

Karen added 4 commits January 17, 2018 10:24
Fix syntax at the end of remote_read and remote_write
fix
last one isn't required .
@gangsta
Copy link
Author

gangsta commented Jan 17, 2018

Another pull is included , support for alertmanager 0.13.0 see https://github.com/prometheus/alertmanager/releases/tag/v0.13.0

@tuxmea
Copy link
Member

tuxmea commented Jan 18, 2018

Thanks. In future: Please keep separate issues in separate PR.

@tuxmea
Copy link
Member

tuxmea commented Jan 18, 2018

@kalinux please find the following patch for spec:

diff --git a/spec/classes/prometheus_spec.rb b/spec/classes/prometheus_spec.rb
index 263d0e8..2590c21 100644
--- a/spec/classes/prometheus_spec.rb
+++ b/spec/classes/prometheus_spec.rb
@@ -253,6 +253,40 @@ describe 'prometheus' do
           end
         end
       end
+
+      context 'with remote write configured' do
+        [
+          {
+            version: '2.0.0-rc.1',
+            remote_write_configs: [
+              'url' => 'http://domain.tld/path',
+            ]
+          }
+        ].each do |parameters|
+          context "with prometheus version #{parameters[:version]}" do
+            let(:params) do
+              parameters
+            end
+
+            prom_version = parameters[:version] || '1.5.2'
+            prom_major = prom_version[0]
+
+            it {
+              is_expected.to compile
+            }
+            it {
+              is_expected.to contain_file('prometheus.yaml').with(
+                'ensure'  => 'present',
+                'path'    => '/etc/prometheus/prometheus.yaml',
+                'owner'   => 'prometheus',
+                'group'   => 'prometheus',
+                'content' => /http:\/\/domain.tld\/path/
+              )
+            }
+          end
+        end
+      end
     end
   end
 end
+

@gangsta
Copy link
Author

gangsta commented Jan 22, 2018

Hi @tuxmea ,

Please review code.

BR
Karen

@tuxmea
Copy link
Member

tuxmea commented Jan 22, 2018

Thank you.
Some rubocop tests fail:

spec/classes/prometheus_spec.rb:262:48: C: Style/TrailingCommaInLiteral: Avoid comma after the last item of an array. (https://github.com/bbatsov/ruby-style-guide#no-trailing-array-commas)
              'url' => 'http://domain.tld/path',
                                               ^
spec/classes/prometheus_spec.rb:272:13: W: Lint/UselessAssignment: Useless assignment to variable - prom_major. (https://github.com/bbatsov/ruby-style-guide#underscore-unused-vars)
            prom_major = prom_version[0]
            ^^^^^^^^^^
spec/classes/prometheus_spec.rb:283:30: C: Style/RegexpLiteral: Use %r around regular expression. (https://github.com/bbatsov/ruby-style-guide#percent-r)
                'content' => /http:\/\/domain.tld\/path/
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^

@gangsta
Copy link
Author

gangsta commented Jan 22, 2018

So how is this can be solved ? 🤔

@tuxmea
Copy link
Member

tuxmea commented Jan 23, 2018

@kalinux
in spec/classes/prometheus_spec.rb:
remove line 272 - here you specify a variable which you don't use
in line 262 remove the trailing comma
in line 283: use %r{http://domain.tld/path}

Try to run check tests prior pushing your changes. For workstation setup you can read on example42 blog developer setup and have a look at the puppet code development and testing section in the example42 blog summary

@gangsta
Copy link
Author

gangsta commented Jan 23, 2018

Hi @tuxmea ,

yeah! we did it, all checks are passed and my master branch is used by me in production so Prometheus 2.0.0 is working very well ! I thinks we are ready to merge branch . Thanks for awesome collaboration.

@tuxmea
Copy link
Member

tuxmea commented Jan 23, 2018

looks good to me. @bastelfreak any comments from your side?

bderickson pushed a commit to genome-vendor/puppet-prometheus that referenced this pull request Jan 30, 2018
Starting with 0.13.0, options must be given with two dashes instead of
one. This patch is borrowed from the larger (and as of time of writing
unmerged) PR found here:

Once voxpupuli#144 has been
merged, this entire fork can go away.
@gangsta
Copy link
Author

gangsta commented Jan 31, 2018

hi @tuxmea ,

I'll say Silence is a sign of acceptance from @bastelfreak . (ps I don't know the origin of “Silence is a sign of acceptance”)

Regards

@tuxmea
Copy link
Member

tuxmea commented Jan 31, 2018

;-) merging now.

@tuxmea tuxmea merged commit ad8e8df into voxpupuli:master Jan 31, 2018
cegeka-jenkins pushed a commit to cegeka/puppet-prometheus that referenced this pull request Aug 28, 2019
Rovanion pushed a commit to Rovanion/puppet-prometheus that referenced this pull request May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants