Skip to content
This repository has been archived by the owner on Jul 27, 2023. It is now read-only.

Feature/collectd addon #1251

Merged
merged 1 commit into from
Apr 11, 2016
Merged

Feature/collectd addon #1251

merged 1 commit into from
Apr 11, 2016

Conversation

BrianHicks
Copy link
Contributor

  • Installs cleanly on a fresh build of most recent master branch
  • Upgrades cleanly from the most recent release
  • Updates documentation relevant to the changes
  • Rebases cleanly onto the latest master

Testing in progress, I just wanted to get this out here so we can have a discussion: this PR removes logstash's collectd forwarding behavior. Motivation is on 05c49f1. Does anyone have feedback of a better way to do this?

@@ -0,0 +1,4 @@
---
- hosts: localhost
Copy link
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 localhost?

@langston-barrett
Copy link
Contributor

Unfortunately, this fails with our Zookeeper checks (which check for /etc/collectd.d/zookeeper.conf).

@langston-barrett
Copy link
Contributor

Looks good otherwise, the addon playbook installs correctly and all services remain healthy. We'll just have to remove that distributive check, which will be part of the distributive refactor that's on its way.

@BrianHicks
Copy link
Contributor Author

@siddharthist is there a way we could remove that check now? Or move it to the collectd checks right away?

@langston-barrett
Copy link
Contributor

It is installed via the monolithic distributive-rpm. If you merge my PRs on mantl-packaging, I can put the new distributive PR in right away, we can merge that and then this.

Or, we could remove distributive.yml from the zookeeper role altogether in the meantime.

@ryane ryane added this to the 1.1 milestone Mar 10, 2016
@BrianHicks BrianHicks force-pushed the feature/collectd-addon branch from 84064c1 to 4589065 Compare March 10, 2016 18:04
@BrianHicks
Copy link
Contributor Author

Rebased onto latest master

@langston-barrett
Copy link
Contributor

This will be unblocked by #1268, which doesn't check for /etc/collectd.d/zookeeper.conf.

@ryane
Copy link
Contributor

ryane commented Mar 23, 2016

now that #1268 is in, we can probably get this merged as soon as the merge conflicts are resolved

@BrianHicks
Copy link
Contributor Author

BrianHicks commented Mar 23, 2016 via email

@BrianHicks BrianHicks force-pushed the feature/collectd-addon branch from 4589065 to cc550cc Compare March 23, 2016 14:23
@BrianHicks
Copy link
Contributor Author

this passes most checks but fails on DO for some reason, for different failure scenarios. Weird.

- name: check if mesos agent is present
stat:
path: /etc/sysconfig/mesos-agent
register: mesos_agent_present
Copy link
Contributor

Choose a reason for hiding this comment

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

is this being used anywhere?

@langston-barrett langston-barrett force-pushed the feature/collectd-addon branch from cc550cc to 51d9119 Compare April 4, 2016 03:40
@langston-barrett
Copy link
Contributor

rebased

@langston-barrett
Copy link
Contributor

@BrianHicks We've seen the most recent DO failure in CI builds before, is that what you were referring to? Is this ready to merge after we fix those variables that @ryane pointed out?

@ryane
Copy link
Contributor

ryane commented Apr 8, 2016

@BrianHicks are the mesos_agent_present mesos_master_present variables being used anywhere?

@BrianHicks
Copy link
Contributor Author

$ ag mesos_agent_present
roles/collectd/tasks/mesos-agent.yml
5:  register: mesos_agent_present

$ ag mesos_master_present
roles/collectd/tasks/mesos-master.yml
5:  register: mesos_master_present

so... no? Do you reckon they ought to be removed?

@ryane
Copy link
Contributor

ryane commented Apr 8, 2016

I kinda think so. they don't break anything but just so there is no confusion...

Note that this removes the logstash collectd integration. This needed to
happen because the Logstash role has become a generic log sink role, and
collectd logs are taking up a huge amount of room in that store. We'll
need to find a storage solution for collectd at a later date
@langston-barrett langston-barrett force-pushed the feature/collectd-addon branch from 0ca38b3 to 8d42e95 Compare April 8, 2016 16:32
@langston-barrett
Copy link
Contributor

I changed that, squashed the commit, and rebased. If tests pass, I'll merge.

@langston-barrett
Copy link
Contributor

Well, they all failed, but not for related reasons.

@ryane
Copy link
Contributor

ryane commented Apr 11, 2016

tested successfully on gce

@ryane ryane merged commit 7b076b9 into master Apr 11, 2016
@ryane ryane deleted the feature/collectd-addon branch April 11, 2016 22:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants