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

Add configuration/support for NFVPE plugins #835

Merged
merged 1 commit into from
Jul 30, 2018

Conversation

paramite
Copy link
Contributor

Pull Request (PR) description

This patch adds support for NFVPE plugins, namely: sysevent, procevent and connectivity.
Those plugins are not merged yet in collectd itself, but once this (hopefully) happens,
we will have configuration patch ready for merge.

Collectd patches:
collectd/collectd#2624
collectd/collectd#2623
collectd/collectd#2622

This Pull Request (PR) fixes the following issues

n/a

@paramite paramite force-pushed the feature/nfvpe-plugins branch 2 times, most recently from ee5ff52 to 26cf834 Compare July 25, 2018 12:33
class collectd::plugin::connectivity (
Enum['present', 'absent'] $ensure = 'present',
Boolean $manage_package = $collectd::manage_package,
Array[String] $interfaces = [],
Copy link
Member

Choose a reason for hiding this comment

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

can you update this to Array[String[1]]? That enforces a minimal string length of one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

Enum['present', 'absent'] $ensure = 'present',
Boolean $manage_package = $collectd::manage_package,
Stdlib::Host $listen_host = '127.0.0.1',
Integer $listen_port = 6666,
Copy link
Member

Choose a reason for hiding this comment

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

Can you use this type please? https://github.com/puppetlabs/puppetlabs-stdlib/blob/master/types/port.pp. We then need to make sure that stdlib 4.25.0 is the lowest supported version in our metadata.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

Boolean $manage_package = $collectd::manage_package,
Optional[String] $process = undef,
Optional[String] $regex_process = undef,
Optional[Integer] $buffer_length = undef,
Copy link
Member

Choose a reason for hiding this comment

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

You can also enforce that this has to be a positive integer: https://puppet.com/docs/puppet/5.5/lang_data_number.html#the-integer-data-type

@paramite paramite force-pushed the feature/nfvpe-plugins branch from 26cf834 to 0e8043a Compare July 26, 2018 09:28
class collectd::plugin::procevent (
Enum['present', 'absent'] $ensure = 'present',
Boolean $manage_package = $collectd::manage_package,
Optional[String] $process = undef,
Copy link
Member

Choose a reason for hiding this comment

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

can you use Optional[String[1]] here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

Enum['present', 'absent'] $ensure = 'present',
Boolean $manage_package = $collectd::manage_package,
Optional[String] $process = undef,
Optional[String] $regex_process = undef,
Copy link
Member

Choose a reason for hiding this comment

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

can you use Optional[String[1]] here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

@bastelfreak bastelfreak added enhancement New feature or request needs-work not ready to merge just yet labels Jul 27, 2018
This patch adds support for NFVPE plugins, namely: sysevent, procevent
and connectivity
@paramite paramite force-pushed the feature/nfvpe-plugins branch from 0e8043a to 28c2b15 Compare July 30, 2018 10:02
@bastelfreak bastelfreak removed the needs-work not ready to merge just yet label Jul 30, 2018
@bastelfreak
Copy link
Member

Thanks!

@bastelfreak bastelfreak merged commit 6996110 into voxpupuli:master Jul 30, 2018
@bastelfreak bastelfreak changed the title Configuration for NFVPE plugins Add configuration/support for NFVPE plugins Jul 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants