Merge ~ahasenack/ubuntu/+source/autofs:cosmic-autofs-dep8-1677751 into ubuntu/+source/autofs:ubuntu/devel

Proposed by Andreas Hasenack
Status: Merged
Merge reported by: Andreas Hasenack
Merged at revision: 356af4048fd325f9db21f020aeea283f6b5bc792
Proposed branch: ~ahasenack/ubuntu/+source/autofs:cosmic-autofs-dep8-1677751
Merge into: ubuntu/+source/autofs:ubuntu/devel
Diff against target: 201 lines (+175/-0)
4 files modified
debian/changelog (+8/-0)
debian/tests/control (+7/-0)
debian/tests/nfs-mount (+25/-0)
debian/tests/smb-mount (+135/-0)
Reviewer Review Type Date Requested Status
Christian Ehrhardt  (community) Approve
Canonical Server Pending
Review via email: mp+347853@code.launchpad.net

Description of the change

Given the recent surprises I had with autofs not working and segfaulting, I thought it's time to add some dep8 tests.

This branch adds tests for samba (authenticated and anonymous) and nfs. You can check the branch out and run with autopkgtest. I run like this:
autopkgtest -o ../dep8-output -U -s --apt-pocket=proposed ./ -- qemu /var/lib/adt-images/autopkgtest-cosmic-amd64.img

The full run takes about 10min.

To post a comment you must log in.
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

First of all - I totally like it.
Thanks for holding up the spirit of bugfixing and improvement in these busy times!

Some comments inline already - I'm still testing the code ...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Tests all look good and succeed.

+1 from a Reviews POV.

Now that Bileto is working again I'd recommend to test it there to get full LP+Arch coverage before an upload.

review: Approve
356af40... by Andreas Hasenack

Be a bit more generous with timeouts

Revision history for this message
Andreas Hasenack (ahasenack) :
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

This is blocked in bileto now: https://bileto.ubuntu.com/#/ticket/3293

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

>
> > + if ! testparm -s 2>&1 | grep -qE "^\[${share}\]"; then
> > + echo "Adding [${share}] share"
> > + cat >> /etc/samba/smb.conf <<EOFEOF
> > +[${share}]
>
> Indentation isn't important, it's just aesthetic. What do you mean by
> inline, just a bunch of echo's instead of cat? Or both [${share}] and the
> lines below it in column 1?
>

All moved from column 1 to the column of "cat"+4
But this is style/opinion-only, so only follow if you prefer that as well.
Keep it as-is otherwise please

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

FYI: I kicked the tests in the ticket, if it works (as it did two weeks ago) we should later on have results

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Check the ouput of all tests on all arches, and if you are ok please go on with the upload.
Since we know it is good now you can also suggest to Debian to help them as well.

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

The armhf tests were skipped because there is no vm for it:

smb-mount SKIP Test requires machine-level isolation but testbed does not provide that
nfs-mount SKIP Test requires machine-level isolation but testbed does not provide that

Is it expected to have no vm for armhf? What's the alternative?

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Looks like that's the way it is on armh :)

