[PATCH] Improve replication test speed by using immediate events

Andrew Bartlett abartlet at samba.org
Fri Jul 21 06:22:09 UTC 2017


With this patch, and the full set of context just for interest, now
attached.  

On Fri, 2017-07-21 at 18:13 +1200, Andrew Bartlett via samba-technical
wrote:
> I was looking as why my nice, lovingly engineered patches to speed up
> the repl_move test, by avoiding fork() and the string2key cost (which
> showed up under perf) didn't really help!
> 
> So I looked at what the server does on a DsReplicaSync, and noticed
> that not only does it wait 1 second just for the heck of it, it will
> waste time and events trying to trigger outbound notifications, rather
> than getting on with pulling in changes. 
> 
> This is what has been making our replication tests so slow, and why the
> tests ran so much faster against Windows!
> 
> It is just a one-run sample but with this patch, on my laptop, also
> including my ccache patches:
> 
> TOP 10 slowest tests
> samba4.drs.repl_move.python(promoted_dc)(promoted_dc) -> 159
> samba4.drs.ridalloc_exop.python(vampire_dc)(vampire_dc) -> 96
> samba4.drs.repl_move.python(vampire_dc)(vampire_dc) -> 75
> samba4.nbt.winsreplication(ad_dc_ntvfs) -> 70
> samba4.drs.samba_tool_drs.python(promoted_dc)(promoted_dc:local) -> 59
> samba4.drs.samba_tool_drs.python(vampire_dc)(vampire_dc:local) -> 58
> samba4.drs.repl_schema.python(vampire_dc)(vampire_dc) -> 49
> samba4.drs.repl_schema.python(vampire_2000_dc)(vampire_2000_dc) -> 47
> samba4.krb5.kdc with account ALLOWED permission to replicate to an
> RODC(rodc) -> 43
> samba4.krb5.kdc with account DENIED permission to replicate to an
> RODC(rodc) -> 42
> 
> compared with just the ccache patches:
> 
> samba4.drs.repl_move.python(promoted_dc)(promoted_dc) -> 453
> samba4.drs.repl_move.python(vampire_dc)(vampire_dc) -> 373
> samba4.drs.repl_schema.python(vampire_dc)(vampire_dc) -> 188
> samba4.drs.repl_schema.python(promoted_dc)(promoted_dc) -> 151
> samba4.drs.ridalloc_exop.python(vampire_dc)(vampire_dc) -> 103
> samba4.drs.replica_sync.python(promoted_dc)(promoted_dc:local) -> 92
> samba4.drs.repl_schema.python(vampire_2000_dc)(vampire_2000_dc) -> 87
> samba4.drs.replica_sync.python(vampire_dc)(vampire_dc:local) -> 81
> samba4.drs.samba_tool_drs.python(vampire_dc)(vampire_dc:local) -> 81
> samba4.drs.samba_tool_drs.python(promoted_dc)(promoted_dc:local) -> 80
> 
> Overall time for make test TESTS="repl drs" dropped from 36min to 19min. 
> 
> I'll run a private autobuild with these, but in the meantime, please
> carefully review!
> 
> The whole set is at:
> 
> https://git.samba.org/?p=abartlet/samba.git/.git;a=shortlog;h=refs/heads/faster-drs-krb5-ccache-p-im
> 
> Thanks,
> 
> Andrew Bartlett
> 
-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba
-------------- next part --------------
From e9a20f845a53e9b10a64c96549b61850acc70218 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 21 Jul 2017 17:52:04 +1200
Subject: [PATCH] s4-drepl: Use tevent_schedule_immediate() in DsReplicaSync
 handler

When we are sent a DsReplicaSync() we should work on inbound replication
(ideally from the requested source, but so far we just start the whole queue)
right away, not after 1 second.

We should also target inbound replication, not any outbound replication
notification that may happen to be due.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dsdb/repl/drepl_periodic.c | 66 ++++++++++----------------------------
 source4/dsdb/repl/drepl_service.c  |  2 +-
 source4/dsdb/repl/drepl_service.h  | 18 +++--------
 3 files changed, 23 insertions(+), 63 deletions(-)

diff --git a/source4/dsdb/repl/drepl_periodic.c b/source4/dsdb/repl/drepl_periodic.c
index d6b9467..04dbc01 100644
--- a/source4/dsdb/repl/drepl_periodic.c
+++ b/source4/dsdb/repl/drepl_periodic.c
@@ -134,63 +134,31 @@ void dreplsrv_run_pending_ops(struct dreplsrv_service *s)
 	}
 }
 
-static void dreplsrv_pending_run(struct dreplsrv_service *service);
-
-static void dreplsrv_pending_handler_te(struct tevent_context *ev, struct tevent_timer *te,
-					struct timeval t, void *ptr)
+static void dreplsrv_pending_pull_handler_im(struct tevent_context *ev,
+					     struct tevent_immediate *im,
+					     void *ptr)
 {
 	struct dreplsrv_service *service = talloc_get_type(ptr, struct dreplsrv_service);
 
-	service->pending.te = NULL;
+	TALLOC_FREE(service->pending.im);
 
-	dreplsrv_pending_run(service);
+	dreplsrv_run_pull_ops(service);
 }
 
-WERROR dreplsrv_pendingops_schedule(struct dreplsrv_service *service, uint32_t next_interval)
+WERROR dreplsrv_pendingops_schedule_pull_now(struct dreplsrv_service *service)
 {
-	TALLOC_CTX *tmp_mem;
-	struct tevent_timer *new_te;
-	struct timeval next_time;
-
-	/* prevent looping */
-	if (next_interval == 0) {
-		next_interval = 1;
-	}
-
-	next_time = timeval_current_ofs(next_interval, 50);
-
-	if (service->pending.te) {
-		/*
-		 * if the timestamp of the new event is higher,
-		 * as current next we don't need to reschedule
-		 */
-		if (timeval_compare(&next_time, &service->pending.next_event) > 0) {
-			return WERR_OK;
-		}
-	}
-
-	/* reset the next scheduled timestamp */
-	service->pending.next_event = next_time;
-
-	new_te = tevent_add_timer(service->task->event_ctx, service,
-				  service->pending.next_event,
-			         dreplsrv_pending_handler_te, service);
-	W_ERROR_HAVE_NO_MEMORY(new_te);
-
-	tmp_mem = talloc_new(service);
-	DEBUG(4,("dreplsrv_pending_schedule(%u) %sscheduled for: %s\n",
-		next_interval,
-		(service->pending.te?"re":""),
-		nt_time_string(tmp_mem, timeval_to_nttime(&next_time))));
-	talloc_free(tmp_mem);
-
-	talloc_free(service->pending.te);
-	service->pending.te = new_te;
+	struct tevent_immediate *new_im;
+	
+	new_im = tevent_create_immediate(service);
+	W_ERROR_HAVE_NO_MEMORY(new_im);
+	
+	tevent_schedule_immediate(new_im, service->task->event_ctx,
+				  dreplsrv_pending_pull_handler_im,
+				  service);
+
+	TALLOC_FREE(service->pending.im);
+	service->pending.im = new_im;
 
 	return WERR_OK;
 }
 
