Skip to content

Add manage_install option#608

Merged
albertvaka merged 8 commits intomasterfrom
albertvaka/add-manage_install
Feb 19, 2020
Merged

Add manage_install option#608
albertvaka merged 8 commits intomasterfrom
albertvaka/add-manage_install

Conversation

@albertvaka
Copy link
Copy Markdown
Contributor

@albertvaka albertvaka commented Feb 11, 2020

  • Add manage_install parameter to disable installing the Agent
  • Split service and repo management, since service code is the same for all Linuxes while repo code is not.

Fixes: #476
Fixes: #604

@albertvaka albertvaka force-pushed the albertvaka/add-manage_install branch 2 times, most recently from 9a168f3 to 9f824ed Compare February 11, 2020 19:35
@albertvaka albertvaka force-pushed the albertvaka/add-manage_install branch from 9f824ed to c5393fd Compare February 12, 2020 10:40
@albertvaka albertvaka force-pushed the albertvaka/add-manage_install branch from 11d900b to 79762cf Compare February 12, 2020 12:40
Comment thread manifests/init.pp Outdated
# $manage_repo
# Boolean to indicate whether this module should attempt to manage
# the package repo. Only for RPM-based distros. Default true.
# Deprecated. Use $manage_install instead.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this really be deprecated? I think $manage_repo and $manage_install have two different purposes:

  • $manage_install indicates that you want the module to install the Agent (which you can turn off if you have custom modules installing it for you)
  • $manage_repo indicates that you want the module to set up the Agent repo (which you can turn off if you have custom modules setting it up for you).

Moreover, the current PR does not really fix #604, as it's still not possible to install the Agent without setting up the repos on Ubuntu.

Copy link
Copy Markdown
Contributor Author

@albertvaka albertvaka Feb 18, 2020

Choose a reason for hiding this comment

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

It's true that manage_install has more implications than manage_repo but since manage_repo wasn't implemented for Ubuntu, I thought that a wider thing like manage_install (which we needed anyway for #476) could also work for both use cases.

Maybe the wording of the deprecation message should be Set manage_install to false and install datadog-agent manually to achieve the same thing as before. I'll change this.

Another option would be to keep both manage_repo and manage_install but then we have to implement it for Ubuntu as well and given how little these options are used, I prefer having one flag that two. Too many options increase complexity.

Comment thread manifests/service.pp
#

class datadog_agent::service(
$service_ensure = 'running',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
$service_ensure = 'running',
String $service_ensure = 'running',

Copy link
Copy Markdown
Contributor Author

@albertvaka albertvaka Feb 19, 2020

Choose a reason for hiding this comment

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

This can also be an enum type (ie: running without quotes) so I don't want to force the type. We do this in several other places in the module.

@albertvaka albertvaka merged commit 7a4ce78 into master Feb 19, 2020
@albertvaka albertvaka deleted the albertvaka/add-manage_install branch February 19, 2020 14:21
cegeka-jenkins pushed a commit to cegeka/puppet-datadog_agent that referenced this pull request Apr 6, 2020
- Add manage_install parameter to disable installing the Agent
- Split service and repo management, since service code is the same for all Linuxes while repo code is not.
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.

$manage_repo not implemented for ubuntu agent

2 participants