[PATCH] samba-tool drs showrepl --summary

Douglas Bagnall douglas.bagnall at catalyst.net.nz
Wed Jun 27 03:23:23 UTC 2018


and the patch.

Douglas

On 27/06/18 15:22, Douglas Bagnall wrote:
> On 10/06/18 09:08, I wrote:
>> A while ago I added a --json option to make `samba-tool drs showrepl`
>> produce machine readable output. This time I'm aiming for *human*
>> readable output with a --summary option. It differs from what is now
>> called --classic (still the default) in that it doesn't go on and on
>> describing things that are perfectly normal. When it finds a problem
>> it reverts to verbosity in the classic style.
>>
>> A typical conversation with it should look like this:
>>
>>   $ samba-tool drs showrepl --summary $DC $CREDS
>>   [ALL GOOD]
>>   $
> 
> but the tests that had grown so happily in the sun of `make test`
> withered in the cruel environment of gitlab CI.
> Now they are better, robuster, only slightly less thorough.
> 
> Also included is a typically excellent patch from Tim Beale.
> 
> These are reviewed and will end up in autobuild soonish.
> 
> cheers,
> Douglas
> 

-------------- next part --------------
From b1b38b49afdd41227846b6b83f3370353bd97c8d Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 7 Jun 2018 14:27:37 +1200
Subject: [PATCH 1/8] samba-tool drs showrepl: add a --color flag

Nothing is using it yet, but the next commit will

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/netcmd/drs.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/python/samba/netcmd/drs.py b/python/samba/netcmd/drs.py
index 235d82b4484..254122a2924 100644
--- a/python/samba/netcmd/drs.py
+++ b/python/samba/netcmd/drs.py
@@ -38,8 +38,10 @@ from samba.dcerpc import drsuapi, misc
 from samba.join import join_clone
 from samba.ndr import ndr_unpack
 from samba.dcerpc import drsblobs
+from samba import colour
 import logging
 
+
 def drsuapi_connect(ctx):
     '''make a DRSUAPI connection to the server'''
     try:
@@ -103,6 +105,8 @@ class cmd_drs_showrepl(Command):
                dest='format', action='store_const', const='classic',
                default=DEFAULT_SHOWREPL_FORMAT),
         Option("-v", "--verbose", help="Be verbose", action="store_true"),
+        Option("--color", help="Use colour output (yes|no|auto)",
+               default='no'),
     ]
 
     takes_args = ["DC?"]
@@ -156,7 +160,8 @@ class cmd_drs_showrepl(Command):
     def run(self, DC=None, sambaopts=None,
             credopts=None, versionopts=None,
             format=DEFAULT_SHOWREPL_FORMAT,
-            verbose=False):
+            verbose=False, color='no'):
+        self.apply_colour_choice(color)
         self.lp = sambaopts.get_loadparm()
         if DC is None:
             DC = common.netcmd_dnsname(self.lp)
-- 
2.17.1


From b7e857cbe9302896a5d68723a43c58470ed3342e Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 7 Jun 2018 14:15:10 +1200
Subject: [PATCH 2/8] samba-tool drs showrepl --summary for a quick local check

The default output ("classic") gives you a lot of very uninteresting
detail when everything is fine. --summary shuts up about things that
are fine but shouts a little bit when things are broken. It doesn't
provide any new information, just tries to present it in a more useful
format.

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/netcmd/drs.py                    |  34 ++++
 .../drs/python/samba_tool_drs_showrepl.py     | 145 ++++++++++++++++++
 2 files changed, 179 insertions(+)

