sous-chefs / openvpn

Development repository for the openvpn cookbook
https://supermarket.chef.io/cookbooks/openvpn
Apache License 2.0
98 stars 160 forks source link

Add support for Amazon Linux #179

Closed jgitlin-p21 closed 3 years ago

jgitlin-p21 commented 4 years ago

Add support for OpenVPN service under Amazon Linux 2. AL2 is basically Redhat 7, so the case node['platform_family'] when 'rhel' was used as the code for case node['platform_family'] when 'amazon'.

Fixes #178

Tested on Amazon Linux 2, converges successfully.

Signed-off-by: Josh Gitlin jgitlin@pinnacle21.com

Description

[Describe what this change achieves]

Issues Resolved

[List any existing issues this PR resolves]

Check List

jgitlin-p21 commented 4 years ago

Perfect, I'll make those changes early next week and post back, thanks @ramereth !

ramereth commented 4 years ago

@jgitlin-p21 could you also update to the latest master branch?

jgitlin-p21 commented 4 years ago

@ramereth CI tests fail for Amazon Linux 2 because the EPEL repo isn't enabled by default. This is what I use in my wrapper cookbook to fix the issue:

execute "enable epel repo" do
  command 'amazon-linux-extras install epel'
  creates "/etc/yum.repos.d/epel.repo"
end

I am not sure what the best fix is for the Sous Chef repo, because enabling EPEL feels out of scope, but at the same time the install recipe won't succeed on Amazon Linux 2 without it, so... I'm torn. I could refactor so the yum_package("openvpn") has a notifies "execute[enable epel repo]", :before and then the execute "enable epel repo" has an only_if { node["platform_family}] == "amazon" } and notifies "execute[disable epel repo", :delayed... 🤷‍♂️

ramereth commented 4 years ago

@jgitlin-p21 instead of doing it that way, why don't you modify: https://github.com/sous-chefs/openvpn/blob/7e34de239daf4bab99e1f5379597ed1a79710074/recipes/install.rb#L19

To use platform_family?('rhel', 'amazonlinux')? I think that should fix the problem.

jgitlin-p21 commented 3 years ago

I have not given up on this PR! Just been busy with unrelated things. Will try and make the proposed changes tomorrow.

jgitlin-p21 commented 3 years ago

OK @ramereth I think this should be good to go. danger is still failing but that's because I lack permissions?

ramereth commented 3 years ago

OK @ramereth I think this should be good to go. danger is still failing but that's because I lack permissions?

Yeah, one of the members has to manually trigger that job since you don't have permission. Looks good.

jgitlin-p21 commented 3 years ago

Apologies @jeffbyrnes and @ramereth I totally forgot there was feedback waiting on me here. I believe I resolved the issues; if you se anything else, or if you'd prefer that I squash-merge my two commits into one nice clean one, just let me know and I'm happy to address!

jgitlin-p21 commented 3 years ago

Decided to do the squash merge/force push to have a cleaner history, didn't like one small cleanup commit in there 🙂

kitchen-porter commented 3 years ago

Released as: 5.2.0

jeffbyrnes commented 3 years ago

@jgitlin-p21 awesome! Thanks for that.