-static void dreplsrv_pending_run(struct dreplsrv_service *service)
-{
-	dreplsrv_run_pending_ops(service);
-}
diff --git a/source4/dsdb/repl/drepl_service.c b/source4/dsdb/repl/drepl_service.c
index 39791b4..8412f90 100644
--- a/source4/dsdb/repl/drepl_service.c
+++ b/source4/dsdb/repl/drepl_service.c
@@ -339,7 +339,7 @@ static NTSTATUS drepl_replica_sync(struct irpc_message *msg,
 	 * schedule replication event to force
 	 * replication as soon as possible
 	 */
-	dreplsrv_pendingops_schedule(service, 0);
+	dreplsrv_pendingops_schedule_pull_now(service);
 
 done:
 	return NT_STATUS_OK;
diff --git a/source4/dsdb/repl/drepl_service.h b/source4/dsdb/repl/drepl_service.h
index 6b85ad6..4b0e43f 100644
--- a/source4/dsdb/repl/drepl_service.h
+++ b/source4/dsdb/repl/drepl_service.h
@@ -187,21 +187,13 @@ struct dreplsrv_service {
 		struct tevent_timer *te;
 	} periodic;
 
-	/* some stuff for running only the pendings ops */
+	/* some stuff for running only the incoming notify ops */
 	struct {
-		/*
-		 * the interval between notify runs
-		 */
-		uint32_t interval;
-
-		/*
-		 * the timestamp for the next event,
-		 * this is the timstamp passed to event_add_timed()
+		/* 
+		 * here we have a reference to the immidicate event that was
+		 * scheduled from the DsReplicaSync
 		 */
-		struct timeval next_event;
-
-		/* here we have a reference to the timed event the schedules the notifies */
-		struct tevent_timer *te;
+		struct tevent_immediate *im;
 	} pending;
 
 	/* some stuff for notify processing */
-- 
2.9.4

-------------- next part --------------
From 25ebeb92636529bc1e3aaad9eac9f1c91804a865 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 30 Jun 2017 11:13:55 +1200
Subject: [PATCH 01/23] autobuild: Run "none" environment tests in samba-o3

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 script/autobuild.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/script/autobuild.py b/script/autobuild.py
index a4ad544..82b011d 100755
--- a/script/autobuild.py
+++ b/script/autobuild.py
@@ -76,7 +76,7 @@ tasks = {
     # We have 'test' before 'install' because, 'test' should work without 'install'
     "samba" : [ ("configure", "./configure.developer --with-selftest-prefix=./bin/ab" + samba_configure_params, "text/plain"),
                 ("make", "make -j", "text/plain"),
-                ("test", "make test FAIL_IMMEDIATELY=1", "text/plain"),
+                ("test", "make test FAIL_IMMEDIATELY=1 TESTS='--exclude-env=none'", "text/plain"),
                 ("install", "make install", "text/plain"),
                 ("check-clean-tree", "script/clean-source-tree.sh", "text/plain"),
                 ("clean", "make clean", "text/plain") ],
@@ -93,11 +93,11 @@ tasks = {
                     " --cross-answers=./bin-xe/cross-answers.txt --with-selftest-prefix=./bin-xa/ab" + samba_configure_params, "text/plain"),
                    ("compare-results", "script/compare_cc_results.py ./bin/c4che/default.cache.py ./bin-xe/c4che/default.cache.py ./bin-xa/c4che/default.cache.py", "text/plain")],
 
-    # test build with -O3 -- catches extra warnings and bugs, tests the ad_dc environments
+    # test build with -O3 -- catches extra warnings and bugs, tests the ad_dc and none environments
     "samba-o3" : [ ("random-sleep", "../script/random-sleep.sh 60 600", "text/plain"),
                    ("configure", "ADDITIONAL_CFLAGS='-O3' ./configure.developer --with-selftest-prefix=./bin/ab --abi-check-disable" + samba_configure_params, "text/plain"),
                    ("make", "make -j", "text/plain"),
-                   ("test", "make quicktest FAIL_IMMEDIATELY=1 TESTS='--include-env=ad_dc'", "text/plain"),
+                   ("test", "make quicktest FAIL_IMMEDIATELY=1 TESTS='--include-env=ad_dc --include-env=none'", "text/plain"),
                    ("install", "make install", "text/plain"),
                    ("check-clean-tree", "script/clean-source-tree.sh", "text/plain"),
                    ("clean", "make clean", "text/plain") ],
-- 
2.9.4


From 3bc32b30e08554b7c24400d65129861d45a587ae Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 30 Jun 2017 11:44:58 +1200
Subject: [PATCH 02/23] travis-ci: Run new samba-nt4 environment

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 .travis.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.travis.yml b/.travis.yml
index 4c68c72..7d5b4cd 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -14,6 +14,7 @@ env:
   - TASK=samba-o3
   - TASK=samba-nopython
   - TASK=samba-systemkrb5
+  - TASK=samba-nt4
   - TASK=ldb
   - TASK=tdb
   - TASK=talloc
-- 
2.9.4


From 7744ffe550c6c8807142e940fe4694700705ff68 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 30 Jun 2017 11:15:40 +1200
Subject: [PATCH 03/23] autobuild: Run nt4_dc and nt4_member tests in parallel

These do not interact with the main AD DC environments, so can run in parallel

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 script/autobuild.py | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/script/autobuild.py b/script/autobuild.py
index 82b011d..b551b2c 100755
--- a/script/autobuild.py
+++ b/script/autobuild.py
@@ -25,6 +25,7 @@ cleanup_list = []
 builddirs = {
     "ctdb"    : "ctdb",
     "samba"  : ".",
+    "samba-nt4"  : ".",
     "samba-xc" : ".",
     "samba-o3" : ".",
     "samba-ctdb" : ".",
@@ -73,14 +74,22 @@ tasks = {
                ("check-clean-tree", "../script/clean-source-tree.sh", "text/plain"),
                ("clean", "make clean", "text/plain") ],
 
-    # We have 'test' before 'install' because, 'test' should work without 'install'
+    # We have 'test' before 'install' because, 'test' should work without 'install (runs ad_dc_ntvfs and all the other envs)'
     "samba" : [ ("configure", "./configure.developer --with-selftest-prefix=./bin/ab" + samba_configure_params, "text/plain"),
                 ("make", "make -j", "text/plain"),
-                ("test", "make test FAIL_IMMEDIATELY=1 TESTS='--exclude-env=none'", "text/plain"),
+                ("test", "make test FAIL_IMMEDIATELY=1 TESTS='--exclude-env=nt4_dc --exclude-env=nt4_member --exclude-env=ad_dc --exclude-env=none'", "text/plain"),
                 ("install", "make install", "text/plain"),
                 ("check-clean-tree", "script/clean-source-tree.sh", "text/plain"),
                 ("clean", "make clean", "text/plain") ],
 
+    # We split out this so the isolated nt4_dc tests do not wait for ad_dc or ad_dc_ntvfs tests (which are long)
+    "samba-nt4" : [ ("configure", "./configure.developer --with-selftest-prefix=./bin/ab" + samba_configure_params, "text/plain"),
+                       ("make", "make -j", "text/plain"),
+                       ("test", "make test FAIL_IMMEDIATELY=1 TESTS='--include-env=nt4_dc --include-env=nt4_member'", "text/plain"),
+                       ("install", "make install", "text/plain"),
+                       ("check-clean-tree", "script/clean-source-tree.sh", "text/plain"),
+                       ("clean", "make clean", "text/plain") ],
+
     "samba-test-only" : [ ("configure", "./configure.developer --with-selftest-prefix=./bin/ab  --abi-check-disable" + samba_configure_params, "text/plain"),
                           ("make", "make -j", "text/plain"),
                           ("test", "make test FAIL_IMMEDIATELY=1 TESTS=${TESTS}", "text/plain") ],
-- 
2.9.4


From 3384c0d5e60f27416c0410f523165926c2828439 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 20 Apr 2017 14:00:21 +1200
Subject: [PATCH 04/23] getncchanges: Do not segfault if somehow we get 0
 results from an ldb_search with scope BASE

This should not happen, but we have seen this happen in autobuild
before the whole-DB locking issues were resolved by
https://bugzilla.samba.org/show_bug.cgi?id=12858

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/rpc_server/drsuapi/getncchanges.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c
index da294a6..a2063aa 100644
--- a/source4/rpc_server/drsuapi/getncchanges.c
+++ b/source4/rpc_server/drsuapi/getncchanges.c
@@ -2578,8 +2578,13 @@ allowed:
 		W_ERROR_HAVE_NO_MEMORY(msg_dn);
 
 
-		/* by re-searching here we avoid having a lot of full
-		 * records in memory between calls to getncchanges
+		/*
+		 * by re-searching here we avoid having a lot of full
+		 * records in memory between calls to getncchanges.
+		 *
+		 * We expect that we may get some objects that vanish
+		 * (tombstone expunge) between the first and second
+		 * check.
 		 */
 		ret = drsuapi_search_with_extended_dn(sam_ctx, obj, &msg_res,
 						      msg_dn,
@@ -2593,6 +2598,16 @@ allowed:
 			continue;
 		}
 
+		if (msg_res->count == 0) {
+			DEBUG(1,("getncchanges: got LDB_SUCCESS but failed"
+				 "to get any results in fetch of DN "
+				 "%s (race with tombstone expunge?)\n",
+				 ldb_dn_get_extended_linearized(obj,
+								msg_dn, 1)));
+			talloc_free(obj);
+			continue;
+		}
+
 		msg = msg_res->msgs[0];
 
 		/*
-- 
2.9.4


From f1cb695f1ad2b195ecf0b83a2e5b33602c1776c2 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 4 Jul 2017 17:27:27 +1200
Subject: [PATCH 05/23] selftest: Add test for password change when NTLM is
 disabled

When NTLM is disabled, the server should reject NTLM-based password
changes. Changing the password is a bit complicated from python, but
because the server should reject the password change outright with
NTLM_BLOCKED, the test doesn't actually need to provide valid
credentials.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11923
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
 python/samba/tests/ntlmauth.py | 46 ++++++++++++++++++++++++++++++------------
 selftest/knownfail             |  2 ++
 2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/python/samba/tests/ntlmauth.py b/python/samba/tests/ntlmauth.py
index 8db1ad0..a232bf2 100644
--- a/python/samba/tests/ntlmauth.py
+++ b/python/samba/tests/ntlmauth.py
@@ -19,13 +19,13 @@ from samba.tests import TestCase
 import os
 
 import samba
-from samba.credentials import Credentials, DONT_USE_KERBEROS
+from samba.credentials import Credentials, DONT_USE_KERBEROS, MUST_USE_KERBEROS
 
 from samba import NTSTATUSError, ntstatus
 import ctypes
 
 from samba import credentials
-from samba.dcerpc import srvsvc
+from samba.dcerpc import srvsvc, samr, lsa
 
 """
 Tests basic NTLM authentication
@@ -37,24 +37,21 @@ class NtlmAuthTests(TestCase):
         super(NtlmAuthTests, self).setUp()
 
         self.lp          = self.get_loadparm()
+        self.server      = os.getenv("SERVER")
 
-
+        self.creds = Credentials()
+        self.creds.guess(self.lp)
+        self.creds.set_username(os.getenv("USERNAME"))
+        self.creds.set_domain(self.server)
+        self.creds.set_password(os.getenv("PASSWORD"))
+        self.creds.set_kerberos_state(DONT_USE_KERBEROS)
 
     def tearDown(self):
         super(NtlmAuthTests, self).tearDown()
 
     def test_ntlm_connection(self):
-        server = os.getenv("SERVER")
-
-        creds = credentials.Credentials()
-        creds.guess(self.lp)
-        creds.set_username(os.getenv("USERNAME"))
-        creds.set_domain(server)
-        creds.set_password(os.getenv("PASSWORD"))
-        creds.set_kerberos_state(DONT_USE_KERBEROS)
-
         try:
-            conn = srvsvc.srvsvc("ncacn_np:%s[smb2,ntlm]" % server, self.lp, creds)
+            conn = srvsvc.srvsvc("ncacn_np:%s[smb2,ntlm]" % self.server, self.lp, self.creds)
 
             self.assertIsNotNone(conn)
         except NTSTATUSError as e:
@@ -65,4 +62,27 @@ class NtlmAuthTests(TestCase):
             else:
                 raise
 
+    def test_samr_change_password(self):
+        self.creds.set_kerberos_state(MUST_USE_KERBEROS)
+        conn = samr.samr("ncacn_np:%s[krb5,seal,smb2]" % os.getenv("SERVER"))
+
+        # we want to check whether this gets rejected outright because NTLM is
+        # disabled, so we don't actually need to encrypt a valid password here
+        server = lsa.String()
+        server.string = self.server
+        username = lsa.String()
+        username.string = os.getenv("USERNAME")
+
+        try:
+            conn.ChangePasswordUser2(server, username, None, None, True, None, None)
+        except NTSTATUSError as e:
+            # changing passwords is rejected when NTLM is disabled
+            enum = ctypes.c_uint32(e[0]).value
+            if enum == ntstatus.NT_STATUS_NTLM_BLOCKED:
+                self.fail("NTLM is disabled on this server")
+            elif enum == ntstatus.NT_STATUS_WRONG_PASSWORD:
+                # expected error case when NTLM is enabled
+                pass
+            else:
+                raise
 
diff --git a/selftest/knownfail b/selftest/knownfail
index 1cba331..f41b99d 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -342,3 +342,5 @@
 ^samba.tests.netlogonsvc.python\(fileserver\)
 # NTLM authentication is (intentionally) disabled in ktest
 ^samba.tests.ntlmauth.python\(ktest\).ntlmauth.NtlmAuthTests.test_ntlm_connection\(ktest\)
+# Disabling NTLM means you can't use samr to change the password
+^samba.tests.ntlmauth.python\(ktest\).ntlmauth.NtlmAuthTests.test_samr_change_password\(ktest\)
-- 
2.9.4


From d43867c956c86acf5a1185f52333772aa966bc17 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 14:44:46 +1200
Subject: [PATCH 06/23] pycredentials: Allow optional "name" argument to
 get_named_ccache() to be missing

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 auth/credentials/pycredentials.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/auth/credentials/pycredentials.c b/auth/credentials/pycredentials.c
index caf30bf..0a29c58 100644
--- a/auth/credentials/pycredentials.c
+++ b/auth/credentials/pycredentials.c
@@ -526,7 +526,7 @@ static PyObject *PyCredentialCacheContainer_from_ccache_container(struct ccache_
 static PyObject *py_creds_get_named_ccache(PyObject *self, PyObject *args)
 {
 	PyObject *py_lp_ctx = Py_None;
-	char *ccache_name;
+	char *ccache_name = NULL;
 	struct loadparm_context *lp_ctx;
 	struct ccache_container *ccc;
 	struct tevent_context *event_ctx;
-- 
2.9.4


From ffc79b963b3c59292d8b7ebf22b7f359cf19ca8d Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 14:47:01 +1200
Subject: [PATCH 07/23] pycredentials: Add set_named_ccache() to set a
 particular credentials cache

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 auth/credentials/pycredentials.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/auth/credentials/pycredentials.c b/auth/credentials/pycredentials.c
index 0a29c58..bf90eb8 100644
--- a/auth/credentials/pycredentials.c
+++ b/auth/credentials/pycredentials.c
@@ -31,6 +31,8 @@
 #include <tevent.h>
 #include "libcli/auth/libcli_auth.h"
 #include "auth/credentials/credentials_internal.h"
+#include "system/kerberos.h"
+#include "auth/kerberos/kerberos.h"
 
 void initcredentials(void);
 
@@ -793,10 +795,37 @@ PyTypeObject PyCredentials = {
 	.tp_methods = py_creds_methods,
 };
 
+static PyObject *py_ccache_name(PyObject *self, PyObject *unused)
+{
+	struct ccache_container *ccc;
+	char *name;
+	PyObject *py_name;
+	int ret;
+	ccc = pytalloc_get_type(self, struct ccache_container);
+
+	ret = krb5_cc_get_full_name(ccc->smb_krb5_context->krb5_context,
+				    ccc->ccache, &name);
+	if (ret == 0) {
+		py_name = PyString_FromStringOrNULL(name);
+		SAFE_FREE(name);
+	} else {
+		PyErr_SetString(PyExc_RuntimeError,
+				"Failed to get ccache name");
+		return NULL;
+	}
+	return py_name;
+}
+
+static PyMethodDef py_ccache_container_methods[] = {
+	{ "get_name", py_ccache_name, METH_NOARGS,
+	  "S.get_name() -> name\nObtain KRB5 credentials cache name." },
+	{ NULL }
+};
 
 PyTypeObject PyCredentialCacheContainer = {
 	.tp_name = "credentials.CredentialCacheContainer",
 	.tp_flags = Py_TPFLAGS_DEFAULT,
+	.tp_methods = py_ccache_container_methods,
 };
 
 MODULE_INIT_FUNC(credentials)
-- 
2.9.4


From aba3dcb425f9b70e707c6aa4cdfaf68857e67eaf Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 14:48:39 +1200
Subject: [PATCH 08/23] selftest: Add tests for credentials.get_named_ccache()

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/krb5_credentials.py | 103 +++++++++++++++++++++++++++++++++
 source4/selftest/tests.py              |   3 +
 2 files changed, 106 insertions(+)
 create mode 100644 python/samba/tests/krb5_credentials.py

diff --git a/python/samba/tests/krb5_credentials.py b/python/samba/tests/krb5_credentials.py
new file mode 100644
index 0000000..a3eb034
--- /dev/null
+++ b/python/samba/tests/krb5_credentials.py
@@ -0,0 +1,103 @@
+# Integration tests for pycredentials
+#
+# Copyright (C) Catalyst IT Ltd. 2017
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+from samba.tests import TestCase, delete_force
+import os
+
+import samba
+from samba.auth import system_session
+from samba.credentials import (
+    Credentials,
+)
+from samba.dsdb import (
+    UF_WORKSTATION_TRUST_ACCOUNT,
+    UF_PASSWD_NOTREQD,
+    UF_NORMAL_ACCOUNT)
+from samba.samdb import SamDB
+
+"""KRB5 Integration tests for pycredentials.
+
+Seperated from py_credentials so as to allow running against just one
+environment so we know the server that we add the user on will be our
+KDC
+
+"""
+
+MACHINE_NAME = "krb5credstest"
+
+class PyKrb5CredentialsTests(TestCase):
+
+    def setUp(self):
+        super(PyKrb5CredentialsTests, self).setUp()
+
+        self.server      = os.environ["SERVER"]
+        self.domain      = os.environ["DOMAIN"]
+        self.host        = os.environ["SERVER_IP"]
+        self.lp          = self.get_loadparm()
+
+        self.credentials = self.get_credentials()
+
+        self.session     = system_session()
+        self.ldb = SamDB(url="ldap://%s" % self.host,
+                         session_info=self.session,
+                         credentials=self.credentials,
+                         lp=self.lp)
+
+        self.create_machine_account()
+
+
+    def tearDown(self):
+        super(PyKrb5CredentialsTests, self).tearDown()
+        delete_force(self.ldb, self.machine_dn)
+
+    def test_get_named_ccache(self):
+        name = "MEMORY:py_creds_machine"
+        ccache = self.machine_creds.get_named_ccache(self.lp,
+                                                     name)
+        self.assertEqual(ccache.get_name(), name)
+
+    def test_get_unnamed_ccache(self):
+        ccache = self.machine_creds.get_named_ccache(self.lp)
+        self.assertIsNotNone(ccache.get_name())
+
+    #
+    # Create the machine account
+    def create_machine_account(self):
+        self.machine_pass = samba.generate_random_password(32, 32)
+        self.machine_name = MACHINE_NAME
+        self.machine_dn = "cn=%s,%s" % (self.machine_name, self.ldb.domain_dn())
+
+        # remove the account if it exists, this will happen if a previous test
+        # run failed
+        delete_force(self.ldb, self.machine_dn)
+
+        utf16pw = unicode(
+            '"' + self.machine_pass.encode('utf-8') + '"', 'utf-8'
+        ).encode('utf-16-le')
+        self.ldb.add({
+            "dn": self.machine_dn,
+            "objectclass": "computer",
+            "sAMAccountName": "%s$" % self.machine_name,
+            "userAccountControl":
+                str(UF_WORKSTATION_TRUST_ACCOUNT | UF_PASSWD_NOTREQD),
+            "unicodePwd": utf16pw})
+
+        self.machine_creds = Credentials()
+        self.machine_creds.guess(self.get_loadparm())
+        self.machine_creds.set_password(self.machine_pass)
+        self.machine_creds.set_username(self.machine_name + "$")
+        self.machine_creds.set_workstation(self.machine_name)
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 15037a2..9e98923 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -663,6 +663,9 @@ planoldpythontestsuite("ad_dc",
                        extra_args=['-U"$USERNAME%$PASSWORD"'])
 
 planpythontestsuite("ad_dc_ntvfs:local", "samba.tests.lsa_string")
+planoldpythontestsuite("ad_dc_ntvfs",
+                       "samba.tests.krb5_credentials",
+                       extra_args=['-U"$USERNAME%$PASSWORD"'])
 for env in ["ad_dc_ntvfs", "vampire_dc", "promoted_dc"]:
     planoldpythontestsuite(env,
                            "samba.tests.py_credentials",
-- 
2.9.4


From d72448ad3b1e42fc274b08e7f0bb8e2fadd4bfcd Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 14:51:22 +1200
Subject: [PATCH 09/23] pycredentials: Add set_named_ccache()

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 auth/credentials/pycredentials.c       | 42 ++++++++++++++++++++++++++++++++++
 python/samba/tests/krb5_credentials.py |  9 ++++++++
 2 files changed, 51 insertions(+)

diff --git a/auth/credentials/pycredentials.c b/auth/credentials/pycredentials.c
index bf90eb8..9c6faef 100644
--- a/auth/credentials/pycredentials.c
+++ b/auth/credentials/pycredentials.c
@@ -571,6 +571,45 @@ static PyObject *py_creds_get_named_ccache(PyObject *self, PyObject *args)
 	return NULL;
 }
 
+static PyObject *py_creds_set_named_ccache(PyObject *self, PyObject *args)
+{
+	int ret;
+	char *newval;
+	const char *error_string;
+	TALLOC_CTX *mem_ctx;
+	struct loadparm_context *lp_ctx;
+	PyObject *py_lp_ctx = Py_None;
+	enum credentials_obtained obt = CRED_SPECIFIED;
+	int _obt = obt;
+	if (!PyArg_ParseTuple(args, "s|iO", &newval, &_obt, &py_lp_ctx))
+		return NULL;
+
+	mem_ctx = talloc_new(NULL);
+	if (mem_ctx == NULL) {
+		PyErr_NoMemory();
+		return NULL;
+	}
+
+	lp_ctx = lpcfg_from_py_object(mem_ctx, py_lp_ctx);
+	if (lp_ctx == NULL) {
+		talloc_free(mem_ctx);
+		return NULL;
+	}
+
+	ret = cli_credentials_set_ccache(PyCredentials_AsCliCredentials(self),
+					 lp_ctx,
+					 newval, CRED_SPECIFIED,
+					 &error_string);
+
+	if (ret != 0) {
+		PyErr_SetString(PyExc_RuntimeError, error_string?error_string:"NULL");
+		return NULL;
+	}
+
+	talloc_free(mem_ctx);
+	Py_RETURN_NONE;
+}
+
 static PyObject *py_creds_set_gensec_features(PyObject *self, PyObject *args)
 {
 	unsigned int gensec_features;
@@ -756,6 +795,9 @@ static PyMethodDef py_creds_methods[] = {
 	{ "guess", py_creds_guess, METH_VARARGS, NULL },
 	{ "set_machine_account", py_creds_set_machine_account, METH_VARARGS, NULL },
 	{ "get_named_ccache", py_creds_get_named_ccache, METH_VARARGS, NULL },
+	{ "set_named_ccache", py_creds_set_named_ccache, METH_VARARGS,
+		"S.set_named_ccache(krb5_ccache_name, obtained, lp) -> None\n"
+		"Set credentials to KRB5 Credentials Cache (by name)." },
 	{ "set_gensec_features", py_creds_set_gensec_features, METH_VARARGS, NULL },
 	{ "get_gensec_features", py_creds_get_gensec_features, METH_NOARGS, NULL },
 	{ "get_forced_sasl_mech", py_creds_get_forced_sasl_mech, METH_NOARGS,
diff --git a/python/samba/tests/krb5_credentials.py b/python/samba/tests/krb5_credentials.py
index a3eb034..cad19da 100644
--- a/python/samba/tests/krb5_credentials.py
+++ b/python/samba/tests/krb5_credentials.py
@@ -74,6 +74,15 @@ class PyKrb5CredentialsTests(TestCase):
         ccache = self.machine_creds.get_named_ccache(self.lp)
         self.assertIsNotNone(ccache.get_name())
 
+    def test_set_named_ccache(self):
+        ccache = self.machine_creds.get_named_ccache(self.lp)
+
+        creds = Credentials()
+        creds.set_named_ccache(ccache.get_name())
+
+        ccache2 = creds.get_named_ccache(self.lp)
+        self.assertEqual(ccache.get_name(), ccache2.get_name())
+
     #
     # Create the machine account
     def create_machine_account(self):
-- 
2.9.4


From 0425577b78d9437be7b1970f1cbdb38846e217b9 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 14:52:39 +1200
Subject: [PATCH 10/23] python/getopt: Add --krb5-ccache (for samba-tool etc)
 to match the C binaries

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/getopt.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/python/samba/getopt.py b/python/samba/getopt.py
index 9e1fb83..befc62b 100644
--- a/python/samba/getopt.py
+++ b/python/samba/getopt.py
@@ -158,6 +158,9 @@ class CredentialsOptions(optparse.OptionGroup):
                         action="callback",
                         help="Use stored machine account password",
                         callback=self._set_machine_pass)
+        self._add_option("--krb5-ccache", metavar="KRB5CCNAME",
+                        action="callback", type=str,
+                        help="Kerberos Credentials cache", callback=self._set_krb5_ccache)
         self.creds = Credentials()
 
     def _add_option(self, *args1, **kwargs):
@@ -198,6 +201,9 @@ class CredentialsOptions(optparse.OptionGroup):
     def _set_simple_bind_dn(self, option, opt_str, arg, parser):
         self.creds.set_bind_dn(arg)
 
+    def _set_krb5_ccache(self, option, opt_str, arg, parser):
+        self.creds.set_named_ccache(arg)
+
     def get_credentials(self, lp, fallback_machine=False):
         """Obtain the credentials set on the command-line.
 
-- 
2.9.4


From e197d89f029dceefc8726368a81378bf19ce25d9 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 15:02:00 +1200
Subject: [PATCH 11/23] selftest: Use self.runsubcmd() to run samba-tool for
 _test_join in ridalloc_exop.py

This is the standard way to run samba-tool from in the test scripts and allows
assertion that the command ran as expected

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/torture/drs/python/ridalloc_exop.py | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/source4/torture/drs/python/ridalloc_exop.py b/source4/torture/drs/python/ridalloc_exop.py
index 9196f4a..eacf3c3 100644
--- a/source4/torture/drs/python/ridalloc_exop.py
+++ b/source4/torture/drs/python/ridalloc_exop.py
@@ -305,14 +305,14 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
     def _test_join(self, server, netbios_name):
         tmpdir = os.path.join(self.tempdir, "targetdir")
         creds = self.get_credentials()
-        cmd = cmd_sambatool.subcommands['domain'].subcommands['join']
-        result = cmd._run("samba-tool domain join",
-                          creds.get_realm(),
-                          "dc", "-U%s%%%s" % (creds.get_username(),
-                                              creds.get_password()),
-                          '--targetdir=%s' % tmpdir,
-                          '--server=%s' % server,
-                          "--option=netbios name = %s" % netbios_name)
+        (result, out, err) = self.runsubcmd("domain", "join",
+                                            creds.get_realm(),
+                                            "dc", "-U%s%%%s" % (creds.get_username(),
+                                                                creds.get_password()),
+                                            '--targetdir=%s' % tmpdir,
+                                            '--server=%s' % server,
+                                            "--option=netbios name = %s" % netbios_name)
+        self.assertCmdSuccess(result, out, err)
         return tmpdir
 
     def _test_force_demote(self, server, netbios_name):
-- 
2.9.4


From a5d542fb839be740a7a73fee524a9e5c0b73bbda Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 15:05:08 +1200
Subject: [PATCH 12/23] selftest: Use self.runsubcmd() to run samba-tool for
 _test_force_demote in ridalloc_exop.py

This is the standard way to run samba-tool from in the test scripts and allows
assertion that the command ran as expected

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/torture/drs/python/ridalloc_exop.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/source4/torture/drs/python/ridalloc_exop.py b/source4/torture/drs/python/ridalloc_exop.py
index eacf3c3..dfab638 100644
--- a/source4/torture/drs/python/ridalloc_exop.py
+++ b/source4/torture/drs/python/ridalloc_exop.py
@@ -317,12 +317,12 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
 
     def _test_force_demote(self, server, netbios_name):
         creds = self.get_credentials()
-        cmd = cmd_sambatool.subcommands['domain'].subcommands['demote']
-        result = cmd._run("samba-tool domain demote",
-                          "-U%s%%%s" % (creds.get_username(),
-                                              creds.get_password()),
-                          '--server=%s' % server,
-                          "--remove-other-dead-server=%s" % netbios_name)
+        (result, out, err) = self.runsubcmd("domain", "demote",
+                                            "-U%s%%%s" % (creds.get_username(),
+                                                          creds.get_password()),
+                                            '--server=%s' % server,
+                                            "--remove-other-dead-server=%s" % netbios_name)
+        self.assertCmdSuccess(result, out, err)
 
     def test_offline_manual_seized_ridalloc_with_dbcheck(self):
         """Peform the same actions as test_offline_samba_tool_seized_ridalloc,
-- 
2.9.4


From 74ba66b4b1b2baebef43b5c80803fd9aeb4cdfb4 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 15:06:10 +1200
Subject: [PATCH 13/23] selftest: Remove unused import in ridalloc_exop.py

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/torture/drs/python/ridalloc_exop.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/source4/torture/drs/python/ridalloc_exop.py b/source4/torture/drs/python/ridalloc_exop.py
index dfab638..80d0111 100644
--- a/source4/torture/drs/python/ridalloc_exop.py
+++ b/source4/torture/drs/python/ridalloc_exop.py
@@ -40,7 +40,6 @@ from samba.drs_utils import drs_DsBind
 from samba.samdb import SamDB
 
 import shutil, tempfile, os
-from samba.netcmd.main import cmd_sambatool
 from samba.auth import system_session, admin_session
 from samba.dbchecker import dbcheck
 from samba.ndr import ndr_pack
-- 
2.9.4


From dcebd57fd8de122d51043cfbc35046b8133d9f14 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 7 Jul 2017 12:53:25 +1200
Subject: [PATCH 14/23] selftest: Use self.runsubcommand() in
 DrsReplicaSyncTestCase

This will allow catching the correct error messages and failure when _net_drs_replicate()
is reworked to not use a subprocess.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/torture/drs/python/replica_sync.py | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/source4/torture/drs/python/replica_sync.py b/source4/torture/drs/python/replica_sync.py
index dd862ce..65e6f1e 100644
--- a/source4/torture/drs/python/replica_sync.py
+++ b/source4/torture/drs/python/replica_sync.py
@@ -70,12 +70,23 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
     def test_ReplDisabled(self):
         """Tests we cann't replicate when replication is disabled"""
         self._disable_inbound_repl(self.dnsname_dc1)
-        try:
-            self._net_drs_replicate(DC=self.dnsname_dc1, fromDC=self.dnsname_dc2, forced=False)
-        except samba.tests.BlackboxProcessError, e:
-            self.assertTrue('WERR_DS_DRA_SINK_DISABLED' in e.stderr)
-        else:
-            self.fail("'drs replicate' command should have failed!")
+        
+        ccache_name = self.get_creds_ccache_name()
+
+        # Tunnel the command line credentials down to the
+        # subcommand to avoid a new kinit
+        cmdline_auth = "--krb5-ccache=%s" % ccache_name
+
+        # bin/samba-tool drs <drs_command> <cmdline_auth>
+        cmd_list = ["drs", "replicate", cmdline_auth]
+
+        nc_dn = self.domain_dn
+        # bin/samba-tool drs replicate <Dest_DC_NAME> <Src_DC_NAME> <Naming Context>
+        cmd_list += [self.dnsname_dc1, self.dnsname_dc2, nc_dn]
+        
+        (result, out, err) = self.runsubcmd(*cmd_list)
+        self.assertCmdFail(result)
+        self.assertTrue('WERR_DS_DRA_SINK_DISABLED' in err)
 
     def test_ReplDisabledForced(self):
         """Tests we can force replicate when replication is disabled"""
-- 
2.9.4


From 796d164fe0b685b0f2bf9e1d26445a6ca306427f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 15:25:36 +1200
Subject: [PATCH 15/23] selftest: Port DrsBaseTestCase._net_drs_replicate() to
 self.runsubcmd()

This avoids forking a subprocess with self.check_run()

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/torture/drs/python/drs_base.py | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
index f07592d..8703617 100644
--- a/source4/torture/drs/python/drs_base.py
+++ b/source4/torture/drs/python/drs_base.py
@@ -108,20 +108,32 @@ class DrsBaseTestCase(SambaToolCmdTest):
         # bin/samba-tool drs <drs_command> <cmdline_auth>
         return "%s drs %s %s" % (samba_tool_cmd, drs_command, cmdline_auth)
 
+    def _samba_tool_cmd_list(self, drs_command):
+        # make command line credentials string
+        creds = self.get_credentials()
+        cmdline_auth = "-U%s/%s%%%s" % (creds.get_domain(),
+                                        creds.get_username(), creds.get_password())
+        # bin/samba-tool drs <drs_command> <cmdline_auth>
+        return ["drs", drs_command, cmdline_auth]
+
     def _net_drs_replicate(self, DC, fromDC, nc_dn=None, forced=True, local=False, full_sync=False):
         if nc_dn is None:
             nc_dn = self.domain_dn
         # make base command line
-        samba_tool_cmdline = self._samba_tool_cmdline("replicate")
+        samba_tool_cmdline = self._samba_tool_cmd_list("replicate")
+        # bin/samba-tool drs replicate <Dest_DC_NAME> <Src_DC_NAME> <Naming Context>
+        samba_tool_cmdline += [DC, fromDC, nc_dn]
+
         if forced:
-            samba_tool_cmdline += " --sync-forced"
+            samba_tool_cmdline += ["--sync-forced"]
         if local:
-            samba_tool_cmdline += " --local"
+            samba_tool_cmdline += ["--local"]
         if full_sync:
-            samba_tool_cmdline += " --full-sync"
-        # bin/samba-tool drs replicate <Dest_DC_NAME> <Src_DC_NAME> <Naming Context>
-        cmd_line = "%s %s %s %s" % (samba_tool_cmdline, DC, fromDC, nc_dn)
-        return self.check_output(cmd_line)
+            samba_tool_cmdline += ["--full-sync"]
+
+        (result, out, err) = self.runsubcmd(*samba_tool_cmdline)
+        self.assertCmdSuccess(result, out, err)
+        self.assertEquals(err,"","Shouldn't be any error messages")
 
     def _enable_inbound_repl(self, DC):
         # make base command line
-- 
2.9.4


From e96806f47adc6203139267b2dbd946f4bf45b29a Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 16:01:56 +1200
Subject: [PATCH 16/23] selftest: Port DrsBaseTestCase to self.runsubcmd()

This avoids forking a subprocess with self.check_run() for:
 - _disable_inbound_repl
 - _enable_inbound_repl
 - _disable_all_repl
 - _enable_all_repl

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/torture/drs/python/drs_base.py | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
index 8703617..97a2bfb 100644
--- a/source4/torture/drs/python/drs_base.py
+++ b/source4/torture/drs/python/drs_base.py
@@ -139,27 +139,39 @@ class DrsBaseTestCase(SambaToolCmdTest):
         # make base command line
         samba_tool_cmd = self._samba_tool_cmdline("options")
         # disable replication
-        self.check_run("%s %s --dsa-option=-DISABLE_INBOUND_REPL" %(samba_tool_cmd, DC))
+        samba_tool_cmdline += [DC, "--dsa-option=-DISABLE_INBOUND_REPL"]
+        (result, out, err) = self.runsubcmd(*samba_tool_cmdline)
+        self.assertCmdSuccess(result, out, err)
+        self.assertEquals(err,"","Shouldn't be any error messages")
 
     def _disable_inbound_repl(self, DC):
         # make base command line
         samba_tool_cmd = self._samba_tool_cmdline("options")
         # disable replication
-        self.check_run("%s %s --dsa-option=+DISABLE_INBOUND_REPL" %(samba_tool_cmd, DC))
+        samba_tool_cmdline += [DC, "--dsa-option=+DISABLE_INBOUND_REPL"]
+        (result, out, err) = self.runsubcmd(*samba_tool_cmdline)
+        self.assertCmdSuccess(result, out, err)
+        self.assertEquals(err,"","Shouldn't be any error messages")
 
     def _enable_all_repl(self, DC):
+        self.enable_inbound_repl(DC)
         # make base command line
-        samba_tool_cmd = self._samba_tool_cmdline("options")
-        # disable replication
-        self.check_run("%s %s --dsa-option=-DISABLE_INBOUND_REPL" %(samba_tool_cmd, DC))
-        self.check_run("%s %s --dsa-option=-DISABLE_OUTBOUND_REPL" %(samba_tool_cmd, DC))
+        samba_tool_cmdline = self._samba_tool_cmdline("options")
+        # enable replication
+        samba_tool_cmdline += [DC, "--dsa-option=-DISABLE_OUTBOUND_REPL"]
+        (result, out, err) = self.runsubcmd(*samba_tool_cmdline)
+        self.assertCmdSuccess(result, out, err)
+        self.assertEquals(err,"","Shouldn't be any error messages")
 
     def _disable_all_repl(self, DC):
+        self.disable_inbound_repl(DC)
         # make base command line
-        samba_tool_cmd = self._samba_tool_cmdline("options")
+        samba_tool_cmdline = self._samba_tool_cmdline("options")
         # disable replication
-        self.check_run("%s %s --dsa-option=+DISABLE_INBOUND_REPL" %(samba_tool_cmd, DC))
-        self.check_run("%s %s --dsa-option=+DISABLE_OUTBOUND_REPL" %(samba_tool_cmd, DC))
+        samba_tool_cmdline += [DC, "--dsa-option=+DISABLE_OUTBOUND_REPL"]
+        (result, out, err) = self.runsubcmd(*samba_tool_cmdline)
+        self.assertCmdSuccess(result, out, err)
+        self.assertEquals(err,"","Shouldn't be any error messages")
 
     def _get_highest_hwm_utdv(self, ldb_conn):
         res = ldb_conn.search("", scope=ldb.SCOPE_BASE, attrs=["highestCommittedUSN"])
-- 
2.9.4


From 2fab0cc755400527dfe32baa154531c8fb90bae4 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 16:05:00 +1200
Subject: [PATCH 17/23] selftest: Port DrsBaseTestCase._enable_inbound_repl()
 to self.runsubcmd()

This avoids forking a subprocess with self.check_run()

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/torture/drs/python/drs_base.py | 31 +++++++++++--------------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
index 97a2bfb..5ddd2a7 100644
--- a/source4/torture/drs/python/drs_base.py
+++ b/source4/torture/drs/python/drs_base.py
@@ -137,10 +137,10 @@ class DrsBaseTestCase(SambaToolCmdTest):
 
     def _enable_inbound_repl(self, DC):
         # make base command line
-        samba_tool_cmd = self._samba_tool_cmdline("options")
+        samba_tool_cmd = self._samba_tool_cmd_list("options")
         # disable replication
-        samba_tool_cmdline += [DC, "--dsa-option=-DISABLE_INBOUND_REPL"]
-        (result, out, err) = self.runsubcmd(*samba_tool_cmdline)
+        samba_tool_cmd += [DC, "--dsa-option=-DISABLE_INBOUND_REPL"]
+        (result, out, err) = self.runsubcmd(*samba_tool_cmd)
         self.assertCmdSuccess(result, out, err)
         self.assertEquals(err,"","Shouldn't be any error messages")
 
@@ -148,30 +148,21 @@ class DrsBaseTestCase(SambaToolCmdTest):
         # make base command line
         samba_tool_cmd = self._samba_tool_cmdline("options")
         # disable replication
-        samba_tool_cmdline += [DC, "--dsa-option=+DISABLE_INBOUND_REPL"]
-        (result, out, err) = self.runsubcmd(*samba_tool_cmdline)
-        self.assertCmdSuccess(result, out, err)
-        self.assertEquals(err,"","Shouldn't be any error messages")
+        self.check_run("%s %s --dsa-option=+DISABLE_INBOUND_REPL" %(samba_tool_cmd, DC))
 
     def _enable_all_repl(self, DC):
-        self.enable_inbound_repl(DC)
         # make base command line
-        samba_tool_cmdline = self._samba_tool_cmdline("options")
-        # enable replication
-        samba_tool_cmdline += [DC, "--dsa-option=-DISABLE_OUTBOUND_REPL"]
-        (result, out, err) = self.runsubcmd(*samba_tool_cmdline)
-        self.assertCmdSuccess(result, out, err)
-        self.assertEquals(err,"","Shouldn't be any error messages")
+        samba_tool_cmd = self._samba_tool_cmdline("options")
+        # disable replication
+        self.check_run("%s %s --dsa-option=-DISABLE_INBOUND_REPL" %(samba_tool_cmd, DC))
+        self.check_run("%s %s --dsa-option=-DISABLE_OUTBOUND_REPL" %(samba_tool_cmd, DC))
 
     def _disable_all_repl(self, DC):
-        self.disable_inbound_repl(DC)
         # make base command line
-        samba_tool_cmdline = self._samba_tool_cmdline("options")
+        samba_tool_cmd = self._samba_tool_cmdline("options")
         # disable replication
-        samba_tool_cmdline += [DC, "--dsa-option=+DISABLE_OUTBOUND_REPL"]
-        (result, out, err) = self.runsubcmd(*samba_tool_cmdline)
-        self.assertCmdSuccess(result, out, err)
-        self.assertEquals(err,"","Shouldn't be any error messages")
+        self.check_run("%s %s --dsa-option=+DISABLE_INBOUND_REPL" %(samba_tool_cmd, DC))
+        self.check_run("%s %s --dsa-option=+DISABLE_OUTBOUND_REPL" %(samba_tool_cmd, DC))
 
     def _get_highest_hwm_utdv(self, ldb_conn):
         res = ldb_conn.search("", scope=ldb.SCOPE_BASE, attrs=["highestCommittedUSN"])
-- 
2.9.4


From 48e45ea34263993d899000b95837f6ef2408d19a Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 16:09:54 +1200
Subject: [PATCH 18/23] selftest: Port DrsBaseTestCase._disable_inbound_repl()
 to self.runsubcmd()

This avoids forking a subprocess with self.check_run()

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/torture/drs/python/drs_base.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
index 5ddd2a7..03131d7 100644
--- a/source4/torture/drs/python/drs_base.py
+++ b/source4/torture/drs/python/drs_base.py
@@ -146,9 +146,12 @@ class DrsBaseTestCase(SambaToolCmdTest):
 
     def _disable_inbound_repl(self, DC):
         # make base command line
-        samba_tool_cmd = self._samba_tool_cmdline("options")
+        samba_tool_cmd = self._samba_tool_cmd_list("options")
         # disable replication
-        self.check_run("%s %s --dsa-option=+DISABLE_INBOUND_REPL" %(samba_tool_cmd, DC))
+        samba_tool_cmd += [DC, "--dsa-option=+DISABLE_INBOUND_REPL"]
+        (result, out, err) = self.runsubcmd(*samba_tool_cmd)
+        self.assertCmdSuccess(result, out, err)
+        self.assertEquals(err,"","Shouldn't be any error messages")
 
     def _enable_all_repl(self, DC):
         # make base command line
-- 
2.9.4


From 881c3a5560452bba1c77060548f27f5bf8de6465 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 16:11:12 +1200
Subject: [PATCH 19/23] selftest: Port DrsBaseTestCase._{en,dis}able_all_repl()
 to self.runsubcmd()

This avoids forking a subprocess with self.check_run()

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/torture/drs/python/drs_base.py | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
index 03131d7..c215003 100644
--- a/source4/torture/drs/python/drs_base.py
+++ b/source4/torture/drs/python/drs_base.py
@@ -98,16 +98,6 @@ class DrsBaseTestCase(SambaToolCmdTest):
     def _make_obj_name(self, prefix):
         return prefix + time.strftime("%s", time.gmtime())
 
-    def _samba_tool_cmdline(self, drs_command):
-        # find out where is net command
-        samba_tool_cmd = os.path.abspath("./bin/samba-tool")
-        # make command line credentials string
-        creds = self.get_credentials()
-        cmdline_auth = "-U%s/%s%%%s" % (creds.get_domain(),
-                                        creds.get_username(), creds.get_password())
-        # bin/samba-tool drs <drs_command> <cmdline_auth>
-        return "%s drs %s %s" % (samba_tool_cmd, drs_command, cmdline_auth)
-
     def _samba_tool_cmd_list(self, drs_command):
         # make command line credentials string
         creds = self.get_credentials()
@@ -154,18 +144,24 @@ class DrsBaseTestCase(SambaToolCmdTest):
         self.assertEquals(err,"","Shouldn't be any error messages")
 
     def _enable_all_repl(self, DC):
+        self._enable_inbound_repl(DC)
         # make base command line
-        samba_tool_cmd = self._samba_tool_cmdline("options")
-        # disable replication
-        self.check_run("%s %s --dsa-option=-DISABLE_INBOUND_REPL" %(samba_tool_cmd, DC))
-        self.check_run("%s %s --dsa-option=-DISABLE_OUTBOUND_REPL" %(samba_tool_cmd, DC))
+        samba_tool_cmd = self._samba_tool_cmd_list("options")
+        # enable replication
+        samba_tool_cmd += [DC, "--dsa-option=-DISABLE_OUTBOUND_REPL"]
+        (result, out, err) = self.runsubcmd(*samba_tool_cmd)
+        self.assertCmdSuccess(result, out, err)
+        self.assertEquals(err,"","Shouldn't be any error messages")
 
     def _disable_all_repl(self, DC):
+        self._disable_inbound_repl(DC)
         # make base command line
-        samba_tool_cmd = self._samba_tool_cmdline("options")
+        samba_tool_cmd = self._samba_tool_cmd_list("options")
         # disable replication
-        self.check_run("%s %s --dsa-option=+DISABLE_INBOUND_REPL" %(samba_tool_cmd, DC))
-        self.check_run("%s %s --dsa-option=+DISABLE_OUTBOUND_REPL" %(samba_tool_cmd, DC))
+        samba_tool_cmd += [DC, "--dsa-option=+DISABLE_OUTBOUND_REPL"]
+        (result, out, err) = self.runsubcmd(*samba_tool_cmd)
+        self.assertCmdSuccess(result, out, err)
+        self.assertEquals(err,"","Shouldn't be any error messages")
 
     def _get_highest_hwm_utdv(self, ldb_conn):
         res = ldb_conn.search("", scope=ldb.SCOPE_BASE, attrs=["highestCommittedUSN"])
-- 
2.9.4


From 85ceeabb3d48a9f1d23693ea06d6781923115c25 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 16:25:19 +1200
Subject: [PATCH 20/23] selftest: Use new --krb5-ccache in drs_base.py

This means that instead of doing a new kinit, the process-wide ccache
is re-used, which is much faster.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/torture/drs/python/drs_base.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
index c215003..8af1af3 100644
--- a/source4/torture/drs/python/drs_base.py
+++ b/source4/torture/drs/python/drs_base.py
@@ -100,9 +100,15 @@ class DrsBaseTestCase(SambaToolCmdTest):
 
     def _samba_tool_cmd_list(self, drs_command):
         # make command line credentials string
+
         creds = self.get_credentials()
-        cmdline_auth = "-U%s/%s%%%s" % (creds.get_domain(),
-                                        creds.get_username(), creds.get_password())
+        ccache = creds.get_named_ccache(self.get_loadparm())
+        ccache_name = ccache.get_name()
+
+        # Tunnel the command line credentials down to the
+        # subcommand to avoid a new kinit
+        cmdline_auth = "--krb5-ccache=%s" % ccache_name
+
         # bin/samba-tool drs <drs_command> <cmdline_auth>
         return ["drs", drs_command, cmdline_auth]
 
-- 
2.9.4


From efb8b1554b3395b23b28b97e8d19dd28ed695cdd Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 16:29:14 +1200
Subject: [PATCH 21/23] selftest: Add and use new helper function
 get_creds_ccache_name()

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/tests/__init__.py         | 7 +++++++
 source4/torture/drs/python/drs_base.py | 4 +---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/python/samba/tests/__init__.py b/python/samba/tests/__init__.py
index 07c68c4..2ddfd9d 100644
--- a/python/samba/tests/__init__.py
+++ b/python/samba/tests/__init__.py
@@ -67,6 +67,13 @@ class TestCase(unittest.TestCase):
     def get_credentials(self):
         return cmdline_credentials
 
+    def get_creds_ccache_name(self):
+        creds = self.get_credentials()
+        ccache = creds.get_named_ccache(self.get_loadparm())
+        ccache_name = ccache.get_name()
+
+        return ccache_name
+
     def hexdump(self, src):
         N = 0
         result = ''
diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
index 8af1af3..9abfa15 100644
--- a/source4/torture/drs/python/drs_base.py
+++ b/source4/torture/drs/python/drs_base.py
@@ -101,9 +101,7 @@ class DrsBaseTestCase(SambaToolCmdTest):
     def _samba_tool_cmd_list(self, drs_command):
         # make command line credentials string
 
-        creds = self.get_credentials()
-        ccache = creds.get_named_ccache(self.get_loadparm())
-        ccache_name = ccache.get_name()
+        ccache_name = self.get_creds_ccache_name()
 
         # Tunnel the command line credentials down to the
         # subcommand to avoid a new kinit
-- 
2.9.4


From 213ceac2f1891f5a51840b3d944c6bfcc49d9b9e Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 6 Jul 2017 16:31:15 +1200
Subject: [PATCH 22/23] selftest: Use get_creds_ccache_name() in fsmo.py

This avoids a new kinit for every role transfer

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/torture/drs/python/fsmo.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/source4/torture/drs/python/fsmo.py b/source4/torture/drs/python/fsmo.py
index 4dd652d..50b7dc3 100644
--- a/source4/torture/drs/python/fsmo.py
+++ b/source4/torture/drs/python/fsmo.py
@@ -58,9 +58,8 @@ class DrsFsmoTestCase(drs_base.DrsBaseTestCase):
         # find out where is samba-tool command
         net_cmd = os.path.abspath("./bin/samba-tool")
         # make command line credentials string
-        creds = self.get_credentials()
-        cmd_line_auth = "-U%s/%s%%%s" % (creds.get_domain(),
-                                         creds.get_username(), creds.get_password())
+        ccache_name = self.get_creds_ccache_name()
+        cmd_line_auth = "--krb5-ccache=%s" % ccache_name
         (result, out, err) = self.runsubcmd("fsmo", "transfer",
                                             "--role=%s" % role,
                                             "-H", "ldap://%s:389" % DC,
-- 
2.9.4


From e9a20f845a53e9b10a64c96549b61850acc70218 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 21 Jul 2017 17:52:04 +1200
Subject: [PATCH 23/23] s4-drepl: Use tevent_schedule_immediate() in
 DsReplicaSync handler

When we are sent a DsReplicaSync() we should work on inbound replication
(ideally from the requested source, but so far we just start the whole queue)
right away, not after 1 second.

We should also target inbound replication, not any outbound replication
notification that may happen to be due.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/dsdb/repl/drepl_periodic.c | 66 ++++++++++----------------------------
 source4/dsdb/repl/drepl_service.c  |  2 +-
 source4/dsdb/repl/drepl_service.h  | 18 +++--------
 3 files changed, 23 insertions(+), 63 deletions(-)

diff --git a/source4/dsdb/repl/drepl_periodic.c b/source4/dsdb/repl/drepl_periodic.c
index d6b9467..04dbc01 100644
--- a/source4/dsdb/repl/drepl_periodic.c
+++ b/source4/dsdb/repl/drepl_periodic.c
@@ -134,63 +134,31 @@ void dreplsrv_run_pending_ops(struct dreplsrv_service *s)
 	}
 }
 
-static void dreplsrv_pending_run(struct dreplsrv_service *service);
-
-static void dreplsrv_pending_handler_te(struct tevent_context *ev, struct tevent_timer *te,
-					struct timeval t, void *ptr)
+static void dreplsrv_pending_pull_handler_im(struct tevent_context *ev,
+					     struct tevent_immediate *im,
+					     void *ptr)
 {
 	struct dreplsrv_service *service = talloc_get_type(ptr, struct dreplsrv_service);
 
-	service->pending.te = NULL;
+	TALLOC_FREE(service->pending.im);
 
-	dreplsrv_pending_run(service);
+	dreplsrv_run_pull_ops(service);
 }
 
-WERROR dreplsrv_pendingops_schedule(struct dreplsrv_service *service, uint32_t next_interval)
+WERROR dreplsrv_pendingops_schedule_pull_now(struct dreplsrv_service *service)
 {
-	TALLOC_CTX *tmp_mem;
-	struct tevent_timer *new_te;
-	struct timeval next_time;
-
-	/* prevent looping */
-	if (next_interval == 0) {
-		next_interval = 1;
-	}
-
-	next_time = timeval_current_ofs(next_interval, 50);
-
-	if (service->pending.te) {
-		/*
-		 * if the timestamp of the new event is higher,
-		 * as current next we don't need to reschedule
-		 */
-		if (timeval_compare(&next_time, &service->pending.next_event) > 0) {
-			return WERR_OK;
-		}
-	}
-
-	/* reset the next scheduled timestamp */
-	service->pending.next_event = next_time;
-
-	new_te = tevent_add_timer(service->task->event_ctx, service,
-				  service->pending.next_event,
-			         dreplsrv_pending_handler_te, service);
-	W_ERROR_HAVE_NO_MEMORY(new_te);
-
-	tmp_mem = talloc_new(service);
-	DEBUG(4,("dreplsrv_pending_schedule(%u) %sscheduled for: %s\n",
-		next_interval,
-		(service->pending.te?"re":""),
-		nt_time_string(tmp_mem, timeval_to_nttime(&next_time))));
-	talloc_free(tmp_mem);
-
-	talloc_free(service->pending.te);
-	service->pending.te = new_te;
+	struct tevent_immediate *new_im;
+	
+	new_im = tevent_create_immediate(service);
+	W_ERROR_HAVE_NO_MEMORY(new_im);
+	
+	tevent_schedule_immediate(new_im, service->task->event_ctx,
+				  dreplsrv_pending_pull_handler_im,
+				  service);
+
+	TALLOC_FREE(service->pending.im);
+	service->pending.im = new_im;
 
 	return WERR_OK;
 }
 
-static void dreplsrv_pending_run(struct dreplsrv_service *service)
-{
-	dreplsrv_run_pending_ops(service);
-}
diff --git a/source4/dsdb/repl/drepl_service.c b/source4/dsdb/repl/drepl_service.c
index 39791b4..8412f90 100644
--- a/source4/dsdb/repl/drepl_service.c
+++ b/source4/dsdb/repl/drepl_service.c
@@ -339,7 +339,7 @@ static NTSTATUS drepl_replica_sync(struct irpc_message *msg,
 	 * schedule replication event to force
 	 * replication as soon as possible
 	 */
-	dreplsrv_pendingops_schedule(service, 0);
+	dreplsrv_pendingops_schedule_pull_now(service);
 
 done:
 	return NT_STATUS_OK;
diff --git a/source4/dsdb/repl/drepl_service.h b/source4/dsdb/repl/drepl_service.h
index 6b85ad6..4b0e43f 100644
--- a/source4/dsdb/repl/drepl_service.h
+++ b/source4/dsdb/repl/drepl_service.h
@@ -187,21 +187,13 @@ struct dreplsrv_service {
 		struct tevent_timer *te;
 	} periodic;
 
-	/* some stuff for running only the pendings ops */
+	/* some stuff for running only the incoming notify ops */
 	struct {
-		/*
-		 * the interval between notify runs
-		 */
-		uint32_t interval;
-
-		/*
-		 * the timestamp for the next event,
-		 * this is the timstamp passed to event_add_timed()
+		/* 
+		 * here we have a reference to the immidicate event that was
+		 * scheduled from the DsReplicaSync
 		 */
-		struct timeval next_event;
-
-		/* here we have a reference to the timed event the schedules the notifies */
-		struct tevent_timer *te;
+		struct tevent_immediate *im;
 	} pending;
 
 	/* some stuff for notify processing */
-- 
2.9.4



More information about the samba-technical mailing list