diff --git a/python/samba/netcmd/drs.py b/python/samba/netcmd/drs.py
index 254122a2924..613f56c8a4f 100644
--- a/python/samba/netcmd/drs.py
+++ b/python/samba/netcmd/drs.py
@@ -101,6 +101,8 @@ class cmd_drs_showrepl(Command):
     takes_options = [
         Option("--json", help="replication details in JSON format",
                dest='format', action='store_const', const='json'),
+        Option("--summary", help="summarize local DRS health",
+               dest='format', action='store_const', const='summary'),
         Option("--classic", help="print local replication details",
                dest='format', action='store_const', const='classic',
                default=DEFAULT_SHOWREPL_FORMAT),
@@ -170,6 +172,7 @@ class cmd_drs_showrepl(Command):
         self.verbose = verbose
 
         output_function = {
+            'summary': self.summary_output,
             'json': self.json_output,
             'classic': self.classic_output,
         }.get(format)
@@ -184,6 +187,37 @@ class cmd_drs_showrepl(Command):
         del data['server']
         json.dump(data, self.outf, indent=2)
 
+    def summary_output(self):
+        """Print a short message if every seems fine, but print details of any
+        links that seem broken."""
+        failing_repsto = []
+        failing_repsfrom = []
+
+        local_data = self.get_local_repl_data()
+        for rep in local_data['repsTo']:
+            if rep["consecutive failures"] != 0 or rep["last success"] == 0:
+                failing_repsto.append(rep)
+
+        for rep in local_data['repsFrom']:
+            if rep["consecutive failures"] != 0 or rep["last success"] == 0:
+                failing_repsto.append(rep)
+
+        if failing_repsto or failing_repsfrom:
+            self.message(colour.c_RED("There are failing connections"))
+            if failing_repsto:
+                self.message(colour.c_RED("Failing outbound connections:"))
+                for rep in failing_repsto:
+                    self.print_neighbour(rep)
+            if failing_repsfrom:
+                self.message(colour.c_RED("Failing inbound connection:"))
+                for rep in failing_repsfrom:
+                    self.print_neighbour(rep)
+
+            return 1
+
+        self.message(colour.c_GREEN("[ALL GOOD]"))
+
+
     def get_local_repl_data(self):
         drsuapi_connect(self)
         samdb_connect(self)
diff --git a/source4/torture/drs/python/samba_tool_drs_showrepl.py b/source4/torture/drs/python/samba_tool_drs_showrepl.py
index 90bb0484a27..f7a806e660e 100644
--- a/source4/torture/drs/python/samba_tool_drs_showrepl.py
+++ b/source4/torture/drs/python/samba_tool_drs_showrepl.py
@@ -20,8 +20,12 @@
 from __future__ import print_function
 import samba.tests
 import drs_base
+from samba.dcerpc import drsuapi
+from samba import drs_utils
 import re
 import json
+import ldb
+import random
 from samba.compat import PY3
 
 if PY3:
@@ -158,3 +162,144 @@ class SambaToolDrsShowReplTests(drs_base.DrsBaseTestCase):
             self.assertTrue(isinstance(n['options'], int))
             self.assertTrue(isinstance(n['replicates NC'], list))
             self.assertRegex("^%s$" % DN_RE, n["remote DN"])
+
+    def _force_all_reps(self, samdb, dc, direction):
+        if direction == 'inbound':
+            info_type = drsuapi.DRSUAPI_DS_REPLICA_INFO_NEIGHBORS
+        elif direction == 'outbound':
+            info_type = drsuapi.DRSUAPI_DS_REPLICA_INFO_REPSTO
+        else:
+            raise ValueError("expected 'inbound' or 'outbound'")
+
+        self._enable_all_repl(dc)
+        lp = self.get_loadparm()
+        creds = self.get_credentials()
+        drsuapi_conn, drsuapi_handle, _ = drs_utils.drsuapi_connect(dc, lp, creds)
+        req1 = drsuapi.DsReplicaGetInfoRequest1()
+        req1.info_type = info_type
+        _, info = drsuapi_conn.DsReplicaGetInfo(drsuapi_handle, 1, req1)
+        for x in info.array:
+            # you might think x.source_dsa_address was the thing, but no.
+            # and we need to filter out RODCs and deleted DCs
+
+            res = []
+            try:
+                res = samdb.search(base=x.source_dsa_obj_dn,
+                                   scope=ldb.SCOPE_BASE,
+                                   attrs=['msDS-isRODC', 'isDeleted'],
+                                   controls=['show_deleted:0'])
+            except ldb.LdbError as e:
+                if e.args[0] != ldb.ERR_NO_SUCH_OBJECT:
+                    raise
+
+            if (len(res) == 0 or
+                len(res[0].get('msDS-isRODC', '')) > 0 or
+                res[0]['isDeleted'] == 'TRUE'):
+                continue
+
+            dsa_dn = str(ldb.Dn(samdb, x.source_dsa_obj_dn).parent())
+            res = samdb.search(base=dsa_dn,
+                               scope=ldb.SCOPE_BASE,
+                               attrs=['dNSHostName'])
+
+            remote = res[0]['dNSHostName'][0]
+            self._enable_all_repl(remote)
+            if direction == 'inbound':
+                src, dest = remote, dc
+            else:
+                src, dest = dc, remote
+            self._net_drs_replicate(dest, src, forced=True)
+
+    def test_samba_tool_showrepl_summary_all_good(self):
+        """Tests 'samba-tool drs showrepl --summary' command."""
+        # To be sure that all is good we need to force replication
+        # with everyone (because others might have it turned off), and
+        # turn replication on for them in case they suddenly decide to
+        # try again.
+        #
+        # We don't restore them to the non-auto-replication state.
+        samdb1 = self.getSamDB("-H", "ldap://%s" % self.dc1, "-U",
+                               self.cmdline_creds)
+        self._enable_all_repl(self.dc1)
+        self._force_all_reps(samdb1, self.dc1, 'inbound')
+        self._force_all_reps(samdb1, self.dc1, 'outbound')
+
+        out = self.check_output("samba-tool drs showrepl --summary %s %s" %
+                                (self.dc1, self.cmdline_creds))
+        self.assertStringsEqual(out, "[ALL GOOD]\n")
+
+        out = self.check_output("samba-tool drs showrepl --summary "
+                                "--color=yes %s %s" %
+                                (self.dc1, self.cmdline_creds))
+        self.assertStringsEqual(out, "\033[1;32m[ALL GOOD]\033[0m\n")
+
+        # --verbose output is still quiet when all is good.
+        out = self.check_output("samba-tool drs showrepl --summary -v %s %s" %
+                                (self.dc1, self.cmdline_creds))
+        self.assertStringsEqual(out, "[ALL GOOD]\n")
+        out = self.check_output("samba-tool drs showrepl --summary -v "
+                                "--color=yes %s %s" %
+                                (self.dc1, self.cmdline_creds))
+        self.assertStringsEqual(out, "\033[1;32m[ALL GOOD]\033[0m\n")
+
+    def test_samba_tool_showrepl_summary_forced_failure(self):
+        """Tests 'samba-tool drs showrepl --summary' command when we break the
+        network on purpose.
+        """
+        self.addCleanup(self._enable_all_repl, self.dc1)
+        self._disable_all_repl(self.dc1)
+
+        samdb1 = self.getSamDB("-H", "ldap://%s" % self.dc1, "-U",
+                               self.cmdline_creds)
+        samdb2 = self.getSamDB("-H", "ldap://%s" % self.dc2, "-U",
+                               self.cmdline_creds)
+        domain_dn = samdb1.domain_dn()
+
+        # Add some things to NOT replicate
+        ou1 = "OU=dc1.%x,%s" % (random.randrange(1 << 64), domain_dn)
+        ou2 = "OU=dc2.%x,%s" % (random.randrange(1 << 64), domain_dn)
+        samdb1.add({
+            "dn": ou1,
+            "objectclass": "organizationalUnit"
+        })
+        self.addCleanup(samdb1.delete, ou1, ['tree_delete:1'])
+        samdb2.add({
+            "dn": ou2,
+            "objectclass": "organizationalUnit"
+        })
+        self.addCleanup(samdb2.delete, ou2, ['tree_delete:1'])
+
+        dn1 = 'cn=u1.%%d,%s' % (ou1)
+        dn2 = 'cn=u2.%%d,%s' % (ou2)
+
+        try:
+            for i in range(100):
+                samdb1.add({
+                    "dn": dn1 % i,
+                    "objectclass": "user"
+                })
+                samdb2.add({
+                    "dn": dn2 % i,
+                    "objectclass": "user"
+                })
+                out = self.check_output("samba-tool drs showrepl --summary -v "
+                                        "%s %s" %
+                                        (self.dc1, self.cmdline_creds))
+                self.assertStringsEqual('[ALL GOOD]', out, strip=True)
+                out = self.check_output("samba-tool drs showrepl --summary -v "
+                                        "--color=yes %s %s" %
+                                        (self.dc2, self.cmdline_creds))
+                self.assertIn('[ALL GOOD]', out)
+
+        except samba.tests.BlackboxProcessError as e:
+            print("Good, failed as expected after %d rounds: %r" % (i, e.cmd))
+            self.assertIn('There are failing connections', e.stdout)
+            self.assertRegexpMatches(e.stdout,
+                                     r'result 845[67] '
+                                     r'\(WERR_DS_DRA_(SINK|SOURCE)_DISABLED\)',
+                                     msg=("The process should have failed "
+                                          "because replication was forced off, "
+                                          "but it failed for some other reason."))
+            self.assertIn('consecutive failure(s).', e.stdout)
+        else:
+            self.fail("No DRS failure noticed after 100 rounds of trying")
-- 
2.17.1


From ed6a2b3895db1bd390d60f7f27cc5b7e0e6d03c6 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Thu, 7 Jun 2018 14:27:52 +1200
Subject: [PATCH 3/8] samba-tool drs show_repl: simplify the collection of DC
 lists

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 python/samba/netcmd/drs.py | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/python/samba/netcmd/drs.py b/python/samba/netcmd/drs.py
index 613f56c8a4f..1999c1e130d 100644
--- a/python/samba/netcmd/drs.py
+++ b/python/samba/netcmd/drs.py
@@ -18,7 +18,6 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
-
 import samba.getopt as options
 import ldb
 import logging
@@ -147,9 +146,7 @@ class cmd_drs_showrepl(Command):
         self.message("\t\tLast success @ %s" % d['last success'])
         self.message("")
 
-    def drsuapi_ReplicaInfo(self, info_type):
-        '''call a DsReplicaInfo'''
-
+    def get_neighbours(self, info_type):
         req1 = drsuapi.DsReplicaGetInfoRequest1()
         req1.info_type = info_type
         try:
@@ -157,7 +154,9 @@ class cmd_drs_showrepl(Command):
                 self.drsuapi_handle, 1, req1)
         except Exception as e:
             raise CommandError("DsReplicaGetInfo of type %u failed" % info_type, e)
