Puppet v4 upgrade #362

bittner · 2017-10-27T15:11:09Z

This PR loosens the dependency on concat and ruby, and replaces puppetserver_gem, trying to address the following issues:

Puppet Server 4.x requires manual installation of puppetserver_gem #250 (puppetserver_gem) -- soft dependency mentioned in README
Concat requirement version bump #328 (concat)
puppetlabs-ruby required version bump #353 (ruby)

Add more versions for supported OSes

bittner · 2017-11-20T08:37:00Z

Any plans to merge this PR?

truthbk · 2017-11-22T17:01:29Z

@bittner I would love to merge this as it would clearly help us cleanup.... I'm not 100% sure off-hand but I think this would break for older puppets. We're trying to keep backward compatibility in this 1.x module.

But this would certainly be valuable for the next major module release which would in all likelihood be puppet 4+ compatible only.

bittner · 2017-11-22T21:42:41Z

You're not sure. That's one thing.

We're all sure that the current implementation breaks for newer Puppets, though. That is real.

truthbk · 2017-11-23T10:34:39Z

@bittner could you be more specific as to what version this breaks in. I'm not saying we're not going to merge this, I'm saying there's stuff in there that could break for some of our users.

truthbk

I agree that we should definitely add the new operating system release versions and release an updated module as soon as possible. I have reservations regarding system_gem though.

truthbk · 2017-11-23T10:39:19Z

metadata.json

+      "version_requirement": ">=2.2.0 <5.0.0"
+    },
+    {
+      "name": "tkishel/system_gem",


This right here is my main concern. See: https://github.com/tkishel/system_gem/blob/master/metadata.json#L24-L28

It's not supported for a bunch of puppet versions we do support, so I guess that's a no-no for now. I understand it addresses an important issue on windows.

truthbk · 2017-11-23T10:42:06Z

metadata.json

-        "12.04",
-        "14.04"
-      ]
+      "operatingsystemrelease": ["10.04", "12.04", "14.04", "16.04", "17.04", "17.10"]


We've only officially supported LTS versions thus far - we should probably stay consistent with that (for now)

bittner · 2017-11-23T13:30:04Z

How "more specific" can we be than referencing 3 specific issues that contain the details that motivate these changes?

that's a no-no for now

When you look at all the other issues and PRs where you say similar things please understand that DataDog users are getting frustrated with the support DataDog is providing for their closed-source platform. We, the "community", are doing free maintenance of this module and all DataDog employees are capable of is saying, "no, this is not going to be merged".

Find a solution that is better than ours. Give suggestions on how to change the PR so that it also covers all your concerns. Start writing or suggest more tests, extend the build matrix to cover the target technologies and versions you care about. -- By saying "no" you're not getting us anywhere, beyond frustration.

truthbk · 2017-11-23T19:37:13Z

@bittner I did never intend to imply this particular PR was not going to get merged. To the contrary, all I said was that tkishel/system_gem was not mergeable due to its implications for older puppets - that was indeed a suggestion on something that could be done to fix the PR as far as we're concerned. The "no-no" was related to that specific element of the PR. The review was meant to be an on-going discussion so we could address the small problems we may have had with it. In fact, that issue was the only big note I had on your PR and was hoping you could address that before we merged it. Your PR is well-thought, well written and will certainly be merged once we address the notes above.

The test matrix, which is green, covers all puppet versions we're meant to support. The problem is that although the CI does give a sense of when something will definitely break, it will not catch all cases where things might break. Depending on a module that does not support those older puppet versions is something we've been avoiding for a while due to issue we have encountered in the past. The puppetserver_gem is indeed one of those cases where a cleaner alternative unfortunately broke the module for our customers.

I apologize for the frustration this may have caused the community and you in particular, we absolutely appreciate the contributions from the community and this module has been driven by you guys from day one.

bittner · 2017-11-24T07:58:05Z

Sorry if I upset you. But to make things clear: Saying somethings is no good is not the same as a suggestion what to better do instead. I replaced the gem-related library for a reason. You should have read the related issue. And I am not alone with this problem.

Please stop making excuses. This only makes things worse. For Datadog. There is clearly passion missing in the maintenance of this module.

truthbk · 2017-11-24T09:57:23Z

@bittner I'm not sure there is a satisfactory solution for the puppetserver_gem issue until we drop support for older puppet versions - as far as I can tell any fix will break the module for some portion of our users. You'd be surprised at the number of people still on legacy puppets.

