[PATCH] Selftest improvements by Jamie and myself

Andrew Bartlett abartlet at samba.org
Wed Mar 21 00:13:20 UTC 2018


G'Day,

Here are some small improvement to our selftest system by Jamie
McClymont.  They aid in the handling of subunit and avoid more of the
cross-environment leaks that have bedevilled us so. 

I also improve our build for --without-ads as I was playing with that
as a build target. 

It has past an autobuild on the catalyst cloud, and is in the tree used
for this successful GitLab CI build:

https://gitlab.com/catalyst-samba/samba/pipelines/19222811

I've reviewed Jamie's patches.

Please can I have a second team review and push!

Andrew Bartlett
-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba



-------------- next part --------------
From 0f32df5e6792cc62c82f12fa5d3bd105c038171d Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Fri, 21 Jul 2017 19:54:36 +1200
Subject: [PATCH 1/8] autobuild: Move defaulttasks to one-per-line

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

diff --git a/script/autobuild.py b/script/autobuild.py
index ebe49bb75cb..fe1a96104fe 100755
--- a/script/autobuild.py
+++ b/script/autobuild.py
@@ -44,7 +44,21 @@ builddirs = {
     "retry"   : "."
     }
 
-defaulttasks = [ "ctdb", "samba", "samba-xc", "samba-o3", "samba-ctdb", "samba-libs", "samba-static", "samba-systemkrb5", "samba-nopython", "ldb", "tdb", "talloc", "replace", "tevent", "pidl" ]
+defaulttasks = [ "ctdb",
+                 "samba",
+                 "samba-xc",
+                 "samba-o3",
+                 "samba-ctdb",
+                 "samba-libs",
+                 "samba-static",
+                 "samba-systemkrb5",
+                 "samba-nopython",
+                 "ldb",
+                 "tdb",
+                 "talloc",
+                 "replace",
+                 "tevent",
+                 "pidl" ]
 
 if os.environ.get("AUTOBUILD_SKIP_SAMBA_O3", "0") == "1":
     defaulttasks.remove("samba-o3")
-- 
2.11.0


From b78e0924753913313981792ea3219e6b28e85250 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Sat, 3 Mar 2018 21:24:47 +1300
Subject: [PATCH 2/8] travis-ci: Only un-shallow for PIDL

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

diff --git a/.travis.yml b/.travis.yml
index 645658b9e7d..9b0a41d4e53 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -30,5 +30,7 @@ before_install:
  - sudo apt-get install --assume-yes acl attr autoconf bind9utils bison build-essential debhelper dnsutils docbook-xml docbook-xsl flex gdb libjansson-dev krb5-user libacl1-dev libaio-dev libarchive-dev libattr1-dev libblkid-dev libbsd-dev libcap-dev libcups2-dev libgnutls-dev libgpgme11-dev libjson-perl libldap2-dev libncurses5-dev libpam0g-dev libparse-yapp-perl libpopt-dev libreadline-dev nettle-dev perl perl-modules pkg-config python-all-dev python-crypto python-dbg python-dev python-dnspython python3-dnspython python-gpgme python3-gpgme python-markdown python3-markdown python3-dev xsltproc zlib1g-dev
 
 script:
- - git fetch --unshallow
+ - if [ $TASK = "pidl" ]; then
+    git fetch --unshallow;
+   fi	
  - ./script/autobuild.py --tail --testbase=/tmp $TASK
-- 
2.11.0


From a8b1a910c960f3bae0fa9f3b84dd905205d9ec26 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Sat, 3 Mar 2018 23:39:24 +1300
Subject: [PATCH 3/8] travis-ci: Use Gold linker for faster builds

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

diff --git a/.travis.yml b/.travis.yml
index 9b0a41d4e53..e80e1225d30 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -28,6 +28,10 @@ matrix:
 before_install:
  - sudo apt-get update -qq
  - sudo apt-get install --assume-yes acl attr autoconf bind9utils bison build-essential debhelper dnsutils docbook-xml docbook-xsl flex gdb libjansson-dev krb5-user libacl1-dev libaio-dev libarchive-dev libattr1-dev libblkid-dev libbsd-dev libcap-dev libcups2-dev libgnutls-dev libgpgme11-dev libjson-perl libldap2-dev libncurses5-dev libpam0g-dev libparse-yapp-perl libpopt-dev libreadline-dev nettle-dev perl perl-modules pkg-config python-all-dev python-crypto python-dbg python-dev python-dnspython python3-dnspython python-gpgme python3-gpgme python-markdown python3-markdown python3-dev xsltproc zlib1g-dev