-        return (info_type, info)
+
+        reps = [self.parse_neighbour(n) for n in info.array]
+        return reps
 
     def run(self, DC=None, sambaopts=None,
             credopts=None, versionopts=None,
@@ -240,12 +239,8 @@ class cmd_drs_showrepl(Command):
         }
 
         conn = self.samdb.search(base=ntds_dn, expression="(objectClass=nTDSConnection)")
-        info = self.drsuapi_ReplicaInfo(
-            drsuapi.DRSUAPI_DS_REPLICA_INFO_NEIGHBORS)[1]
-        repsfrom =  [self.parse_neighbour(n) for n in info.array]
-        info = self.drsuapi_ReplicaInfo(
-            drsuapi.DRSUAPI_DS_REPLICA_INFO_REPSTO)[1]
-        repsto = [self.parse_neighbour(n) for n in info.array]
+        repsfrom = self.get_neighbours(drsuapi.DRSUAPI_DS_REPLICA_INFO_NEIGHBORS)
+        repsto = self.get_neighbours(drsuapi.DRSUAPI_DS_REPLICA_INFO_REPSTO)
 
         conn_details = []
         for c in conn:
-- 
2.17.1


From d5b1c7cedbfe550471c3d08c58ef1ed6f1f44db4 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Sun, 10 Jun 2018 21:03:47 +0200
Subject: [PATCH 4/8] samba-tool drs showrepl: Skip deleted DSAs when checking
 for success