The solution, I believe goes through releasing a new major version of the module that drops support for older puppets. That said, most of the work here would benefit the current module.

Ideally, in that new module scenario integrations should be resources, as opposed to the current approach. But we could maybe leave that for a yet another major release after that one.

* Bump versions for dependencies Add more versions for supported OSes * Use system_gem instead of puppetserver_gem * Revert "Use system_gem instead of puppetserver_gem" This reverts commit df2cfcd. * [metadata] puppetserver is a _soft_ dependency that would break older puppets - install manually

ardrigh · 2017-12-12T02:53:43Z

@truthbk

Can I ask what Datadog's business thinking for supporting unsupported versions of Puppet?
Is there an official time frame for branching to a 2.x release to fix these types of problems?

Version 6 of the Datadog agent is available as a beta, and our company is looking at trying to use it to test (hopefully with this module).

Now seems like it would be a great time to cut some losses on unsupported versions of Puppet and create branch for release 2.x? People can submit fixes to a 1.x branch if they need to pin more gems.

The Puppet Enterprise LTS roadmap lists 2016.4 as the minimum. But this module lists requirement for PE 3.2. And Puppet 2.7 when the current release is 5.3

Customers on the older inactive versions are strongly encouraged to plan adopting the newer releases to take full benefits of latest features Puppet has to offer. Puppet Enterprise versions 2015.2, 2015.3, 2016.1, 2016.2, 2016.5 and 2017.1 are past the extended support period and will enter the End of Life (EOL) phase as of February 21, 2018. Versions 3.8 and older have been in EOL phase since December 2016. Please contact the technical support team for advice and assistance on moving from an inactive version to a current version of Puppet Enterprise.

I looked at merging our fork with the current release and the Travis failed because another gem requires Ruby 2.1 - so half the test suite automatically failed. That's going to become a losing maintenance game and certainly not something I would bother fixing.

PE 2016.4 runs on Ruby 2.1.9 - that cuts out a good half of the travis.yml lines by itself.

You could also entirely remove the module dependency on remote_file by updating to Puppet 4 that supports http: for a file resource. The currently supported Puppet LTS release supports this.
Puppet Docs for 4.6 show it is supported for a few versions now.

There would be a few PRs I would submit to fix some problems, but I am really starting to question how useful this module will be if it continues to be pinned to Puppet 2.7

bittner · 2017-12-12T09:40:34Z

@ardrigh Datadog has told me that tomorrow (and every two months) they have community code review at 4pm CET. I'm not sure whether this is a public meeting, though. The meeting location is G+ Hangouts and the #integrations channel of https://datadog.slack.com.