Please tag and upload, thanks

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index 5efc4bf..db1546d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
1autofs (5.1.2-3ubuntu2) cosmic; urgency=medium
2
3 * Add DEP8 tests (LP: #1677751):
4 - d/tests/control, d/tests/smb-mount: smb automount DEP8 test
5 - d/tests/control, d/tests/nfs-mount: nfs automount DEP8 test
6
7 -- Andreas Hasenack <andreas@canonical.com> Tue, 12 Jun 2018 17:35:31 -0300
8
1autofs (5.1.2-3ubuntu1) cosmic; urgency=medium9autofs (5.1.2-3ubuntu1) cosmic; urgency=medium
210
3 * Merge with Debian unstable (LP: #1775232). Remaining changes:11 * Merge with Debian unstable (LP: #1775232). Remaining changes:
diff --git a/debian/tests/control b/debian/tests/control
4new file mode 10064412new file mode 100644
index 0000000..0058590
--- /dev/null
+++ b/debian/tests/control
@@ -0,0 +1,7 @@
1Tests: smb-mount
2Depends: @, cifs-utils, samba, smbclient
3Restrictions: isolation-machine, needs-root, allow-stderr
4
5Tests: nfs-mount
6Depends: @, nfs-common, nfs-server
7Restrictions: isolation-machine, needs-root, allow-stderr
diff --git a/debian/tests/nfs-mount b/debian/tests/nfs-mount
0new file mode 1006448new file mode 100644
index 0000000..f3ed7c1
--- /dev/null
+++ b/debian/tests/nfs-mount
@@ -0,0 +1,25 @@
1#!/bin/sh
2
3set -x
4set -e
5
6now=$(date --utc)
7result=0
8
9echo "creating /nfspub nfs export"
10mkdir -p /nfspub
11echo "${now} - This is an nfs public export" > /nfspub/nfs-public.txt
12echo "/nfspub *(ro,no_subtree_check)" > /etc/exports
13exportfs -avr
14
15echo "Configuring automount for nfs on /net"
16mkdir -p /etc/auto.master.d
17echo '/net -hosts -intr,soft --timeout=180' > /etc/auto.master.d/net.autofs
18systemctl restart autofs
19
20echo "Accessing the autofs mountpoint"
21timeout 30s grep -qE "${now}.*nfs public" /net/localhost/nfspub/nfs-public.txt || result=$?
22if [ "$result" -ne "0" ]; then
23 echo "test failed, couldn't access /net/localhost/nfspub/nfs-public.txt"
24 exit ${result}
25fi
diff --git a/debian/tests/smb-mount b/debian/tests/smb-mount
0new file mode 10064426new file mode 100644
index 0000000..b9b685b
--- /dev/null
+++ b/debian/tests/smb-mount
@@ -0,0 +1,135 @@
1#!/bin/sh
2
3set -x
4
5add_smb_share() {
6 local share="$1"
7 local sharepath="$2"
8 local public="$3"
9
10 if ! testparm -s 2>&1 | grep -qE "^\[${share}\]"; then
11 echo "Adding [${share}] share"
12 cat >> /etc/samba/smb.conf <<EOFEOF
13[${share}]
14 path = ${sharepath}
15 read only = no
16EOFEOF
17 if [ "${public}" = "yes" ]; then
18 cat >> /etc/samba/smb.conf <<EOFEOF
19 guest ok = yes
20EOFEOF
21 else
22 cat >> /etc/samba/smb.conf <<EOFEOF
23 guest ok = no
24EOFEOF
25 fi
26 systemctl reload smbd.service
27 else
28 echo "No need to add private [${share}] share, continuing."
29 fi
30}
31
32create_user() {
33 local username="$1"
34 local password="$2"
35
36 useradd -m "$username"
37 echo "Setting samba password for the ${username} user"
38 echo "${password}\n${password}" | smbpasswd -s -a ${username}
39}
40
41
42now=$(date --utc)
43result=0
44# Remount everything in case someone is running this interactively
45# and mounts from previous runs and previous usernames are still lingering
46systemctl restart autofs
47
48echo "Setting up a public samba share in /pub"
49mkdir -p /pub
50echo "${now} - This is the public samba share." > /pub/hello-public.txt
51add_smb_share pub /pub yes
52
53echo "Setting up a private samba share in /private"
54mkdir -p /private
55echo "${now} - This is the private samba share." > /private/hello-private.txt
56add_smb_share private /private no
57
58username="smbtest$$"
59password="$$"
60echo "Creating a local test user called ${username}"
61create_user "${username}" "${password}"
62
63echo "Setting up autofs credentials for the private share"
64mkdir -m 0700 -p /etc/creds
65cat > /etc/creds/localhost <<EOFEOF
66username=${username}
67password=${password}
68domain=WORKGROUP
69EOFEOF
70chmod 0600 /etc/creds/localhost
71
72if ! grep -qE "^/cifs" /etc/auto.master; then
73 echo "Configuring autofs for the /cifs mountpoint"
74 echo "/cifs /etc/auto.smb --timeout=180" >> /etc/auto.master
75 systemctl restart autofs
76fi
77
78# We use a trick here. By storing the credentials in /etc/creds/localhost,
79# that means the autofs key is "localhost", and the credentials will be used
80# when accessing /cifs/localhost.
81# If instead we access /cifs/127.0.0.1, since there is no corresponding entry in
82# /etc/creds, that access will be as a guest, i.e., with no credentials
83echo "Testing authenticated share automount"
84timeout 30s grep -qE "${now}.*private" /cifs/localhost/private/hello-private.txt || result=$?
85if [ "$result" -ne "0" ]; then
86 echo "test failed"
87 exit ${result}
88fi
89
90echo "Confirming with smbstatus that an authenticated connection was used"
91output=$(smbstatus 2>&1)
92echo "${output}" | grep -q "${username}" || result=$?
93if [ "${result}" -ne "0" ]; then
94 echo "test failed"
95 echo "smbstatus output:"
96 echo "${smbstatus}"
97 exit "${result}"
98fi
99
100echo "Confirming with mount -t cifs that the filesystem is mounted with authentication"
101output=$(mount -t cifs 2>&1)
102echo "${output}" | grep -qE "on /cifs/localhost/private.*username=${username}" || result=$?
103if [ "${result}" -ne "0" ]; then
104 echo "test failed"
105 echo "mount -t cifs output:"
106 echo "${output}"
107 exit ${result}
108fi
109
110echo "Testing unauthenticated share automount"
111timeout 30s grep -q "${now}.*public" /cifs/127.0.0.1/pub/hello-public.txt || result=$?
112if [ "$result" -ne "0" ]; then
113 echo "test failed"
114 exit ${result}
115fi
116
117echo "Confirming with smbstatus that a guest connection was used"
118output=$(smbstatus 2>&1)
119echo "${output}" | grep -q nobody || result=$?
120if [ "${result}" -ne "0" ]; then
121 echo "test failed"
122 echo "smbstatus output:"
123 echo "${smbstatus}"
124 exit ${result}
125fi
126
127echo "Confirming with mount -t cifs that the filesystem is mounted without authentication"
128output=$(mount -t cifs 2>&1)
129echo "${output}" | grep -qE "on /cifs/127.0.0.1/pub.*sec=none" || result=$?
130if [ "${result}" -ne "0" ]; then
131 echo "test failed"
132 echo "mount -t cifs output:"
133 echo "${output}"
134 exit ${result}
135fi

Subscribers

People subscribed via source and target branches