The deleted DSAs are ignored by the server replication code, so ignore past failures
here also.

The repsFrom and repsTo entries will eventually be removed by the KCC.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 python/samba/netcmd/drs.py | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/python/samba/netcmd/drs.py b/python/samba/netcmd/drs.py
index 1999c1e130d..0b4b1cce84e 100644
--- a/python/samba/netcmd/drs.py
+++ b/python/samba/netcmd/drs.py
@@ -114,16 +114,28 @@ class cmd_drs_showrepl(Command):
 
     def parse_neighbour(self, n):
         """Convert an ldb neighbour object into a python dictionary"""
+        dsa_objectguid = str(n.source_dsa_obj_guid)
         d = {
             'NC dn': n.naming_context_dn,
-            "DSA objectGUID": str(n.source_dsa_obj_guid),
+            "DSA objectGUID": dsa_objectguid,
             "last attempt time": nttime2string(n.last_attempt),
             "last attempt message": drs_errmsg(n.result_last_attempt),
             "consecutive failures": n.consecutive_sync_failures,
             "last success": nttime2string(n.last_success),
-            "NTDS DN": str(n.source_dsa_obj_dn)
+            "NTDS DN": str(n.source_dsa_obj_dn),
+            'is deleted': False
         }
 
+        try:
+            self.samdb.search(base="<GUID=%s>" % dsa_objectguid,
+                              scope=ldb.SCOPE_BASE,
+                              attrs=[])
+        except ldb.LdbError as e:
+            (errno, _) = e.args
+            if errno == ldb.ERR_NO_SUCH_OBJECT:
+                d['is deleted'] = True
+            else:
+                raise
         try:
             (site, server) = drs_parse_ntds_dn(n.source_dsa_obj_dn)
             d["DSA"] = "%s\%s" % (site, server)
@@ -194,10 +206,14 @@ class cmd_drs_showrepl(Command):
 
         local_data = self.get_local_repl_data()
         for rep in local_data['repsTo']:
+            if rep['is deleted']:
+                continue
             if rep["consecutive failures"] != 0 or rep["last success"] == 0:
                 failing_repsto.append(rep)
 
         for rep in local_data['repsFrom']:
+            if rep['is deleted']:
+                continue
             if rep["consecutive failures"] != 0 or rep["last success"] == 0:
                 failing_repsto.append(rep)
 
-- 
2.17.1


From 0b997130a1765e1ab4926019a79b8c2359e1f3d0 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed, 13 Jun 2018 12:54:57 +1200
Subject: [PATCH 5/8] samba-tool drs showrepl test: remove useless print

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/torture/drs/python/samba_tool_drs_showrepl.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/source4/torture/drs/python/samba_tool_drs_showrepl.py b/source4/torture/drs/python/samba_tool_drs_showrepl.py
index f7a806e660e..939adf9a0e0 100644
--- a/source4/torture/drs/python/samba_tool_drs_showrepl.py
+++ b/source4/torture/drs/python/samba_tool_drs_showrepl.py
@@ -129,8 +129,6 @@ class SambaToolDrsShowReplTests(drs_base.DrsBaseTestCase):
         out = self.check_output("samba-tool drs showrepl %s %s --json" %
                                 (self.dc1, self.cmdline_creds))
 
-        print(out)
-
         d = json.loads(out)
         self.assertEqual(set(d), set(['repsFrom',
                                       'repsTo',
-- 
2.17.1


From be70ed6a146026018890c1a7a64d460f4ba2bfb1 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed, 13 Jun 2018 16:28:55 +1200
Subject: [PATCH 6/8] s4/torture/drs/python: don't double-call enable/disable
 replication

This is repeating work done in setup/teardown or doubling up in place (self._enable_all_repl includes self._enable_inbound_repl)

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 source4/torture/drs/python/repl_schema.py       | 3 ---
 source4/torture/drs/python/replica_sync_rodc.py | 2 --
 2 files changed, 5 deletions(-)

diff --git a/source4/torture/drs/python/repl_schema.py b/source4/torture/drs/python/repl_schema.py
index 61c08be6db9..89346af724d 100644
--- a/source4/torture/drs/python/repl_schema.py
+++ b/source4/torture/drs/python/repl_schema.py
@@ -308,9 +308,6 @@ class DrsReplSchemaTestCase(drs_base.DrsBaseTestCase):
 
         This ensures that the server
         """
-        # disable automatic replication temporary
-        self._disable_all_repl(self.dnsname_dc1)
-        self._disable_all_repl(self.dnsname_dc2)
 
        # add new attributeSchema object
         (a_ldn, a_dn) = self._schema_new_attr(self.ldb_dc1, "attr-OU-S", 14)
diff --git a/source4/torture/drs/python/replica_sync_rodc.py b/source4/torture/drs/python/replica_sync_rodc.py
index 907cef49792..d93c4549647 100644
--- a/source4/torture/drs/python/replica_sync_rodc.py
+++ b/source4/torture/drs/python/replica_sync_rodc.py
@@ -43,14 +43,12 @@ class DrsReplicaSyncTestCase(drs_base.DrsBaseTestCase):
 
     def setUp(self):
         super(DrsReplicaSyncTestCase, self).setUp()
-        self._disable_inbound_repl(self.dnsname_dc1)
         self._disable_all_repl(self.dnsname_dc1)
         self.ou1 = None
         self.ou2 = None
 
     def tearDown(self):
         # re-enable replication
-        self._enable_inbound_repl(self.dnsname_dc1)
         self._enable_all_repl(self.dnsname_dc1)
 
         super(DrsReplicaSyncTestCase, self).tearDown()
-- 
2.17.1


From b6e59e5be3ef4429573475d827dfe248f978faf1 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 25 Jun 2018 14:00:59 +1200
Subject: [PATCH 7/8] provision: set 'binddns dir' when making new smb.conf

When creating a new smb.conf from scratch during a join/clone/etc, the
'binddns dir' setting still uses the source smb.conf/default setting,
instead of the targetdir sub-directory.

I noticed this problem when trying to create a new testenv - the
provision() was trying to create /usr/local/samba/bind-dns directory,
which would fail if samba hadn't already been installed on the host
machine.

Now that this is fixed, we also need to fix tests that were explicitly
asserting that no unexpected directories were left behind after the test
completes.

This change also breaks the upgradeprovision script. The upgrade-
provision calls newprovision() to create a reference provision in a
temporary directory. However, previously this temporary provision was
creating the bind-dns directory in the actual upgrade directory as a
side-effect, e.g. it did a provision() with
targetdir=alpha13_upgrade_full/private/referenceprovisionLBKBh2 and this
ended up creating alpha13_upgrade_full/bind-dns as a side-effect.
The provision() now creates bind-dns in the specified targetdir, but
this means check_for_DNS() fails (it tries to create bind-dns sub-
directories, but the upgrade's bind-dns doesn't exist). I've avoided
this problem by making sure bind-dns exists as part of the
check_for_DNS() processing.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
 python/samba/provision/__init__.py           | 2 ++
 python/samba/tests/join.py                   | 1 +
 python/samba/tests/samdb.py                  | 2 +-
 source4/scripting/bin/samba_upgradeprovision | 3 +++
 source4/torture/drs/python/samba_tool_drs.py | 1 +
 5 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
index 36f50f252ed..e5718949626 100644
--- a/python/samba/provision/__init__.py
+++ b/python/samba/provision/__init__.py
@@ -744,10 +744,12 @@ def make_smbconf(smbconf, hostname, domain, realm, targetdir,
         global_settings["lock dir"] = os.path.abspath(targetdir)
         global_settings["state directory"] = os.path.abspath(os.path.join(targetdir, "state"))
         global_settings["cache directory"] = os.path.abspath(os.path.join(targetdir, "cache"))
+        global_settings["binddns dir"] = os.path.abspath(os.path.join(targetdir, "bind-dns"))
 
         lp.set("lock dir", os.path.abspath(targetdir))
         lp.set("state directory",  global_settings["state directory"])
         lp.set("cache directory", global_settings["cache directory"])
+        lp.set("binddns dir", global_settings["binddns dir"])
 
     if eadb:
         if use_ntvfs and not lp.get("posix:eadb"):
diff --git a/python/samba/tests/join.py b/python/samba/tests/join.py
index 1f9fab1d72a..17de3ab84ce 100644
--- a/python/samba/tests/join.py
+++ b/python/samba/tests/join.py
@@ -73,6 +73,7 @@ class JoinTestCase(DNSTKeyTest):
             shutil.rmtree(os.path.join(self.tempdir, "etc"))
             shutil.rmtree(os.path.join(self.tempdir, "msg.lock"))
             os.unlink(os.path.join(self.tempdir, "names.tdb"))
+            shutil.rmtree(os.path.join(self.tempdir, "bind-dns"))
 
         self.join_ctx.cleanup_old_join(force=True)
 
diff --git a/python/samba/tests/samdb.py b/python/samba/tests/samdb.py
index d4279b4aed4..8c477b291db 100644
--- a/python/samba/tests/samdb.py
+++ b/python/samba/tests/samdb.py
@@ -59,7 +59,7 @@ class SamDBTestCase(TestCaseInTempDir):
         for f in ['names.tdb']:
             os.remove(os.path.join(self.tempdir, f))
 
-        for d in ['etc', 'msg.lock', 'private', 'state']:
+        for d in ['etc', 'msg.lock', 'private', 'state', 'bind-dns']:
             shutil.rmtree(os.path.join(self.tempdir, d))
 
         super(SamDBTestCase, self).tearDown()
diff --git a/source4/scripting/bin/samba_upgradeprovision b/source4/scripting/bin/samba_upgradeprovision
index 9d3e73604a2..5d040d21d66 100755
--- a/source4/scripting/bin/samba_upgradeprovision
+++ b/source4/scripting/bin/samba_upgradeprovision
@@ -226,6 +226,9 @@ def check_for_DNS(refprivate, private, refbinddns_dir, binddns_dir, dns_backend)
     if not os.path.exists(dnsfile):
         shutil.copy("%s/dns_update_list" % refprivate, "%s" % dnsfile)
 
+    if not os.path.exists(binddns_dir):
+        os.mkdir(binddns_dir)
+
     if dns_backend not in ['BIND9_DLZ', 'BIND9_FLATFILE']:
        return
 
diff --git a/source4/torture/drs/python/samba_tool_drs.py b/source4/torture/drs/python/samba_tool_drs.py
index 502c8096603..29c6016471c 100644
--- a/source4/torture/drs/python/samba_tool_drs.py
+++ b/source4/torture/drs/python/samba_tool_drs.py
@@ -47,6 +47,7 @@ class SambaToolDrsTests(drs_base.DrsBaseTestCase):
             shutil.rmtree(os.path.join(self.tempdir, "msg.lock"))
             os.remove(os.path.join(self.tempdir, "names.tdb"))
             shutil.rmtree(os.path.join(self.tempdir, "state"))
+            shutil.rmtree(os.path.join(self.tempdir, "bind-dns"))
         except Exception:
             pass
 
-- 
2.17.1


From b04343693da03ec1fe20f8b42134a154d77a8862 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed, 27 Jun 2018 13:55:16 +1200
Subject: [PATCH 8/8] s4/torture/samba_tool_drs_showrepl: use
 assertRegexpMatches

rather than a local rewrite special to this file.

Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 .../drs/python/samba_tool_drs_showrepl.py     | 94 ++++++++++---------
 1 file changed, 49 insertions(+), 45 deletions(-)

diff --git a/source4/torture/drs/python/samba_tool_drs_showrepl.py b/source4/torture/drs/python/samba_tool_drs_showrepl.py
index 939adf9a0e0..69d2674c2a8 100644
--- a/source4/torture/drs/python/samba_tool_drs_showrepl.py
+++ b/source4/torture/drs/python/samba_tool_drs_showrepl.py
@@ -41,12 +41,6 @@ DN_RE = r'(?:(?:CN|DC)=[\\:\w -]+,)+DC=com'
 class SambaToolDrsShowReplTests(drs_base.DrsBaseTestCase):
     """Blackbox test case for samba-tool drs."""
 
-    def assertRegex(self, exp, s, flags=0):
-        m = re.search(exp, s, flags=flags)
-        if m is None:
-            self.fail("%r did not match /%s/" % (s, exp))
-        return m
-
     def setUp(self):
         super(SambaToolDrsShowReplTests, self).setUp()
 
@@ -88,11 +82,12 @@ class SambaToolDrsShowReplTests(drs_base.DrsBaseTestCase):
         self.assertEqual(_outbound, ' OUTBOUND NEIGHBORS ')
         self.assertEqual(_conn, ' KCC CONNECTION OBJECTS ')
 
-        self.assertRegex(r'^Default-First-Site-Name\\LOCALDC\s+'
-                         r"DSA Options: %s\s+"
-                         r"DSA object GUID: %s\s+"
-                         r"DSA invocationId: %s" %
-                         (HEX8_RE, GUID_RE, GUID_RE), header)
+        self.assertRegexpMatches(header,
+                                 r'^Default-First-Site-Name\\LOCALDC\s+'
+                                 r"DSA Options: %s\s+"
+                                 r"DSA object GUID: %s\s+"
+                                 r"DSA invocationId: %s" %
+                                 (HEX8_RE, GUID_RE, GUID_RE))
 
         # We don't assert the DomainDnsZones and ForestDnsZones are
         # there because we don't know that they have been set up yet.
@@ -100,28 +95,35 @@ class SambaToolDrsShowReplTests(drs_base.DrsBaseTestCase):
         for p in ['CN=Configuration,DC=samba,DC=example,DC=com',
                   'DC=samba,DC=example,DC=com',
                   'CN=Schema,CN=Configuration,DC=samba,DC=example,DC=com']:
-            self.assertRegex(r'%s\n'
-                             r'\tDefault-First-Site-Name\\[A-Z]+ via RPC\n'
-                             r'\t\tDSA object GUID: %s\n'
-                             r'\t\tLast attempt @ [^\n]+\n'
-                             r'\t\t\d+ consecutive failure\(s\).\n'
-                             r'\t\tLast success @ [^\n]+\n'
-                             r'\n' % (p, GUID_RE), inbound)
-
-            self.assertRegex(r'%s\n'
-                             r'\tDefault-First-Site-Name\\[A-Z]+ via RPC\n'
-                             r'\t\tDSA object GUID: %s\n'
-                             r'\t\tLast attempt @ [^\n]+\n'
-                             r'\t\t\d+ consecutive failure\(s\).\n'
-                             r'\t\tLast success @ [^\n]+\n'
-                             r'\n' % (p, GUID_RE), outbound)
-
-        self.assertRegex(r'Connection --\n'
-                         r'\tConnection name: %s\n'
-                         r'\tEnabled        : TRUE\n'
-                         r'\tServer DNS name : \w+.samba.example.com\n'
-                         r'\tServer DN name  : %s'
-                         r'\n' % (GUID_RE, DN_RE), conn)
+            self.assertRegexpMatches(
+                inbound,
+                r'%s\n'
+                r'\tDefault-First-Site-Name\\[A-Z]+ via RPC\n'
+                r'\t\tDSA object GUID: %s\n'
+                r'\t\tLast attempt @ [^\n]+\n'
+                r'\t\t\d+ consecutive failure\(s\).\n'
+                r'\t\tLast success @ [^\n]+\n'
+                r'\n' % (p, GUID_RE),
+                msg="%s inbound missing" % p)
+
+            self.assertRegexpMatches(
+                outbound,
+                r'%s\n'
+                r'\tDefault-First-Site-Name\\[A-Z]+ via RPC\n'
+                r'\t\tDSA object GUID: %s\n'
+                r'\t\tLast attempt @ [^\n]+\n'
+                r'\t\t\d+ consecutive failure\(s\).\n'
+                r'\t\tLast success @ [^\n]+\n'
+                r'\n' % (p, GUID_RE),
+                msg="%s outbound missing" % p)
+
+        self.assertRegexpMatches(conn,
+                                 r'Connection --\n'
+                                 r'\tConnection name: %s\n'
+                                 r'\tEnabled        : TRUE\n'
+                                 r'\tServer DNS name : \w+.samba.example.com\n'
+                                 r'\tServer DN name  : %s'
+                                 r'\n' % (GUID_RE, DN_RE))
 
     def test_samba_tool_showrepl_json(self):
         """Tests 'samba-tool drs showrepl --json' command.
@@ -137,29 +139,30 @@ class SambaToolDrsShowReplTests(drs_base.DrsBaseTestCase):
 
         # dsa
         for k in ["objectGUID", "invocationId"]:
-            self.assertRegex('^%s$' % GUID_RE, d['dsa'][k])
+            self.assertRegexpMatches(d['dsa'][k], '^%s$' % GUID_RE)
         self.assertTrue(isinstance(d['dsa']["options"], int))
 
         # repsfrom and repsto
         for reps in (d['repsFrom'], d['repsTo']):
             for r in reps:
                 for k in ('NC dn', "NTDS DN"):
-                    self.assertRegex('^%s$' % DN_RE, r[k])
+                    self.assertRegexpMatches(r[k], '^%s$' % DN_RE)
                 for k in ("last attempt time",
                           "last attempt message",
                           "last success"):
                     self.assertTrue(isinstance(r[k], json_str))
-                self.assertRegex('^%s$' % GUID_RE, r["DSA objectGUID"])
+                self.assertRegexpMatches(r["DSA objectGUID"], '^%s$' % GUID_RE)
                 self.assertTrue(isinstance(r["consecutive failures"], int))
 
         # ntdsconnection
         for n in d["NTDSConnections"]:
-            self.assertRegex(r'^[\w]+\.samba\.example\.com$', n["dns name"])
-            self.assertRegex("^%s$" % GUID_RE, n["name"])
+            self.assertRegexpMatches(n["dns name"],
+                                     r'^[\w]+\.samba\.example\.com$')
+            self.assertRegexpMatches(n["name"], "^%s$" % GUID_RE)
             self.assertTrue(isinstance(n['enabled'], bool))
             self.assertTrue(isinstance(n['options'], int))
             self.assertTrue(isinstance(n['replicates NC'], list))
-            self.assertRegex("^%s$" % DN_RE, n["remote DN"])
+            self.assertRegexpMatches(n["remote DN"], "^%s$" % DN_RE)
 
     def _force_all_reps(self, samdb, dc, direction):
         if direction == 'inbound':
@@ -292,12 +295,13 @@ class SambaToolDrsShowReplTests(drs_base.DrsBaseTestCase):
         except samba.tests.BlackboxProcessError as e:
             print("Good, failed as expected after %d rounds: %r" % (i, e.cmd))
             self.assertIn('There are failing connections', e.stdout)
-            self.assertRegexpMatches(e.stdout,
-                                     r'result 845[67] '
-                                     r'\(WERR_DS_DRA_(SINK|SOURCE)_DISABLED\)',
-                                     msg=("The process should have failed "
-                                          "because replication was forced off, "
-                                          "but it failed for some other reason."))
+            self.assertRegexpMatches(
+                e.stdout,
+                r'result 845[67] '
+                r'\(WERR_DS_DRA_(SINK|SOURCE)_DISABLED\)',
+                msg=("The process should have failed "
+                     "because replication was forced off, "
+                     "but it failed for some other reason."))
             self.assertIn('consecutive failure(s).', e.stdout)
         else:
             self.fail("No DRS failure noticed after 100 rounds of trying")
-- 
2.17.1



More information about the samba-technical mailing list