@masci Can you tell us more? Maybe this would be a chance to get good input from the community at large? (Unfortunately, I can't join tomorrow due to a client meeting.)

truthbk · 2017-12-12T11:54:08Z

@ardrigh there are plans in the short-term to drop support for old puppets. What you outline is exactly what we have decided to do. We do have customers on puppet 3.x and the 1.x version of the module has been important to maintain for them.

That said, the plan is to release a new 2.x version very early in 2018 (targeting late January) to support active puppet versions and drop support for older puppets moving forward. I will be opening a PR very soon.

Although we might backport features if heavily requested (hoping to phase out 1.x gradually as customers abandon EOL'd puppets), development moving forward will be focused on providing a module relevant in the current context of puppet.

I know it's been a little painful, hopefully we can make that a thing of the past very soon.

Thank you!

truthbk · 2017-12-12T11:56:20Z

@ardrigh on another note, regarding beta 6. There was work merged into this module to support agent6 - docs will get updated ahead of the 1.12.0 release. I have tested the current state of master on puppet5 and it works fine, I presume you might have to cherry-pick some of the work into your fork as opposed to a full merge to get it to fly with your fork if you would rather not use the official module.

Let me know if you need more information or help getting it to work.

ardrigh · 2017-12-12T20:00:32Z

@truthbk thanks for the replies, that's all very helpful information :)

We don't have many changes in our fork at the moment. The previous rpm key changes were integrated into the main release along with the pinned gem fix.

I did see the updates for the beta 6 agent added to this module. Is the 1.12 release due soon?
We can wait to work off that release.

bittner · 2017-12-12T20:17:51Z

I did see the updates for the beta 6 agent added to this module

@ardrigh Can you quote the commit that made this? We're having trouble, it seems, exactly with that addition (see below for the output of an RSpec test we have). That makes it impossible for us to use the current master branch.

Evaluation Error: Unknown variable: '::apt_agent6_beta_repo'.

ardrigh · 2017-12-12T20:49:41Z

@bittner ah I was just referring to the commit messages for #356

I haven't actually tried to run the current master, I pulled in the tag 1.11 to look at against our changes.
I might have some time to look at the current master later.

) * Bump versions for dependencies Add more versions for supported OSes * Use system_gem instead of puppetserver_gem * Revert "Use system_gem instead of puppetserver_gem" This reverts commit df2cfcd. * [metadata] puppetserver is a _soft_ dependency that would break older puppets - install manually

truthbk · 2018-02-27T12:07:14Z

#387 has been merged and should address most if not all concerns raised in this PR. It will be released soon (like today). Please open any PRs to address any possible bug or limitation against the latest master after the release. Thank you @bittner

@ardrigh #387 should allow you to jump on the agent6 bandwagon hopefully without too many issues. Let us know how it goes. As said above, we will be releasing v 2.0 of the module later today.

ardrigh · 2018-02-28T01:41:03Z

@truthbk Thanks for the update. I'll give the new release a spin when it's officially available.

bittner added 2 commits October 27, 2017 15:26

Bump versions for dependencies

6bb9407

Add more versions for supported OSes

Use system_gem instead of puppetserver_gem

df2cfcd

truthbk self-assigned this Nov 16, 2017

truthbk reviewed Nov 23, 2017

View reviewed changes

truthbk mentioned this pull request Nov 24, 2017

Puppet v4 upgrades - subset of @bittner work in #362 #370

Merged

truthbk added the community label Nov 24, 2017

ghost force-pushed the feature/puppet-v4-upgrade branch 2 times, most recently from 5623b0d to df2cfcd Compare February 5, 2018 16:17

truthbk closed this Feb 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Puppet v4 upgrade #362

Puppet v4 upgrade #362

bittner commented Oct 27, 2017

bittner commented Nov 20, 2017

truthbk commented Nov 22, 2017 •

edited

Loading

bittner commented Nov 22, 2017

truthbk commented Nov 23, 2017

truthbk left a comment

truthbk Nov 23, 2017 •

edited

Loading

truthbk Nov 23, 2017

bittner commented Nov 23, 2017

truthbk commented Nov 23, 2017

bittner commented Nov 24, 2017

truthbk commented Nov 24, 2017 •

edited

Loading

ardrigh commented Dec 12, 2017

bittner commented Dec 12, 2017 •

edited

Loading

truthbk commented Dec 12, 2017 •

edited

Loading

truthbk commented Dec 12, 2017

ardrigh commented Dec 12, 2017

bittner commented Dec 12, 2017

ardrigh commented Dec 12, 2017

truthbk commented Feb 27, 2018

ardrigh commented Feb 28, 2018

Puppet v4 upgrade #362

Puppet v4 upgrade #362

Conversation

bittner commented Oct 27, 2017

bittner commented Nov 20, 2017

truthbk commented Nov 22, 2017 • edited Loading

bittner commented Nov 22, 2017

truthbk commented Nov 23, 2017

truthbk left a comment

Choose a reason for hiding this comment

truthbk Nov 23, 2017 • edited Loading

Choose a reason for hiding this comment

truthbk Nov 23, 2017

Choose a reason for hiding this comment

bittner commented Nov 23, 2017

truthbk commented Nov 23, 2017

bittner commented Nov 24, 2017

truthbk commented Nov 24, 2017 • edited Loading

ardrigh commented Dec 12, 2017

bittner commented Dec 12, 2017 • edited Loading

truthbk commented Dec 12, 2017 • edited Loading

truthbk commented Dec 12, 2017

ardrigh commented Dec 12, 2017

bittner commented Dec 12, 2017

ardrigh commented Dec 12, 2017

truthbk commented Feb 27, 2018

ardrigh commented Feb 28, 2018

truthbk commented Nov 22, 2017 •

edited

Loading

truthbk Nov 23, 2017 •

edited

Loading

truthbk commented Nov 24, 2017 •

edited

Loading

bittner commented Dec 12, 2017 •

edited

Loading

truthbk commented Dec 12, 2017 •

edited

Loading