Review Request 226: Add helper to retrieve the target unit name
by Stephen Gallagher
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/226/
-----------------------------------------------------------
Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen Gallagher, and Thomas Woerner.
Repository: rolekit
Description
-------
Add helper to retrieve the target unit name
Diffs
-----
src/rolekit/util.py 4c29d5afde87fa463846355fed7ae1c44255a385
src/rolekit/server/rolebase.py 510190a5fbd66858417e9f75f73ded63e6236b85
src/rolekit/server/io/rolesettings.py c2424f1b166eb9eec5c59e3b79047cb482ca9081
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/226/diff/
Testing
-------
Thanks,
Stephen Gallagher
8 years, 5 months
Review Request 227: Don't attempt to force the role to a state
by Stephen Gallagher
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/227/
-----------------------------------------------------------
Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen Gallagher, and Thomas Woerner.
Repository: rolekit
Description
-------
Previously, when we started up we would check whether a role was
in the state we expected it to be (ready-to-start or running) and
start or stop it if it didn't match our expectation. This is the
wrong behavior; we need to be reporting the real state and not
arbitrarily altering the system. (For example, someone may have
chosen to stop one of the services through systemd for a good
reason).
Diffs
-----
src/rolekit/server/rolebase.py 510190a5fbd66858417e9f75f73ded63e6236b85
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/227/diff/
Testing
-------
Thanks,
Stephen Gallagher
8 years, 5 months
Review Request 228: Remove failure-notification signal code
by Stephen Gallagher
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/228/
-----------------------------------------------------------
Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen Gallagher, and Thomas Woerner.
Repository: rolekit
Description
-------
This is in preparation for the new approach where we rely on systemd
to properly report its state, rather than expecting services to
notify us when they fail (which is bad if we miss something).
Diffs
-----
src/rolekit/server/roled.py d9c7d5e1df71ad3341d54877174cbabb991ad91e
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/228/diff/
Testing
-------
Thanks,
Stephen Gallagher
8 years, 5 months
Review Request 230: Get role state from systemd
by Stephen Gallagher
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/230/
-----------------------------------------------------------
Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen Gallagher, and Thomas Woerner.
Repository: rolekit
Description
-------
Get role state from systemd
Diffs
-----
src/rolekit/config/__init__.py.in 05d9f7b2bda59cf0b4ed6e1f277880ff2092b3c7
src/rolekit/dbus_utils.py a5f5a1d5ee4f32865fca6459615ea3359f3ca11d
src/rolekit/server/io/rolesettings.py c2424f1b166eb9eec5c59e3b79047cb482ca9081
src/rolekit/server/io/systemd.py b105483c7fa689a43a960ef78aa1105f7aa58b42
src/rolekit/server/rolebase.py 510190a5fbd66858417e9f75f73ded63e6236b85
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/230/diff/
Testing
-------
Thanks,
Stephen Gallagher
8 years, 5 months
Review Request 231: Simplify creating target units
by Stephen Gallagher
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/231/
-----------------------------------------------------------
Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen Gallagher, and Thomas Woerner.
Repository: rolekit
Description
-------
We no longer require the failure unit or the extension units. Now
that we are getting the state from systemd directly, we will simply
set up the Requires/RequiredBy/BindsTo settings appropriately so
that rolekit and systemd are always kept in sync.
Diffs
-----
config/roles/databaseserver/role.py 1af90551aff0f681ee8c2f787780e76f24330927
config/roles/domaincontroller/role.py 9fefd60aed47888d2e7884a4b70a6ad76dd104d8
config/roles/memcache/role.py d12974128a75ef38666bbdd7d70b7623631e3166
config/roles/testrole/role.py b5fda8dc92b42735a7b0ce5c6b21fed7cf6fd015
src/rolekit/server/rolebase.py 510190a5fbd66858417e9f75f73ded63e6236b85
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/231/diff/
Testing
-------
Thanks,
Stephen Gallagher
8 years, 5 months
Review Request 232: Update Vagrantfile for Fedora 23
by Stephen Gallagher
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/232/
-----------------------------------------------------------
Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen Gallagher, and Thomas Woerner.
Repository: rolekit
Description
-------
Also add workaround for docker bug
The docker bug is that in many cases (including the Vagrant case), Docker 1.8.2 (the current version in the stable repository) will fail to initialize on first startup. So this patch forces us to use the known-good 1.7.0 from the Fedora 23 frozen package set in order to do the first-time initialization, then we update to the latest version. This part of the patch can be reverted once 1.9.1 lands in the stable repository, since it seems to fix the issue.
Diffs
-----
Vagrantfile 27230401a49f10756198f6153080938b655153e3
contrib/vagrant/rolekitdev-bootstrap.sh 43f374b9f6ddff22b7f80c6abd18ce1425bae125
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/232/diff/
Testing
-------
Built and tested changes for reviews 226-231 with `vagrant rsync && vagrant provision` using this new vagrant box. It worked properly.
Thanks,
Stephen Gallagher
8 years, 5 months
Re: Review Request 231: Simplify creating target units
by Stephen Gallagher
> On Nov. 16, 2015, 3:40 p.m., Nils Philippsen wrote:
> > src/rolekit/server/rolebase.py, lines 1263-1264
> > <http://reviewboard-fedoraserver.rhcloud.com/r/231/diff/2/?file=1220#file1...>
> >
> > Why is `RoleDeploymentValues` derived from dict? The class doesn't implement any part of the dict interface, nor does any code seem to use the dict interface on any of its objects.
I was copying stuff around and forgot to drop that.
> On Nov. 16, 2015, 3:40 p.m., Nils Philippsen wrote:
> > src/rolekit/server/rolebase.py, line 1313
> > <http://reviewboard-fedoraserver.rhcloud.com/r/231/diff/2/?file=1220#file1...>
> >
> > Why not simply attempt duck-typing? Should we care if units is a real list, rather than an object that is sequence-like enough to `extend()` and existing list?
I just dropped this assertion. I think it was a leftover from an earlier version of that function.
> On Nov. 16, 2015, 3:40 p.m., Nils Philippsen wrote:
> > src/rolekit/server/rolebase.py, lines 1321-1336
> > <http://reviewboard-fedoraserver.rhcloud.com/r/231/diff/2/?file=1220#file1...>
> >
> > How about:
> >
> > ```
> > # == Create the [Unit] Section == #
> > unitfile['Unit'] = {
> > 'Description': self._desc,
> > 'Requires': tuple(self._requirements),
> > 'BindsTo': tuple(self._requirements),
> > # All roles are also assumed to require the network, so
> > # we will start it after network.target
> > 'After': "network.target",
> > }
> >
> > # == Create the [Install] section == #
> > unitfile['Install'] = {
> > 'RequiredBy': tuple(self._requirements),
> > # All roles are enabled for the multi-user.target
> > 'WantedBy': "multi-user.target",
> > }
> > ```
Honestly, I find that considerably less readable than my version.
- Stephen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/231/#review556
-----------------------------------------------------------
On Nov. 20, 2015, 8:51 p.m., Stephen Gallagher wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard-fedoraserver.rhcloud.com/r/231/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2015, 8:51 p.m.)
>
>
> Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen Gallagher, and Thomas Woerner.
>
>
> Repository: rolekit
>
>
> Description
> -------
>
> We no longer require the failure unit or the extension units. Now
> that we are getting the state from systemd directly, we will simply
> set up the Requires/RequiredBy/BindsTo settings appropriately so
> that rolekit and systemd are always kept in sync.
>
>
> Diffs
> -----
>
> config/roles/databaseserver/role.py 1af90551aff0f681ee8c2f787780e76f24330927
> config/roles/domaincontroller/role.py 9fefd60aed47888d2e7884a4b70a6ad76dd104d8
> config/roles/memcache/role.py d12974128a75ef38666bbdd7d70b7623631e3166
> config/roles/testrole/role.py b5fda8dc92b42735a7b0ce5c6b21fed7cf6fd015
> src/rolekit/server/io/systemd.py b105483c7fa689a43a960ef78aa1105f7aa58b42
> src/rolekit/server/rolebase.py 510190a5fbd66858417e9f75f73ded63e6236b85
>
> Diff: http://reviewboard-fedoraserver.rhcloud.com/r/231/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Stephen Gallagher
>
>
8 years, 5 months
Re: Review Request 230: Get role state from systemd
by Stephen Gallagher
> On Nov. 16, 2015, 3:19 p.m., Nils Philippsen wrote:
> > src/rolekit/server/rolebase.py, lines 130-132
> > <http://reviewboard-fedoraserver.rhcloud.com/r/230/diff/2/?file=1209#file1...>
> >
> > I'm not sure I'm too happy with overloading the state this way. Just from looking at the code, I think it should work but I still think it would be cleaner if we kept the state as `SYSTEMD` so it's explicit that one needs to consult systemd (whether by listening to signals or polling) to find out the state of the target. AIUI, this state gets saved to the JSON file in `/etc/rolekit/roles`, and I'd like it better if none of "must consult systemd" states are ever persisted there.
OK, so it turns out that there was actually only one case where it would get written out to disk: in the deferred role processing. I've modified that code so that it will always explicitly set the state to NASCENT before writing out the deferred settings JSON. I then modified rolesettings.py to only set the SYSTEMD state if something other than NASCENT or ERROR is present in the settings data.
I've examined the rest of the code and with the exception of deferred processing, settings.write() is never called anywhere before change_state(), so SYSTEMD should never appear in the file.
> On Nov. 16, 2015, 3:19 p.m., Nils Philippsen wrote:
> > src/rolekit/server/io/systemd.py, lines 63-69
> > <http://reviewboard-fedoraserver.rhcloud.com/r/230/diff/2/?file=1208#file1...>
> >
> > That's pretty minimalistic. Are you sure you don't want to put this in a later commit that adds a little code? ;)
This was leftover from a first implementation that I ultimately scrapped but forgot to remove. New version of the patch corrects this.
> On Nov. 16, 2015, 3:19 p.m., Nils Philippsen wrote:
> > src/rolekit/server/io/rolesettings.py, line 30
> > <http://reviewboard-fedoraserver.rhcloud.com/r/230/diff/2/?file=1207#file1...>
> >
> > This bogus import comes from review #226. Please remove it in that commit.
Done.
- Stephen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/230/#review555
-----------------------------------------------------------
On Nov. 12, 2015, 9:21 p.m., Stephen Gallagher wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard-fedoraserver.rhcloud.com/r/230/
> -----------------------------------------------------------
>
> (Updated Nov. 12, 2015, 9:21 p.m.)
>
>
> Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen Gallagher, and Thomas Woerner.
>
>
> Repository: rolekit
>
>
> Description
> -------
>
> Get role state from systemd
>
>
> Diffs
> -----
>
> src/rolekit/config/__init__.py.in 05d9f7b2bda59cf0b4ed6e1f277880ff2092b3c7
> src/rolekit/dbus_utils.py a5f5a1d5ee4f32865fca6459615ea3359f3ca11d
> src/rolekit/server/io/rolesettings.py c2424f1b166eb9eec5c59e3b79047cb482ca9081
> src/rolekit/server/io/systemd.py b105483c7fa689a43a960ef78aa1105f7aa58b42
> src/rolekit/server/rolebase.py 510190a5fbd66858417e9f75f73ded63e6236b85
>
> Diff: http://reviewboard-fedoraserver.rhcloud.com/r/230/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Stephen Gallagher
>
>
8 years, 5 months
Re: Review Request 229: Add helper to convert systemd unit state
by Stephen Gallagher
> On Nov. 16, 2015, 2:34 p.m., Nils Philippsen wrote:
> > src/rolekit/dbus_utils.py, lines 296-308
> > <http://reviewboard-fedoraserver.rhcloud.com/r/229/diff/1/?file=1199#file1...>
> >
> > How about:
> >
> > ```
> > ...
> > sd2rk_state = {
> > 'active': RUNNING,
> > 'inactive': READY_TO_START,
> > 'activating': STARTING,
> > 'deactivating': STOPPING,
> > }
> > ...
> > def map_systemd_state(systemd_state):
> > # If the state is "failed" or anything else not explicitly called
> > # out here, treat it as being in ERROR state.
> > return self.sd2rk_state.get(systemd_state, ERROR)
> > ...
> > ```
Sure, that was my C background talking. I'll switch to the pythonic way.
- Stephen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/229/#review554
-----------------------------------------------------------
On Nov. 10, 2015, 11:25 p.m., Stephen Gallagher wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard-fedoraserver.rhcloud.com/r/229/
> -----------------------------------------------------------
>
> (Updated Nov. 10, 2015, 11:25 p.m.)
>
>
> Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen Gallagher, and Thomas Woerner.
>
>
> Repository: rolekit
>
>
> Description
> -------
>
> We want to translate ActiveState values to our persistent and
> transitional states.
>
>
> Diffs
> -----
>
> src/rolekit/dbus_utils.py a5f5a1d5ee4f32865fca6459615ea3359f3ca11d
>
> Diff: http://reviewboard-fedoraserver.rhcloud.com/r/229/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Stephen Gallagher
>
>
8 years, 5 months