Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
On 13/03/15 11:55, Petr Spacek wrote: On 13.3.2015 11:34, Jan Cholasta wrote: Dne 13.3.2015 v 11:17 Martin Kosek napsal(a): On 03/13/2015 11:00 AM, Petr Spacek wrote: On 13.3.2015 10:42, Alexander Bokovoy wrote: On Fri, 13 Mar 2015, Petr Spacek wrote: On 13.3.2015 10:18, Martin Kosek wrote: On 03/12/2015 05:10 PM, Rob Crittenden wrote: Petr Spacek wrote: On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) No. There is no rush for this, at least not for the promise of a future fix that will never come. I checked the code and to me, the proper fix looks like instrumenting ldap.update_entry calls in upgrade plugins with if options.test: log message else do the update right? I see just couple places that would need to be updated: $ git grep -E (ldap|conn).update_entry ipaserver/install/plugins ipaserver/install/plugins/dns.py: ldap.update_entry(container_entry) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(base_entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(entry) ipaserver/install/plugins/update_pacs.py: ldap.update_entry(entry) ipaserver/install/plugins/update_referint.py: ldap.update_entry(entry) ipaserver/install/plugins/update_services.py: ldap.update_entry(entry) ipaserver/install/plugins/update_uniqueness.py: ldap.update_entry(uid_uniqueness_plugin) So from my POV, very quick fix. In that case, I would also prefer a fix now than a ticket that would never be done. I really dislike this approach because I consider it flawed by design. Plugin author has to think about it all the time and do not forget to add if otherwise ... too bad. I can see two 'safer' ways to do that: - LDAP transactions :-) - 'mock_writes=True' option in LDAP backend which would print modlists instead of applying them (and return success to the caller). Both cases eliminate the need to scatter 'ifs' all over update plugins and do not add risk of forgetting about one of them when adding/changing plugin code. I like idea about mock_writes=True. However, I think we still need to make sure plugin writers rely on options.test value to see that we aren't going to write the data. The reason for it is that we might get into configurations where plugins would be doing updates based on earlier performed tasks. If task configuration is not going to be written, its status will never be correct and plugin would get an error. That is exactly why I mentioned LDAP transactions. There is no other way how to test complex plugins which actually read own writes (except mocking the whole LDAP interface somewhere). While this may be a good idea long term, I do not think any of us is considering implementing the LDAP transaction support within work on this refactoring. So in this thread, let us focus on how to fix options.test mid-term. I currently see 2 proposed ways: - Making the plugins aware of options.test - Make ldap2 write operations only print the update and not do it. Although thinking of this approach, I think it may make some plugins like DNS loop forever. IIRC, at least DNS upgrade plugin have loop - search for all unfixed DNS zones - fix them with ldap update - was the search truncated, i.e. are there more zones to update? if yes, go back to start - Make the plugins not call {add,update,delete}_entry themselves but rather return the updates like they should. This is what the ticket (https://fedorahosted.org/freeipa/ticket/3448) requests and what should be done to make --test work for them. How do you propose to handle iterative updates like the DNS upgrade mentioned by Martin^1? Return set of updates along with boolean 'call me again'? Something else? So instead of DNS commands logic, which can be used in plugin, we should reimplement the dnzone commands in upgrade plugin, to get modlist? And keep watching it and do same
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
On 03/12/2015 05:10 PM, Rob Crittenden wrote: Petr Spacek wrote: On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) No. There is no rush for this, at least not for the promise of a future fix that will never come. I checked the code and to me, the proper fix looks like instrumenting ldap.update_entry calls in upgrade plugins with if options.test: log message else do the update right? I see just couple places that would need to be updated: $ git grep -E (ldap|conn).update_entry ipaserver/install/plugins ipaserver/install/plugins/dns.py:ldap.update_entry(container_entry) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(base_entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(entry) ipaserver/install/plugins/update_pacs.py:ldap.update_entry(entry) ipaserver/install/plugins/update_referint.py: ldap.update_entry(entry) ipaserver/install/plugins/update_services.py: ldap.update_entry(entry) ipaserver/install/plugins/update_uniqueness.py: ldap.update_entry(uid_uniqueness_plugin) So from my POV, very quick fix. In that case, I would also prefer a fix now than a ticket that would never be done. Martin -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
On 13.3.2015 10:18, Martin Kosek wrote: On 03/12/2015 05:10 PM, Rob Crittenden wrote: Petr Spacek wrote: On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) No. There is no rush for this, at least not for the promise of a future fix that will never come. I checked the code and to me, the proper fix looks like instrumenting ldap.update_entry calls in upgrade plugins with if options.test: log message else do the update right? I see just couple places that would need to be updated: $ git grep -E (ldap|conn).update_entry ipaserver/install/plugins ipaserver/install/plugins/dns.py:ldap.update_entry(container_entry) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(base_entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(entry) ipaserver/install/plugins/update_pacs.py:ldap.update_entry(entry) ipaserver/install/plugins/update_referint.py: ldap.update_entry(entry) ipaserver/install/plugins/update_services.py: ldap.update_entry(entry) ipaserver/install/plugins/update_uniqueness.py: ldap.update_entry(uid_uniqueness_plugin) So from my POV, very quick fix. In that case, I would also prefer a fix now than a ticket that would never be done. I really dislike this approach because I consider it flawed by design. Plugin author has to think about it all the time and do not forget to add if otherwise ... too bad. I can see two 'safer' ways to do that: - LDAP transactions :-) - 'mock_writes=True' option in LDAP backend which would print modlists instead of applying them (and return success to the caller). Both cases eliminate the need to scatter 'ifs' all over update plugins and do not add risk of forgetting about one of them when adding/changing plugin code. -- Petr^2 Spacek -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
On Fri, 13 Mar 2015, Petr Spacek wrote: On 13.3.2015 10:18, Martin Kosek wrote: On 03/12/2015 05:10 PM, Rob Crittenden wrote: Petr Spacek wrote: On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) No. There is no rush for this, at least not for the promise of a future fix that will never come. I checked the code and to me, the proper fix looks like instrumenting ldap.update_entry calls in upgrade plugins with if options.test: log message else do the update right? I see just couple places that would need to be updated: $ git grep -E (ldap|conn).update_entry ipaserver/install/plugins ipaserver/install/plugins/dns.py:ldap.update_entry(container_entry) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(base_entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(entry) ipaserver/install/plugins/update_pacs.py:ldap.update_entry(entry) ipaserver/install/plugins/update_referint.py: ldap.update_entry(entry) ipaserver/install/plugins/update_services.py: ldap.update_entry(entry) ipaserver/install/plugins/update_uniqueness.py: ldap.update_entry(uid_uniqueness_plugin) So from my POV, very quick fix. In that case, I would also prefer a fix now than a ticket that would never be done. I really dislike this approach because I consider it flawed by design. Plugin author has to think about it all the time and do not forget to add if otherwise ... too bad. I can see two 'safer' ways to do that: - LDAP transactions :-) - 'mock_writes=True' option in LDAP backend which would print modlists instead of applying them (and return success to the caller). Both cases eliminate the need to scatter 'ifs' all over update plugins and do not add risk of forgetting about one of them when adding/changing plugin code. I like idea about mock_writes=True. However, I think we still need to make sure plugin writers rely on options.test value to see that we aren't going to write the data. The reason for it is that we might get into configurations where plugins would be doing updates based on earlier performed tasks. If task configuration is not going to be written, its status will never be correct and plugin would get an error. -- / Alexander Bokovoy -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
On 13.3.2015 10:42, Alexander Bokovoy wrote: On Fri, 13 Mar 2015, Petr Spacek wrote: On 13.3.2015 10:18, Martin Kosek wrote: On 03/12/2015 05:10 PM, Rob Crittenden wrote: Petr Spacek wrote: On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) No. There is no rush for this, at least not for the promise of a future fix that will never come. I checked the code and to me, the proper fix looks like instrumenting ldap.update_entry calls in upgrade plugins with if options.test: log message else do the update right? I see just couple places that would need to be updated: $ git grep -E (ldap|conn).update_entry ipaserver/install/plugins ipaserver/install/plugins/dns.py:ldap.update_entry(container_entry) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(base_entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(entry) ipaserver/install/plugins/update_pacs.py: ldap.update_entry(entry) ipaserver/install/plugins/update_referint.py: ldap.update_entry(entry) ipaserver/install/plugins/update_services.py: ldap.update_entry(entry) ipaserver/install/plugins/update_uniqueness.py: ldap.update_entry(uid_uniqueness_plugin) So from my POV, very quick fix. In that case, I would also prefer a fix now than a ticket that would never be done. I really dislike this approach because I consider it flawed by design. Plugin author has to think about it all the time and do not forget to add if otherwise ... too bad. I can see two 'safer' ways to do that: - LDAP transactions :-) - 'mock_writes=True' option in LDAP backend which would print modlists instead of applying them (and return success to the caller). Both cases eliminate the need to scatter 'ifs' all over update plugins and do not add risk of forgetting about one of them when adding/changing plugin code. I like idea about mock_writes=True. However, I think we still need to make sure plugin writers rely on options.test value to see that we aren't going to write the data. The reason for it is that we might get into configurations where plugins would be doing updates based on earlier performed tasks. If task configuration is not going to be written, its status will never be correct and plugin would get an error. That is exactly why I mentioned LDAP transactions. There is no other way how to test complex plugins which actually read own writes (except mocking the whole LDAP interface somewhere). -- Petr^2 Spacek -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
On 03/13/2015 11:00 AM, Petr Spacek wrote: On 13.3.2015 10:42, Alexander Bokovoy wrote: On Fri, 13 Mar 2015, Petr Spacek wrote: On 13.3.2015 10:18, Martin Kosek wrote: On 03/12/2015 05:10 PM, Rob Crittenden wrote: Petr Spacek wrote: On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) No. There is no rush for this, at least not for the promise of a future fix that will never come. I checked the code and to me, the proper fix looks like instrumenting ldap.update_entry calls in upgrade plugins with if options.test: log message else do the update right? I see just couple places that would need to be updated: $ git grep -E (ldap|conn).update_entry ipaserver/install/plugins ipaserver/install/plugins/dns.py:ldap.update_entry(container_entry) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(base_entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(entry) ipaserver/install/plugins/update_pacs.py:ldap.update_entry(entry) ipaserver/install/plugins/update_referint.py: ldap.update_entry(entry) ipaserver/install/plugins/update_services.py: ldap.update_entry(entry) ipaserver/install/plugins/update_uniqueness.py: ldap.update_entry(uid_uniqueness_plugin) So from my POV, very quick fix. In that case, I would also prefer a fix now than a ticket that would never be done. I really dislike this approach because I consider it flawed by design. Plugin author has to think about it all the time and do not forget to add if otherwise ... too bad. I can see two 'safer' ways to do that: - LDAP transactions :-) - 'mock_writes=True' option in LDAP backend which would print modlists instead of applying them (and return success to the caller). Both cases eliminate the need to scatter 'ifs' all over update plugins and do not add risk of forgetting about one of them when adding/changing plugin code. I like idea about mock_writes=True. However, I think we still need to make sure plugin writers rely on options.test value to see that we aren't going to write the data. The reason for it is that we might get into configurations where plugins would be doing updates based on earlier performed tasks. If task configuration is not going to be written, its status will never be correct and plugin would get an error. That is exactly why I mentioned LDAP transactions. There is no other way how to test complex plugins which actually read own writes (except mocking the whole LDAP interface somewhere). While this may be a good idea long term, I do not think any of us is considering implementing the LDAP transaction support within work on this refactoring. So in this thread, let us focus on how to fix options.test mid-term. I currently see 2 proposed ways: - Making the plugins aware of options.test - Make ldap2 write operations only print the update and not do it. Although thinking of this approach, I think it may make some plugins like DNS loop forever. IIRC, at least DNS upgrade plugin have loop - search for all unfixed DNS zones - fix them with ldap update - was the search truncated, i.e. are there more zones to update? if yes, go back to start -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
On 13.3.2015 11:34, Jan Cholasta wrote: Dne 13.3.2015 v 11:17 Martin Kosek napsal(a): On 03/13/2015 11:00 AM, Petr Spacek wrote: On 13.3.2015 10:42, Alexander Bokovoy wrote: On Fri, 13 Mar 2015, Petr Spacek wrote: On 13.3.2015 10:18, Martin Kosek wrote: On 03/12/2015 05:10 PM, Rob Crittenden wrote: Petr Spacek wrote: On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) No. There is no rush for this, at least not for the promise of a future fix that will never come. I checked the code and to me, the proper fix looks like instrumenting ldap.update_entry calls in upgrade plugins with if options.test: log message else do the update right? I see just couple places that would need to be updated: $ git grep -E (ldap|conn).update_entry ipaserver/install/plugins ipaserver/install/plugins/dns.py: ldap.update_entry(container_entry) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(base_entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(entry) ipaserver/install/plugins/update_pacs.py: ldap.update_entry(entry) ipaserver/install/plugins/update_referint.py: ldap.update_entry(entry) ipaserver/install/plugins/update_services.py: ldap.update_entry(entry) ipaserver/install/plugins/update_uniqueness.py: ldap.update_entry(uid_uniqueness_plugin) So from my POV, very quick fix. In that case, I would also prefer a fix now than a ticket that would never be done. I really dislike this approach because I consider it flawed by design. Plugin author has to think about it all the time and do not forget to add if otherwise ... too bad. I can see two 'safer' ways to do that: - LDAP transactions :-) - 'mock_writes=True' option in LDAP backend which would print modlists instead of applying them (and return success to the caller). Both cases eliminate the need to scatter 'ifs' all over update plugins and do not add risk of forgetting about one of them when adding/changing plugin code. I like idea about mock_writes=True. However, I think we still need to make sure plugin writers rely on options.test value to see that we aren't going to write the data. The reason for it is that we might get into configurations where plugins would be doing updates based on earlier performed tasks. If task configuration is not going to be written, its status will never be correct and plugin would get an error. That is exactly why I mentioned LDAP transactions. There is no other way how to test complex plugins which actually read own writes (except mocking the whole LDAP interface somewhere). While this may be a good idea long term, I do not think any of us is considering implementing the LDAP transaction support within work on this refactoring. So in this thread, let us focus on how to fix options.test mid-term. I currently see 2 proposed ways: - Making the plugins aware of options.test - Make ldap2 write operations only print the update and not do it. Although thinking of this approach, I think it may make some plugins like DNS loop forever. IIRC, at least DNS upgrade plugin have loop - search for all unfixed DNS zones - fix them with ldap update - was the search truncated, i.e. are there more zones to update? if yes, go back to start - Make the plugins not call {add,update,delete}_entry themselves but rather return the updates like they should. This is what the ticket (https://fedorahosted.org/freeipa/ticket/3448) requests and what should be done to make --test work for them. How do you propose to handle iterative updates like the DNS upgrade mentioned by Martin^1? Return set of updates along with boolean 'call me again'? Something else? -- Petr^2 Spacek -- Manage your subscription for the Freeipa-devel mailing list:
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
Dne 13.3.2015 v 11:17 Martin Kosek napsal(a): On 03/13/2015 11:00 AM, Petr Spacek wrote: On 13.3.2015 10:42, Alexander Bokovoy wrote: On Fri, 13 Mar 2015, Petr Spacek wrote: On 13.3.2015 10:18, Martin Kosek wrote: On 03/12/2015 05:10 PM, Rob Crittenden wrote: Petr Spacek wrote: On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) No. There is no rush for this, at least not for the promise of a future fix that will never come. I checked the code and to me, the proper fix looks like instrumenting ldap.update_entry calls in upgrade plugins with if options.test: log message else do the update right? I see just couple places that would need to be updated: $ git grep -E (ldap|conn).update_entry ipaserver/install/plugins ipaserver/install/plugins/dns.py: ldap.update_entry(container_entry) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(base_entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(entry) ipaserver/install/plugins/update_pacs.py: ldap.update_entry(entry) ipaserver/install/plugins/update_referint.py: ldap.update_entry(entry) ipaserver/install/plugins/update_services.py: ldap.update_entry(entry) ipaserver/install/plugins/update_uniqueness.py: ldap.update_entry(uid_uniqueness_plugin) So from my POV, very quick fix. In that case, I would also prefer a fix now than a ticket that would never be done. I really dislike this approach because I consider it flawed by design. Plugin author has to think about it all the time and do not forget to add if otherwise ... too bad. I can see two 'safer' ways to do that: - LDAP transactions :-) - 'mock_writes=True' option in LDAP backend which would print modlists instead of applying them (and return success to the caller). Both cases eliminate the need to scatter 'ifs' all over update plugins and do not add risk of forgetting about one of them when adding/changing plugin code. I like idea about mock_writes=True. However, I think we still need to make sure plugin writers rely on options.test value to see that we aren't going to write the data. The reason for it is that we might get into configurations where plugins would be doing updates based on earlier performed tasks. If task configuration is not going to be written, its status will never be correct and plugin would get an error. That is exactly why I mentioned LDAP transactions. There is no other way how to test complex plugins which actually read own writes (except mocking the whole LDAP interface somewhere). While this may be a good idea long term, I do not think any of us is considering implementing the LDAP transaction support within work on this refactoring. So in this thread, let us focus on how to fix options.test mid-term. I currently see 2 proposed ways: - Making the plugins aware of options.test - Make ldap2 write operations only print the update and not do it. Although thinking of this approach, I think it may make some plugins like DNS loop forever. IIRC, at least DNS upgrade plugin have loop - search for all unfixed DNS zones - fix them with ldap update - was the search truncated, i.e. are there more zones to update? if yes, go back to start - Make the plugins not call {add,update,delete}_entry themselves but rather return the updates like they should. This is what the ticket (https://fedorahosted.org/freeipa/ticket/3448) requests and what should be done to make --test work for them. -- Jan Cholasta -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
On 13/03/15 12:50, Jan Cholasta wrote: Dne 13.3.2015 v 12:08 Petr Spacek napsal(a): On 13.3.2015 12:01, Martin Basti wrote: On 13/03/15 11:55, Petr Spacek wrote: On 13.3.2015 11:34, Jan Cholasta wrote: Dne 13.3.2015 v 11:17 Martin Kosek napsal(a): On 03/13/2015 11:00 AM, Petr Spacek wrote: On 13.3.2015 10:42, Alexander Bokovoy wrote: On Fri, 13 Mar 2015, Petr Spacek wrote: On 13.3.2015 10:18, Martin Kosek wrote: On 03/12/2015 05:10 PM, Rob Crittenden wrote: Petr Spacek wrote: On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) No. There is no rush for this, at least not for the promise of a future fix that will never come. I checked the code and to me, the proper fix looks like instrumenting ldap.update_entry calls in upgrade plugins with if options.test: log message else do the update right? I see just couple places that would need to be updated: $ git grep -E (ldap|conn).update_entry ipaserver/install/plugins ipaserver/install/plugins/dns.py: ldap.update_entry(container_entry) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(base_entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(entry) ipaserver/install/plugins/update_pacs.py: ldap.update_entry(entry) ipaserver/install/plugins/update_referint.py: ldap.update_entry(entry) ipaserver/install/plugins/update_services.py: ldap.update_entry(entry) ipaserver/install/plugins/update_uniqueness.py: ldap.update_entry(uid_uniqueness_plugin) So from my POV, very quick fix. In that case, I would also prefer a fix now than a ticket that would never be done. I really dislike this approach because I consider it flawed by design. Plugin author has to think about it all the time and do not forget to add if otherwise ... too bad. I can see two 'safer' ways to do that: - LDAP transactions :-) - 'mock_writes=True' option in LDAP backend which would print modlists instead of applying them (and return success to the caller). Both cases eliminate the need to scatter 'ifs' all over update plugins and do not add risk of forgetting about one of them when adding/changing plugin code. I like idea about mock_writes=True. However, I think we still need to make sure plugin writers rely on options.test value to see that we aren't going to write the data. The reason for it is that we might get into configurations where plugins would be doing updates based on earlier performed tasks. If task configuration is not going to be written, its status will never be correct and plugin would get an error. That is exactly why I mentioned LDAP transactions. There is no other way how to test complex plugins which actually read own writes (except mocking the whole LDAP interface somewhere). While this may be a good idea long term, I do not think any of us is considering implementing the LDAP transaction support within work on this refactoring. So in this thread, let us focus on how to fix options.test mid-term. I currently see 2 proposed ways: - Making the plugins aware of options.test - Make ldap2 write operations only print the update and not do it. Although thinking of this approach, I think it may make some plugins like DNS loop forever. IIRC, at least DNS upgrade plugin have loop - search for all unfixed DNS zones - fix them with ldap update - was the search truncated, i.e. are there more zones to update? if yes, go back to start - Make the plugins not call {add,update,delete}_entry themselves but rather return the updates like they should. This is what the ticket (https://fedorahosted.org/freeipa/ticket/3448) requests and what should be done to make --test work for them. How do you propose to handle iterative updates like the DNS upgrade mentioned by Martin^1? Return set of updates along with boolean 'call me again'? Something else? So
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
On 13/03/15 10:18, Martin Kosek wrote: On 03/12/2015 05:10 PM, Rob Crittenden wrote: Petr Spacek wrote: On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) No. There is no rush for this, at least not for the promise of a future fix that will never come. I checked the code and to me, the proper fix looks like instrumenting ldap.update_entry calls in upgrade plugins with if options.test: log message else do the update right? I see just couple places that would need to be updated: $ git grep -E (ldap|conn).update_entry ipaserver/install/plugins ipaserver/install/plugins/dns.py: ldap.update_entry(container_entry) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(base_entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(entry) ipaserver/install/plugins/update_pacs.py: ldap.update_entry(entry) ipaserver/install/plugins/update_referint.py: ldap.update_entry(entry) ipaserver/install/plugins/update_services.py: ldap.update_entry(entry) ipaserver/install/plugins/update_uniqueness.py: ldap.update_entry(uid_uniqueness_plugin) So from my POV, very quick fix. In that case, I would also prefer a fix now than a ticket that would never be done. Martin And ldap.add_entry, del_entry, ipa add/mod/del commands -- Martin Basti -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
Jan Cholasta wrote: Dne 13.3.2015 v 12:08 Petr Spacek napsal(a): On 13.3.2015 12:01, Martin Basti wrote: On 13/03/15 11:55, Petr Spacek wrote: On 13.3.2015 11:34, Jan Cholasta wrote: Dne 13.3.2015 v 11:17 Martin Kosek napsal(a): On 03/13/2015 11:00 AM, Petr Spacek wrote: On 13.3.2015 10:42, Alexander Bokovoy wrote: On Fri, 13 Mar 2015, Petr Spacek wrote: On 13.3.2015 10:18, Martin Kosek wrote: On 03/12/2015 05:10 PM, Rob Crittenden wrote: Petr Spacek wrote: On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) No. There is no rush for this, at least not for the promise of a future fix that will never come. I checked the code and to me, the proper fix looks like instrumenting ldap.update_entry calls in upgrade plugins with if options.test: log message else do the update right? I see just couple places that would need to be updated: $ git grep -E (ldap|conn).update_entry ipaserver/install/plugins ipaserver/install/plugins/dns.py: ldap.update_entry(container_entry) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/fix_replica_agreements.py: repl.conn.update_entry(replica) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_idranges.py: ldap.update_entry(entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(base_entry) ipaserver/install/plugins/update_managed_permissions.py: ldap.update_entry(entry) ipaserver/install/plugins/update_pacs.py: ldap.update_entry(entry) ipaserver/install/plugins/update_referint.py: ldap.update_entry(entry) ipaserver/install/plugins/update_services.py: ldap.update_entry(entry) ipaserver/install/plugins/update_uniqueness.py: ldap.update_entry(uid_uniqueness_plugin) So from my POV, very quick fix. In that case, I would also prefer a fix now than a ticket that would never be done. I really dislike this approach because I consider it flawed by design. Plugin author has to think about it all the time and do not forget to add if otherwise ... too bad. I can see two 'safer' ways to do that: - LDAP transactions :-) - 'mock_writes=True' option in LDAP backend which would print modlists instead of applying them (and return success to the caller). Both cases eliminate the need to scatter 'ifs' all over update plugins and do not add risk of forgetting about one of them when adding/changing plugin code. I like idea about mock_writes=True. However, I think we still need to make sure plugin writers rely on options.test value to see that we aren't going to write the data. The reason for it is that we might get into configurations where plugins would be doing updates based on earlier performed tasks. If task configuration is not going to be written, its status will never be correct and plugin would get an error. That is exactly why I mentioned LDAP transactions. There is no other way how to test complex plugins which actually read own writes (except mocking the whole LDAP interface somewhere). While this may be a good idea long term, I do not think any of us is considering implementing the LDAP transaction support within work on this refactoring. So in this thread, let us focus on how to fix options.test mid-term. I currently see 2 proposed ways: - Making the plugins aware of options.test - Make ldap2 write operations only print the update and not do it. Although thinking of this approach, I think it may make some plugins like DNS loop forever. IIRC, at least DNS upgrade plugin have loop - search for all unfixed DNS zones - fix them with ldap update - was the search truncated, i.e. are there more zones to update? if yes, go back to start - Make the plugins not call {add,update,delete}_entry themselves but rather return the updates like they should. This is what the ticket (https://fedorahosted.org/freeipa/ticket/3448) requests and what should be done to make --test work for them. How do you propose to handle iterative updates like the DNS upgrade mentioned by Martin^1? Return set of updates along
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. -- David Kupka -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
On 12.3.2015 17:05, Petr Spacek wrote: On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) BTW 'proper' fix would be to implement transactions in DS, run all updates in one huge transaction and do roll-back at the end. That would allow us to actually test updates which can depend on each other etc. -- Petr^2 Spacek -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. rob -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
Petr Spacek wrote: On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) No. There is no rush for this, at least not for the promise of a future fix that will never come. rob -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
On 12.3.2015 16:23, Rob Crittenden wrote: David Kupka wrote: On 03/06/2015 06:00 PM, Martin Basti wrote: Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. Ideally we want to fix all plugins to dry-run the upgrade not just skip when there is '--test' option but it is a good first step. Works for me, ACK. I agree that this breaks the spirit of --test and think it should be fixed before committing. Considering how often is the option is used ... I do not think that this requires 'proper' fix now. It was broken for *years* so this patch is a huge improvement and IMHO should be commited in current form. We can re-visit it later on, open a ticket :-) -- Petr^2 Spacek -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [PATCH 0208] Respect --test option in upgrade plugins
Upgrade plugins which modify LDAP data directly should not be executed in --test mode. This patch is a workaround, to ensure update with --test option will not modify any LDAP data. https://fedorahosted.org/freeipa/ticket/3448 Patch attached. -- Martin Basti From 0616696080c25531d9dd5c30d7e32e0a7da9ed6c Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Fri, 6 Mar 2015 17:44:47 +0100 Subject: [PATCH] Server Upgrade: respect --test option in plugins Several plugins do the LDAP data modification directly. In test mode these plugis should not be executed. https://fedorahosted.org/freeipa/ticket/3448 --- ipaserver/install/plugins/dns.py| 9 + ipaserver/install/plugins/fix_replica_agreements.py | 4 ipaserver/install/plugins/update_idranges.py| 7 +++ ipaserver/install/plugins/update_managed_permissions.py | 4 ipaserver/install/plugins/update_pacs.py| 4 ipaserver/install/plugins/update_referint.py| 3 +++ ipaserver/install/plugins/update_services.py| 4 7 files changed, 35 insertions(+) diff --git a/ipaserver/install/plugins/dns.py b/ipaserver/install/plugins/dns.py index f562978bcbcc02321c0e9a668af88b4f596f8556..2e33982e7d53cad794cca5867aa94a0df766eff8 100644 --- a/ipaserver/install/plugins/dns.py +++ b/ipaserver/install/plugins/dns.py @@ -60,6 +60,10 @@ class update_dnszones(PostUpdate): order=MIDDLE def execute(self, **options): +if not options.get('live_run'): +self.log.info(Test mode: skipping 'update_dnszones') +return False, False, () + ldap = self.obj.backend if not dns_container_exists(ldap): return (False, False, []) @@ -159,6 +163,11 @@ class update_master_to_dnsforwardzones(PostUpdate): backup_path = u'%s%s' % (backup_dir, backup_filename) def execute(self, **options): +if not options.get('live_run'): +self.log.info(Test mode: skipping + 'update_master_to_dnsforwardzones') +return False, False, () + ldap = self.obj.backend # check LDAP if forwardzones already uses new semantics dns_container_dn = DN(api.env.container_dns, api.env.basedn) diff --git a/ipaserver/install/plugins/fix_replica_agreements.py b/ipaserver/install/plugins/fix_replica_agreements.py index a5ff4819fa4c432b378a9f1c0f6952bc312a6792..597a133145e1cad2265386bf6eb55d47fefa86e3 100644 --- a/ipaserver/install/plugins/fix_replica_agreements.py +++ b/ipaserver/install/plugins/fix_replica_agreements.py @@ -38,6 +38,10 @@ class update_replica_attribute_lists(PreUpdate): order = MIDDLE def execute(self, **options): +if not options.get('live_run'): +self.log.info(Test mode: skipping + 'update_replica_attribute_lists') +return False, False, () # We need an IPAdmin connection to the backend self.log.debug(Start replication agreement exclude list update task) conn = ipaldap.IPAdmin(api.env.host, ldapi=True, realm=api.env.realm) diff --git a/ipaserver/install/plugins/update_idranges.py b/ipaserver/install/plugins/update_idranges.py index 1aa5fa7631fd35a7aaf4a23a5eee44e4e0a2e904..cc462ef1265e3bee2137df1c787d93b048981e25 100644 --- a/ipaserver/install/plugins/update_idranges.py +++ b/ipaserver/install/plugins/update_idranges.py @@ -33,6 +33,9 @@ class update_idrange_type(PostUpdate): order = MIDDLE def execute(self, **options): +if not options.get('live_run'): +self.log.info(Test mode: skipping 'update_idrange_type') +return False, False, () ldap = self.obj.backend base_dn = DN(api.env.container_ranges, api.env.basedn) @@ -121,6 +124,10 @@ class update_idrange_baserid(PostUpdate): order = LAST def execute(self, **options): +if not options.get('live_run'): +self.log.info(Test mode: skipping 'update_idrange_baserid') +return False, False, () + ldap = self.obj.backend base_dn = DN(api.env.container_ranges, api.env.basedn) diff --git a/ipaserver/install/plugins/update_managed_permissions.py b/ipaserver/install/plugins/update_managed_permissions.py index 430a2919a315bfd8d8e6174a915890d44b782c5c..55e1068540d99ad189aa85682ba9ef1e96293abb 100644 --- a/ipaserver/install/plugins/update_managed_permissions.py +++ b/ipaserver/install/plugins/update_managed_permissions.py @@ -402,6 +402,10 @@ class update_managed_permissions(PostUpdate): def execute(self, **options): +if not options.get('live_run'): +self.log.info(Test mode: skipping 'update_managed_permissions') +return False, False, () + ldap = self.api.Backend[ldap2] anonymous_read_aci = self.get_anonymous_read_aci(ldap) diff --git a/ipaserver/install/plugins/update_pacs.py b/ipaserver/install/plugins/update_pacs.py