[CLOSED] Systemd Provider

[pjnorton]

Issue by TauZero
Wed Mar 23 19:37:55 2016
Originally opened as mistifyio/mistify#66

---

List any issues affected/resolved by this pr. https://github.com/blog/1506-closing-issues-via-pull-requests :
Resolves #52

Description:
Adds a basic Systemd Provider with the following task handlers available:

• systemd-create
• systemd-disable
• systemd-enable
• systemd-get
• systemd-list
• systemd-remove
• systemd-restart
• systemd-start
• systemd-stop

---

This change is

---

TauZero included the following code: https://github.com/mistifyio/mistify/pull/66/commits

[pjnorton]

Comment by mmlb
Fri Mar 25 18:10:42 2016

---

I'm thinking we should test this withing a mistify-os container like I'm doing in lochness.

---

Review status: 0 of 25 files reviewed at latest revision, all discussions resolved, some commit checks broke.

---

Comments from the review on Reviewable.io

[pjnorton]

Comment by TauZero
Fri Mar 25 22:04:02 2016

---

Since Travis only has ubuntu trusty, need to set it up like lochness to run in a mistifyos docker container.

[pjnorton]

Comment by joshie
Wed Mar 30 18:55:52 2016

---

LGTM

[pjnorton]

Comment by mmlb
Wed Mar 30 19:28:21 2016

---

Reviewed 16 of 28 files at r1, 13 of 13 files at r3.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

---

providers/systemd/action.go, line 43 [r3] (raw file):
shouldn't this be a struct member?

---

providers/systemd/config.go, line 36 [r3] (raw file):
this seems weird to trim any spaces here

---

providers/systemd/config_test.go, line 27 [r3] (raw file):
yeah I would expect for this to be an error

---

providers/systemd/create.go, line 43 [r3] (raw file):
is there an issue for this?

---

providers/systemd/create_test.go, line 28 [r3] (raw file):
zfs-create...? isn't it supposed to be systemd-create?

---

Comments from the review on Reviewable.io

[pjnorton]

Comment by TauZero
Wed Mar 30 19:38:32 2016

---

Review status: 25 of 26 files reviewed at latest revision, 5 unresolved discussions.

---

providers/systemd/action.go, line 43 [r3] (raw file):
Is it ok to leave that open? I got more of a "open and close as needed" feel from https://godoc.org/github.com/coreos/go-systemd/dbus#New . It's entirely possible I read too much into that and it's fine to hold it open, in which case I'll create it in systemd.New() and put it in the struct.

---

providers/systemd/config.go, line 36 [r3] (raw file):
It trims leading and trailing spaces.

---

providers/systemd/config_test.go, line 27 [r3] (raw file):
Depends if you agree with leading/trailing whitespace trimming. Very common in the webdev world, maybe not so common systems level?

---

providers/systemd/create.go, line 43 [r3] (raw file):
I'll create one in the new repo after the migration.

---

providers/systemd/create_test.go, line 28 [r3] (raw file):
Ah, woops. Doesn't really matter here, but good to fix.

---

Comments from the review on Reviewable.io

[pjnorton]

Comment by mmlb
Wed Mar 30 19:49:35 2016

---

Review status: 25 of 26 files reviewed at latest revision, 5 unresolved discussions.

---

providers/systemd/action.go, line 43 [r3] (raw file):
Yeah I think so, its similar to a tcp/udp Conn, open up early in the program and keep it around. each dbus connection is going to lead to a new unix:// connection + auth each request currently.

---

providers/systemd/config.go, line 36 [r3] (raw file):
my point is this shouldn't be done here.

---

providers/systemd/config_test.go, line 27 [r3] (raw file):
nope, we aren't forgiving. We expect for callers to mean what they say and say what they mean.

---

providers/systemd/create.go, line 43 [r3] (raw file):
sgtm

---

providers/systemd/create_test.go, line 28 [r3] (raw file):
you probably want to do

cd providers/systemd
sed -i 's|\(acomm.NewRequest("\)zfs|\1systemd|'

a lot of other cases too ;)

---

Comments from the review on Reviewable.io

[pjnorton]

Comment by mmlb
Wed Mar 30 20:03:33 2016

---

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

Comments from the review on Reviewable.io

[pjnorton]

Comment by TauZero
Wed Mar 30 20:40:14 2016

---

Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks broke.

---

providers/systemd/action.go, line 43 [r3] (raw file):
Done.

---

providers/systemd/config.go, line 36 [r3] (raw file):
Unforgiving it is. Done.

---

providers/systemd/create_test.go, line 28 [r3] (raw file):
Fixed.

---

Comments from the review on Reviewable.io

[pjnorton]

Comment by mmlb
Wed Mar 30 20:46:56 2016

---

Reviewed 15 of 15 files at r5.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from the review on Reviewable.io