rolekit-commits

rolekit-commits@lists.fedorahosted.org

266 discussions

Start a nNew thread

Review Request 226: Add helper to retrieve the target unit name
by Stephen Gallagher 25 Nov '15

25 Nov '15

1 3

Review Request 227: Don't attempt to force the role to a state
by Stephen Gallagher 25 Nov '15

25 Nov '15

----------------------------------------------------------- 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

1 1

Review Request 228: Remove failure-notification signal code
by Stephen Gallagher 25 Nov '15

25 Nov '15

1 1

Review Request 229: Add helper to convert systemd unit state
by Stephen Gallagher 25 Nov '15

25 Nov '15

1 3

Review Request 230: Get role state from systemd
by Stephen Gallagher 25 Nov '15

25 Nov '15

----------------------------------------------------------- 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

1 5

Review Request 231: Simplify creating target units
by Stephen Gallagher 25 Nov '15

25 Nov '15

----------------------------------------------------------- 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

1 6

Review Request 232: Update Vagrantfile for Fedora 23
by Stephen Gallagher 25 Nov '15

25 Nov '15

----------------------------------------------------------- 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

1 4

Re: Review Request 231: Simplify creating target units
by Stephen Gallagher 20 Nov '15

20 Nov '15

> 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#file122…> > > > > 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#file122…> > > > > 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#file122…> > > > > 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 > >

1 0

Re: Review Request 230: Get role state from systemd
by Stephen Gallagher 20 Nov '15

20 Nov '15

> 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#file120…> > > > > 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#file120…> > > > > 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#file120…> > > > > 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 > >

1 0

Re: Review Request 229: Add helper to convert systemd unit state
by Stephen Gallagher 20 Nov '15

20 Nov '15

> 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#file119…> > > > > 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 > >

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

rolekit-commits