+ - sudo apt-get install --assume-yes binutils-gold 
+ - sudo update-alternatives --install "/usr/bin/ld" "ld" "/usr/bin/ld.gold" 20
+ - sudo update-alternatives --install "/usr/bin/ld" "ld" "/usr/bin/ld.bfd" 10
+ - sudo update-alternatives --set ld /usr/bin/ld.gold
 
 script:
  - if [ $TASK = "pidl" ]; then
-- 
2.11.0


From 8d43c94e99ee4ceef55fb9304f86f379f306e0e4 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Sun, 4 Mar 2018 22:31:00 +1300
Subject: [PATCH 4/8] libsmb: Use the same #ifdef for is_our_primary_domain()
 as the only caller

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

diff --git a/source3/libsmb/namequery_dc.c b/source3/libsmb/namequery_dc.c
index eb34741defa..d08456590f6 100644
--- a/source3/libsmb/namequery_dc.c
+++ b/source3/libsmb/namequery_dc.c
@@ -32,7 +32,7 @@
  Is this our primary domain ?
 **********************************************************************/
 
-#ifdef HAVE_KRB5
+#ifdef HAVE_ADS
 static bool is_our_primary_domain(const char *domain)
 {
 	int role = lp_server_role();
-- 
2.11.0


From 4c9e6a09e04b51cc8c4a58301caa7a8b027d1c3e Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Sun, 4 Mar 2018 22:57:22 +1300
Subject: [PATCH 5/8] s3-libnet: move rpc_join label into HAVE_ADS block with
 only caller

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 source3/libnet/libnet_join.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source3/libnet/libnet_join.c b/source3/libnet/libnet_join.c
index 5db2ca09ccc..bbe604b729e 100644
--- a/source3/libnet/libnet_join.c
+++ b/source3/libnet/libnet_join.c
@@ -2665,9 +2665,10 @@ static WERROR libnet_DomainJoin(TALLOC_CTX *mem_ctx,
 		DEBUG(5, ("failed to precreate account in ou %s: %s",
 			r->in.account_ou, ads_errstr(ads_status)));
 	}
+ rpc_join:
+
 #endif /* HAVE_ADS */
 
- rpc_join:
 	if ((r->in.join_flags & WKSSVC_JOIN_FLAGS_JOIN_UNSECURE) &&
 	    (r->in.join_flags & WKSSVC_JOIN_FLAGS_MACHINE_PWD_PASSED)) {
 		status = libnet_join_joindomain_rpc_unsecure(mem_ctx, r, cli);
-- 
2.11.0


From 7d08e91dd154d79e8206f1ccc6470eab73e538db Mon Sep 17 00:00:00 2001
From: Jamie McClymont <jamiemcclymont at catalyst.net.nz>
Date: Mon, 29 Jan 2018 18:59:34 +1300
Subject: [PATCH 6/8] selftest: Clear environment before provision

Currently, if an environment is being provisioned after a test which used
ad_member_rfc2307, the provisioning process has all of the following in its
environment:

{
	'DC_NETBIOSNAME'               => 'LOCALDC',
	'DC_PASSWORD'                  => 'locDCpass1',
	'DC_SERVER_IP'                 => '127.0.0.21',
	'DC_SERVER_IPV6'               => 'fd00:0000:0000:0000:0000:0000:5357:5f15',
	'DC_SERVER'                    => 'localdc',
	'DC_USERNAME'                  => 'Administrator',
	'DOMAIN'                       => 'SAMBADOMAIN',
	'LOCAL_PATH'                   => '/.../st/ad_member_rfc2307/share',
	'LOCK_DIR'                     => '/.../st/ad_member_rfc2307/lockdir',
	'NETBIOSNAME'                  => 'RFC2307MEMBER',
	'NMBD_SOCKET_DIR'              => '/.../st/ad_member_rfc2307/nmbd',
	'NSS_WRAPPER_GROUP'            => '/.../st/ad_member_rfc2307/private/group',
	'NSS_WRAPPER_HOSTNAME'         => 'rfc2307member.samba.example.com',
	'NSS_WRAPPER_HOSTS'            => '/.../st/hosts',
	'NSS_WRAPPER_MODULE_FN_PREFIX' => 'winbind',
	'NSS_WRAPPER_MODULE_SO_PATH'   => '/.../bin/default/nsswitch/libnss-wrapper-winbind.so',
	'NSS_WRAPPER_PASSWD'           => '/.../st/ad_member_rfc2307/private/passwd',
	'PASSWORD'                     => 'loCalMemberPass',
	'REALM'                        => 'SAMBA.EXAMPLE.COM',
	'RESOLV_WRAPPER_HOSTS'         => '/.../st/dns_host_file',
	'SELFTEST_WINBINDD_SOCKET_DIR' => '/.../st/ad_member_rfc2307/winbindd',
	'SERVER_IP'                    => '127.0.0.34',
	'SERVER_IPV6'                  => 'fd00:0000:0000:0000:0000:0000:5357:5f22',
	'SERVER'                       => 'RFC2307MEMBER',
	'USERID'                       => '55668',
	'USERNAME'                     => 'jamiemcclymont',
}

Unsurprisingly, some of these can cause issues for the provisioning process, if
a reduced subset of tests is being run which causes the provision to encounter
never-before-seen pairs of adjacent environments.

For example, a run with only
	TESTS='--include-env=vampire_dc --include-env=ad_member_rfc2307'
would fail to start up the vampire_dc with:
	Could not find machine account in secrets database:
	Failed to fetch machine account password from secrets.ldb:
	Could not find entry to match filter:
	'(&(flatname=SAMBADOMAIN)(objectclass=primaryDomain))' base: 'cn=Primary Domains': No such object: dsdb_search at ../source4/dsdb/common/util.c:4641

Signed-off-by: Jamie McClymont <jamiemcclymont at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 selftest/selftest.pl | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/selftest/selftest.pl b/selftest/selftest.pl
index 42c1e62736f..a772613b8e1 100755
--- a/selftest/selftest.pl
+++ b/selftest/selftest.pl
@@ -942,6 +942,19 @@ sub setup_env($$)
 
 	$option = "client" if $option eq "";
 
+	# Initially clear out the environment for the provision, so previous envs'
+	# variables don't leak in. Provisioning steps must explicitly set their
+	# necessary variables when calling out to other executables
+	foreach (@exported_envvars) {
+		unless ($_ == "NSS_WRAPPER_HOSTS" ||
+		        $_ == "RESOLV_WRAPPER_HOSTS")
+		{
+			delete $ENV{$_};
+		}
+	}
+	delete $ENV{SOCKET_WRAPPER_DEFAULT_IFACE};
+	delete $ENV{SMB_CONF_PATH};
+
 	$ENV{KRB5CCNAME} = "FILE:${selftest_krbt_ccache_path}.${envname}/ignore";
 
 	if (defined(get_running_env($envname))) {
-- 
2.11.0


From d9bec8994d01fbfd994d7cf67e86a5b3e157245c Mon Sep 17 00:00:00 2001
From: Jamie McClymont <jamiemcclymont at catalyst.net.nz>
Date: Wed, 10 Jan 2018 13:28:13 +1300
Subject: [PATCH 7/8] s4:selftest: explicitly set NSS/RESOLV_WAPPER_* in
 wait_for_start

These variables were previously set directly on the selftest process
for the purpose of making this ldbsearch call, allowing them to leak
into other environments.

Signed-off-by: Jamie McClymont <jamiemcclymont at catalyst.net.nz>
Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 selftest/target/Samba4.pm | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 842cd1e1bb6..51a175b25e8 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -215,14 +215,6 @@ sub wait_for_start($$)
 
 	# Ensure we have the first RID Set before we start tests.  This makes the tests more reliable.
 	if ($testenv_vars->{SERVER_ROLE} eq "domain controller") {
-		# Add hosts file for name lookups
-		$ENV{NSS_WRAPPER_HOSTS} = $testenv_vars->{NSS_WRAPPER_HOSTS};
-		if (defined($testenv_vars->{RESOLV_WRAPPER_CONF})) {
-			$ENV{RESOLV_WRAPPER_CONF} = $testenv_vars->{RESOLV_WRAPPER_CONF};
-		} else {
-			$ENV{RESOLV_WRAPPER_HOSTS} = $testenv_vars->{RESOLV_WRAPPER_HOSTS};
-		}
-
 		print "waiting for working LDAP and a RID Set to be allocated\n";
 		my $ldbsearch = Samba::bindir_path($self, "ldbsearch");
 		my $count = 0;
@@ -234,7 +226,21 @@ sub wait_for_start($$)
 			$search_dn = "cn=RID Set,cn=$testenv_vars->{NETBIOSNAME},ou=domain controllers,$base_dn";
 		}
 		my $max_wait = 60;
-		my $cmd = "$ldbsearch $testenv_vars->{CONFIGURATION} -H ldap://$testenv_vars->{SERVER} -U$testenv_vars->{USERNAME}%$testenv_vars->{PASSWORD} -s base -b \"$search_dn\"";
+
+		# Add hosts file for name lookups
+		my $cmd = "NSS_WRAPPER_HOSTS='$testenv_vars->{NSS_WRAPPER_HOSTS}' ";
+		if (defined($testenv_vars->{RESOLV_WRAPPER_CONF})) {
+			$cmd .= "RESOLV_WRAPPER_CONF='$testenv_vars->{RESOLV_WRAPPER_CONF}' ";
+		} else {
+			$cmd .= "RESOLV_WRAPPER_HOSTS='$testenv_vars->{RESOLV_WRAPPER_HOSTS}' ";
+		}
+
+		$cmd .= "$ldbsearch ";
+		$cmd .= "$testenv_vars->{CONFIGURATION} ";
+		$cmd .= "-H ldap://$testenv_vars->{SERVER} ";
+		$cmd .= "-U$testenv_vars->{USERNAME}%$testenv_vars->{PASSWORD} ";
+		$cmd .= "-s base ";
+		$cmd .= "-b '$search_dn' ";
 		while (system("$cmd >/dev/null") != 0) {
 			$count++;
 			if ($count > $max_wait) {
-- 
2.11.0


From 4ac0a9f12f2847de9817e353ba1c8a00c39547a0 Mon Sep 17 00:00:00 2001
From: Jamie McClymont <jamiemcclymont at catalyst.net.nz>
Date: Thu, 25 Jan 2018 17:23:06 +1300
Subject: [PATCH 8/8] selftest: consistently produce high-res UTC time

Currently some subunit reporters throughout the codebase provide low-res time,
meaning timestamps jump back and forth in the subunit file. Also, some subunit
reporters produce UTC timestamps while others produce local time. UTC was chosen
as the standard for this commit since all of the timestamps end with a Z (= Zulu
= UTC).

Signed-off-by: Jamie McClymont <jamiemcclymont at catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/torture/subunit.c         | 4 ++--
 selftest/Subunit.pm           | 6 ++++--
 selftest/selftest.pl          | 7 ++++---
 source4/torture/rpc/witness.c | 4 ++--
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/lib/torture/subunit.c b/lib/torture/subunit.c
index 46f1b65053d..deb96ffa3a9 100644
--- a/lib/torture/subunit.c
+++ b/lib/torture/subunit.c
@@ -62,9 +62,9 @@ static void torture_subunit_report_time(struct torture_context *tctx)
 		return;
 	}
 
-	tmp = localtime(&tp.tv_sec);
+	tmp = gmtime(&tp.tv_sec);
 	if (!tmp) {
-		perror("localtime");
+		perror("gmtime");
 		return;
 	}
 
diff --git a/selftest/Subunit.pm b/selftest/Subunit.pm
index 1cc0e721696..07f4b8ff97d 100644
--- a/selftest/Subunit.pm
+++ b/selftest/Subunit.pm
@@ -16,6 +16,7 @@
 
 package Subunit;
 use POSIX;
+use Time::HiRes;
 
 require Exporter;
 @ISA = qw(Exporter);
@@ -43,10 +44,11 @@ sub end_test($$;$)
 	}
 }
 
-sub report_time($)
+sub report_time()
 {
 	my ($time) = @_;
-	my ($sec, $min, $hour, $mday, $mon, $year, $wday, $yday, $isdst) = localtime($time);
+	$time = Time::HiRes::time() unless (defined($time));
+	my ($sec, $min, $hour, $mday, $mon, $year, $wday, $yday, $isdst) = gmtime($time);
 	$sec = ($time - int($time) + $sec);
 	my $msg = sprintf("%f", $sec);
 	if (substr($msg, 1, 1) eq ".") {
diff --git a/selftest/selftest.pl b/selftest/selftest.pl
index a772613b8e1..995abd71a04 100755
--- a/selftest/selftest.pl
+++ b/selftest/selftest.pl
@@ -28,6 +28,7 @@ use lib "$RealBin";
 use Subunit;
 use SocketWrapper;
 use target::Samba;
+use Time::HiRes qw(time);
 
 eval {
 require Time::HiRes;
@@ -150,9 +151,9 @@ sub run_testsuite($$$$$)
 
 	Subunit::start_testsuite($name);
 	Subunit::progress_push();
-	Subunit::report_time(time());
+	Subunit::report_time();
 	system($cmd);
-	Subunit::report_time(time());
+	Subunit::report_time();
 	Subunit::progress_pop();
 
 	if ($? == -1) {
@@ -781,7 +782,7 @@ my $suitestotal = $#todo + 1;
 
 unless ($opt_list) {
 	Subunit::progress($suitestotal);
-	Subunit::report_time(time());
+	Subunit::report_time();
 }
 
 my $i = 0;
diff --git a/source4/torture/rpc/witness.c b/source4/torture/rpc/witness.c
index 4b44e91dbcf..4e7c682a7ee 100644
--- a/source4/torture/rpc/witness.c
+++ b/source4/torture/rpc/witness.c
@@ -790,9 +790,9 @@ static void torture_subunit_report_time(struct torture_context *tctx)
 		return;
 	}
 
-	tmp = localtime(&tp.tv_sec);
+	tmp = gmtime(&tp.tv_sec);
 	if (!tmp) {
-		torture_comment(tctx, "failed to call localtime");
+		torture_comment(tctx, "failed to call gmtime");
 		return;
 	}
 
-- 
2.11.0



More information about the samba-technical mailing list