Merge ~smoser/ubuntu/+source/open-iscsi:bug/1785108-bionic-net-interface-handler-runs-always into ubuntu/+source/open-iscsi:ubuntu/bionic-devel
- Git
- lp:~smoser/ubuntu/+source/open-iscsi
- bug/1785108-bionic-net-interface-handler-runs-always
- Merge into ubuntu/bionic-devel
Proposed by
Scott Moser
Status: | Merged |
---|---|
Merged at revision: | fdb1b8c3131415fffc202e1922e723f16f66525e |
Proposed branch: | ~smoser/ubuntu/+source/open-iscsi:bug/1785108-bionic-net-interface-handler-runs-always |
Merge into: | ubuntu/+source/open-iscsi:ubuntu/bionic-devel |
Diff against target: |
273 lines (+99/-19) 7 files modified
debian/changelog (+8/-0) debian/net-interface-handler (+18/-4) debian/tests/README-boot-test.md (+10/-5) debian/tests/patch-image (+44/-5) debian/tests/test-open-iscsi.py (+15/-1) debian/tests/tgt-boot-test (+2/-2) debian/tests/xkvm (+2/-2) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
git-ubuntu developers | Pending | ||
Review via email: mp+352757@code.launchpad.net |
Commit message
Description of the change
see commit message
To post a comment you must log in.
Revision history for this message
Scott Moser (smoser) wrote : | # |
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/debian/changelog b/debian/changelog | |||
2 | index 83773dc..b660fef 100644 | |||
3 | --- a/debian/changelog | |||
4 | +++ b/debian/changelog | |||
5 | @@ -1,3 +1,11 @@ | |||
6 | 1 | open-iscsi (2.0.874-5ubuntu2.1) bionic; urgency=medium | ||
7 | 2 | |||
8 | 3 | * d/tests: pull back cloud image test updates from cosmic. | ||
9 | 4 | * d/net-interface-handler: Apply changes only for the iscsi-root | ||
10 | 5 | (LP: #1785108) | ||
11 | 6 | |||
12 | 7 | -- Scott Moser <smoser@ubuntu.com> Wed, 08 Aug 2018 09:09:02 -0400 | ||
13 | 8 | |||
14 | 1 | open-iscsi (2.0.874-5ubuntu2) bionic; urgency=medium | 9 | open-iscsi (2.0.874-5ubuntu2) bionic; urgency=medium |
15 | 2 | 10 | ||
16 | 3 | * debian/tests: | 11 | * debian/tests: |
17 | diff --git a/debian/net-interface-handler b/debian/net-interface-handler | |||
18 | index 7717f61..15d35c6 100755 | |||
19 | --- a/debian/net-interface-handler | |||
20 | +++ b/debian/net-interface-handler | |||
21 | @@ -10,12 +10,23 @@ | |||
22 | 10 | # ifupdown appears to have no way to do this without also running | 10 | # ifupdown appears to have no way to do this without also running |
23 | 11 | # /etc/network/*.d/ scripts. | 11 | # /etc/network/*.d/ scripts. |
24 | 12 | 12 | ||
25 | 13 | assert_interface() { | ||
26 | 14 | # udev sets INTERFACE to the name of the currently-processed nic. | ||
27 | 15 | [ -n "$INTERFACE" ] && return 0 | ||
28 | 16 | echo "environment variable INTERFACE not set." 1>&2; | ||
29 | 17 | return 1 | ||
30 | 18 | } | ||
31 | 19 | |||
32 | 13 | start() { | 20 | start() { |
33 | 14 | CR=" | 21 | CR=" |
34 | 15 | " | 22 | " |
35 | 23 | assert_interface || return | ||
36 | 16 | ifile=/run/initramfs/open-iscsi.interface | 24 | ifile=/run/initramfs/open-iscsi.interface |
39 | 17 | if [ -f "$ifile" ] && read iface < "$ifile" && | 25 | |
40 | 18 | ! grep -qs "^$iface=" /run/network/ifstate; then | 26 | [ -f "$ifile" ] && read iface < "$ifile" || return 0 |
41 | 27 | [ "$INTERFACE" = "$iface" ] || return | ||
42 | 28 | |||
43 | 29 | if ! grep -qs "^$iface=" /run/network/ifstate; then | ||
44 | 19 | mkdir -p /run/network | 30 | mkdir -p /run/network |
45 | 20 | echo "$iface=$iface" >>/run/network/ifstate | 31 | echo "$iface=$iface" >>/run/network/ifstate |
46 | 21 | 32 | ||
47 | @@ -51,9 +62,12 @@ EOF | |||
48 | 51 | } | 62 | } |
49 | 52 | 63 | ||
50 | 53 | stop() { | 64 | stop() { |
51 | 65 | assert_interface || return | ||
52 | 54 | ifile=/run/initramfs/open-iscsi.interface | 66 | ifile=/run/initramfs/open-iscsi.interface |
55 | 55 | if [ -f "$ifile" ] && read iface < "$ifile" && | 67 | [ -f "$ifile" ] && read iface < "$ifile" || return 0 |
56 | 56 | grep -qs "^$iface=" /run/network/ifstate; then | 68 | [ "$INTERFACE" = "$iface" ] || return |
57 | 69 | |||
58 | 70 | if grep -qs "^$iface=" /run/network/ifstate; then | ||
59 | 57 | grep -v "^$iface=" /run/network/ifstate >/run/network/.ifstate.tmp || true | 71 | grep -v "^$iface=" /run/network/ifstate >/run/network/.ifstate.tmp || true |
60 | 58 | mv /run/network/.ifstate.tmp /run/network/ifstate | 72 | mv /run/network/.ifstate.tmp /run/network/ifstate |
61 | 59 | 73 | ||
62 | diff --git a/debian/tests/README-boot-test.md b/debian/tests/README-boot-test.md | |||
63 | index 526090c..e45bbf0 100644 | |||
64 | --- a/debian/tests/README-boot-test.md | |||
65 | +++ b/debian/tests/README-boot-test.md | |||
66 | @@ -44,7 +44,7 @@ The test case in `debian/tests/test-open-iscsi.py` uses some helper tools. | |||
67 | 44 | using the updated initramfs we wouldn't really be testing the new | 44 | using the updated initramfs we wouldn't really be testing the new |
68 | 45 | open-iscsi. | 45 | open-iscsi. |
69 | 46 | 46 | ||
71 | 47 | It will upgrade any packages inside that are mentioned in | 47 | It will upgrade any packages inside that are mentioned in |
72 | 48 | ADT_TEST_TRIGGERS environment. It will also install open-iscsi if | 48 | ADT_TEST_TRIGGERS environment. It will also install open-iscsi if |
73 | 49 | it is not in that list. | 49 | it is not in that list. |
74 | 50 | 50 | ||
75 | @@ -53,6 +53,9 @@ The test case in `debian/tests/test-open-iscsi.py` uses some helper tools. | |||
76 | 53 | repos into the target. This is necessary for the autopackage test | 53 | repos into the target. This is necessary for the autopackage test |
77 | 54 | environment that adds local package repositories to sources.list.d. | 54 | environment that adds local package repositories to sources.list.d. |
78 | 55 | 55 | ||
79 | 56 | It may also make other changes to the image to workaround bugs. | ||
80 | 57 | See --help output for list of bugs. | ||
81 | 58 | |||
82 | 56 | * **get-image**: This downloads an image from cloud-images.ubuntu.com. See | 59 | * **get-image**: This downloads an image from cloud-images.ubuntu.com. See |
83 | 57 | its Usage for more information. it downloads via stream data and verifies | 60 | its Usage for more information. it downloads via stream data and verifies |
84 | 58 | download. One thing to note is that it does not overwrite existing files. | 61 | download. One thing to note is that it does not overwrite existing files. |
85 | @@ -82,14 +85,16 @@ Testing manually looks like this: | |||
86 | 82 | ## Get the image you want. This creates out.d/disk.img and disk.img.dist | 85 | ## Get the image you want. This creates out.d/disk.img and disk.img.dist |
87 | 83 | $ get-image xenial.d xenial | 86 | $ get-image xenial.d xenial |
88 | 84 | 87 | ||
91 | 85 | ## patch the image with an open-iscsi, which creates xenial/kernel | 88 | ## patch the image with an open-iscsi, which creates xenial.d/kernel |
92 | 86 | ## and xenial/initrd from the kernel and initramfs inside the image. | 89 | ## and xenial.d/initrd from the kernel and initramfs inside the image. |
93 | 87 | $ apt-get download open-iscsi | 90 | $ apt-get download open-iscsi |
94 | 88 | $ deb=$(ls open-iscsi_*.deb | tail -n 1) | 91 | $ deb=$(ls open-iscsi_*.deb | tail -n 1) |
96 | 89 | $ patch-image xenial/disk.img "$deb" --kernel=xenial/kernel --initrd=xenial/initrd | 92 | $ sudo ./debian/tests/patch-image \ |
97 | 93 | --kernel=xenial.d/kernel --initrd=xenial.d/initrd | ||
98 | 94 | xenial.d/disk.img "$deb" | ||
99 | 90 | 95 | ||
100 | 91 | ## Boot the system, log in, look around. | 96 | ## Boot the system, log in, look around. |
102 | 92 | $ tgt-boot-test -v xenial/disk.img xenial/kernel xenial/initrd | 97 | $ tgt-boot-test -v xenial.d/disk.img xenial.d/kernel xenial.d/initrd |
103 | 93 | 98 | ||
104 | 94 | 99 | ||
105 | 95 | ### Features of tgt-boot-test ### | 100 | ### Features of tgt-boot-test ### |
106 | diff --git a/debian/tests/patch-image b/debian/tests/patch-image | |||
107 | index caba3da..b9b34e3 100755 | |||
108 | --- a/debian/tests/patch-image | |||
109 | +++ b/debian/tests/patch-image | |||
110 | @@ -11,9 +11,18 @@ Usage() { | |||
111 | 11 | cat <<EOF | 11 | cat <<EOF |
112 | 12 | Usage: ${0##*/} [options] image [packages] | 12 | Usage: ${0##*/} [options] image [packages] |
113 | 13 | Patch image, upgrading [packages]. | 13 | Patch image, upgrading [packages]. |
117 | 14 | --kernel FILE copy kernel out to FILE | 14 | --kernel FILE copy kernel out to FILE |
118 | 15 | --initrd FILE copy initrd out to FILE | 15 | --initrd FILE copy initrd out to FILE |
119 | 16 | --no-copy-apt do not copy apt repos in. | 16 | |
120 | 17 | --krd-only only copy out kernel/initrd do not change image. | ||
121 | 18 | this is incompatible with 'packages' | ||
122 | 19 | it is a short-cut to specifying all the '--no-*' | ||
123 | 20 | options below. | ||
124 | 21 | |||
125 | 22 | --no-copy-apt do not copy host's apt repos in. | ||
126 | 23 | |||
127 | 24 | image modifications: | ||
128 | 25 | --no-update-fstab do not modify fstab (LP: #1732028) | ||
129 | 17 | 26 | ||
130 | 18 | if no packages are provided, and ADT_TEST_TRIGGERS is set | 27 | if no packages are provided, and ADT_TEST_TRIGGERS is set |
131 | 19 | in environment, then it will be read for the list of packages. | 28 | in environment, then it will be read for the list of packages. |
132 | @@ -70,6 +79,17 @@ bin_packages_from_source_pkg() { | |||
133 | 70 | _RET=$(set -f; for i in ${ret}; do echo "$i"; done | sort -u) | 79 | _RET=$(set -f; for i in ${ret}; do echo "$i"; done | sort -u) |
134 | 71 | } | 80 | } |
135 | 72 | 81 | ||
136 | 82 | update_fstab() { | ||
137 | 83 | # update_fstab(path) modify the fstab at path | ||
138 | 84 | # to remove problematic entries (LP: #1732028) | ||
139 | 85 | local fstab="$1" | ||
140 | 86 | if [ ! -e "$fstab.patch-image-dist" ]; then | ||
141 | 87 | cp "$fstab" "$fstab.patch-image-dist" || | ||
142 | 88 | { error "failed backing up $fstab to $fstab.dist"; return 1; } | ||
143 | 89 | fi | ||
144 | 90 | sed -i '/^LABEL=UEFI/s/^/#/' "$fstab" | ||
145 | 91 | } | ||
146 | 92 | |||
147 | 73 | main() { | 93 | main() { |
148 | 74 | local short_opts="hv" | 94 | local short_opts="hv" |
149 | 75 | local long_opts="help,no-copy-apt,initrd:,kernel:,verbose" | 95 | local long_opts="help,no-copy-apt,initrd:,kernel:,verbose" |
150 | @@ -81,13 +101,16 @@ main() { | |||
151 | 81 | { bad_Usage; return; } | 101 | { bad_Usage; return; } |
152 | 82 | 102 | ||
153 | 83 | local cur="" next="" | 103 | local cur="" next="" |
155 | 84 | local kernel="" initrd="" copy_apt=true | 104 | local kernel="" initrd="" copy_apt=true krd_only=false |
156 | 105 | local update_fstab=true | ||
157 | 85 | 106 | ||
158 | 86 | while [ $# -ne 0 ]; do | 107 | while [ $# -ne 0 ]; do |
159 | 87 | cur="$1"; next="$2"; | 108 | cur="$1"; next="$2"; |
160 | 88 | case "$cur" in | 109 | case "$cur" in |
161 | 89 | -h|--help) Usage ; exit 0;; | 110 | -h|--help) Usage ; exit 0;; |
162 | 111 | --krd-only) krd_only=true; shift;; | ||
163 | 90 | --no-copy-apt) copy_apt=false; shift;; | 112 | --no-copy-apt) copy_apt=false; shift;; |
164 | 113 | --no-update-fstab) update_fstab=false; shift;; | ||
165 | 91 | --kernel) kernel=$next; shift;; | 114 | --kernel) kernel=$next; shift;; |
166 | 92 | --initrd) initrd=$next; shift;; | 115 | --initrd) initrd=$next; shift;; |
167 | 93 | -v|--verbose) VERBOSITY=$((${VERBOSITY}+1));; | 116 | -v|--verbose) VERBOSITY=$((${VERBOSITY}+1));; |
168 | @@ -121,6 +144,17 @@ main() { | |||
169 | 121 | local packages=( ) | 144 | local packages=( ) |
170 | 122 | packages=( "$@" ) | 145 | packages=( "$@" ) |
171 | 123 | 146 | ||
172 | 147 | if $krd_only; then | ||
173 | 148 | if [ "${#packages[@]}" != 0 -a "${packages[*]}" != "none" ]; then | ||
174 | 149 | error "--krd-only is incompatible with packages." | ||
175 | 150 | return 1 | ||
176 | 151 | fi | ||
177 | 152 | debug 1 "--krd-only provided no changes will be done." | ||
178 | 153 | copy_apt=false | ||
179 | 154 | update_fstab=false | ||
180 | 155 | packages=( "none" ) | ||
181 | 156 | fi | ||
182 | 157 | |||
183 | 124 | if [ "${#packages[@]}" = "1" -a "${packages[0]}" = "none" ]; then | 158 | if [ "${#packages[@]}" = "1" -a "${packages[0]}" = "none" ]; then |
184 | 125 | packages=( ) | 159 | packages=( ) |
185 | 126 | elif [ "${#packages[@]}" -eq 0 -a -n "${ADT_TEST_TRIGGERS}" ]; then | 160 | elif [ "${#packages[@]}" -eq 0 -a -n "${ADT_TEST_TRIGGERS}" ]; then |
186 | @@ -185,8 +219,13 @@ main() { | |||
187 | 185 | done | 219 | done |
188 | 186 | fi | 220 | fi |
189 | 187 | 221 | ||
190 | 222 | if $update_fstab; then | ||
191 | 223 | update_fstab "$target/etc/fstab" || | ||
192 | 224 | { error "failed updating /etc/fstab in target"; return 1; } | ||
193 | 225 | fi | ||
194 | 226 | |||
195 | 188 | mount -o bind /sys "$target"/sys || : | 227 | mount -o bind /sys "$target"/sys || : |
197 | 189 | mount -o bind /proc "$target"/proc || : | 228 | mount -o bind /proc "$target"/proc || : |
198 | 190 | mount -o bind /dev "$target"/dev || : | 229 | mount -o bind /dev "$target"/dev || : |
199 | 191 | mount -o bind /dev/pts "$target"/dev/pts || : | 230 | mount -o bind /dev/pts "$target"/dev/pts || : |
200 | 192 | 231 | ||
201 | diff --git a/debian/tests/test-open-iscsi.py b/debian/tests/test-open-iscsi.py | |||
202 | index e7e37a8..27a5e0b 100644 | |||
203 | --- a/debian/tests/test-open-iscsi.py | |||
204 | +++ b/debian/tests/test-open-iscsi.py | |||
205 | @@ -75,8 +75,19 @@ bukket: | |||
206 | 75 | exit | 75 | exit |
207 | 76 | fi | 76 | fi |
208 | 77 | systemd-resolve --status --no-pager | 77 | systemd-resolve --status --no-pager |
209 | 78 | - &add_and_remove_tuntap | | ||
210 | 79 | #!/bin/sh | ||
211 | 80 | # LP: #1785108 would break dns when any device was removed. | ||
212 | 81 | tapdev="mytap0" | ||
213 | 82 | echo ==== Adding $tapdev ==== | ||
214 | 83 | ip tuntap add mode tap user root $tapdev | ||
215 | 84 | udevadm settle | ||
216 | 85 | echo ==== Removing $tapdev ==== | ||
217 | 86 | ip tuntap del mode tap $tapdev | ||
218 | 87 | udevadm settle | ||
219 | 78 | 88 | ||
220 | 79 | runcmd: | 89 | runcmd: |
221 | 90 | - [ sh, -c, *add_and_remove_tuntap ] | ||
222 | 80 | - [ mkdir, -p, /output ] | 91 | - [ mkdir, -p, /output ] |
223 | 81 | - [ cp, /etc/resolv.conf, /output] | 92 | - [ cp, /etc/resolv.conf, /output] |
224 | 82 | - [ sh, -c, *get_resolved_status, --, /output/systemd-resolve-status.txt] | 93 | - [ sh, -c, *get_resolved_status, --, /output/systemd-resolve-status.txt] |
225 | @@ -271,7 +282,10 @@ class CloudImageTest(testlib.TestlibCase, PrivateOpenIscsiTest): | |||
226 | 271 | self.info['initrd']] | 282 | self.info['initrd']] |
227 | 272 | sys.stderr.write(' '.join(cmd) + "\n") | 283 | sys.stderr.write(' '.join(cmd) + "\n") |
228 | 273 | 284 | ||
230 | 274 | subprocess.check_call(cmd) | 285 | env = os.environ.copy() |
231 | 286 | env['BOOT_TIMEOUT'] = env.get('BOOT_TIMEOUT', '60m') | ||
232 | 287 | subprocess.check_call(cmd, env=env) | ||
233 | 288 | |||
234 | 275 | files = self.extract_files(output_disk) | 289 | files = self.extract_files(output_disk) |
235 | 276 | print("files: %s" % files) | 290 | print("files: %s" % files) |
236 | 277 | resolvconf = files.get('resolv.conf', "NO_RESOLVCONF_FOUND") | 291 | resolvconf = files.get('resolv.conf', "NO_RESOLVCONF_FOUND") |
237 | diff --git a/debian/tests/tgt-boot-test b/debian/tests/tgt-boot-test | |||
238 | index 2f8bff3..6ed7a9b 100755 | |||
239 | --- a/debian/tests/tgt-boot-test | |||
240 | +++ b/debian/tests/tgt-boot-test | |||
241 | @@ -514,8 +514,8 @@ main() { | |||
242 | 514 | local mem="512" start="$SECONDS" ret="" | 514 | local mem="512" start="$SECONDS" ret="" |
243 | 515 | local cmd="" | 515 | local cmd="" |
244 | 516 | cmd=( | 516 | cmd=( |
247 | 517 | timeout --kill-after=1m --signal=TERM ${BOOT_TIMEOUT:-60m} | 517 | ${BOOT_TIMEOUT:+timeout --kill-after=1m --signal=TERM $BOOT_TIMEOUT} |
248 | 518 | xkvm "${netdev_args[@]}" ${overlay_drive_xkvm} "${pt[@]}" -- | 518 | xkvm -v -v "${netdev_args[@]}" ${overlay_drive_xkvm} "${pt[@]}" -- |
249 | 519 | -echr 0x5 | 519 | -echr 0x5 |
250 | 520 | -m $mem ${serial_log:+-serial "file:$serial_log"} -nographic | 520 | -m $mem ${serial_log:+-serial "file:$serial_log"} -nographic |
251 | 521 | -kernel "${t_kernel}" -initrd "${t_initrd}" | 521 | -kernel "${t_kernel}" -initrd "${t_initrd}" |
252 | diff --git a/debian/tests/xkvm b/debian/tests/xkvm | |||
253 | index 9015140..9513723 100755 | |||
254 | --- a/debian/tests/xkvm | |||
255 | +++ b/debian/tests/xkvm | |||
256 | @@ -292,7 +292,7 @@ get_bios_opts() { | |||
257 | 292 | 292 | ||
258 | 293 | if [ -n "$nvram" ]; then | 293 | if [ -n "$nvram" ]; then |
259 | 294 | if [ ! -f "$nvram" ]; then | 294 | if [ ! -f "$nvram" ]; then |
261 | 295 | cp "$nvram_src" "$nvram" || | 295 | cp "$nvram_src" "$nvram" || |
262 | 296 | { error "failed copy $nvram_src to $nvram"; return 1; } | 296 | { error "failed copy $nvram_src to $nvram"; return 1; } |
263 | 297 | debug 1 "copied $nvram_src to $nvram" | 297 | debug 1 "copied $nvram_src to $nvram" |
264 | 298 | fi | 298 | fi |
265 | @@ -664,7 +664,7 @@ main() { | |||
266 | 664 | 664 | ||
267 | 665 | local bus_devices | 665 | local bus_devices |
268 | 666 | bus_devices=( -device "$virtio_scsi_bus,id=virtio-scsi-xkvm" ) | 666 | bus_devices=( -device "$virtio_scsi_bus,id=virtio-scsi-xkvm" ) |
270 | 667 | cmd=( "${kvmcmd[@]}" "${archopts[@]}" | 667 | cmd=( "${kvmcmd[@]}" "${archopts[@]}" |
271 | 668 | "${bios_opts[@]}" | 668 | "${bios_opts[@]}" |
272 | 669 | "${bus_devices[@]}" | 669 | "${bus_devices[@]}" |
273 | 670 | "${netargs[@]}" | 670 | "${netargs[@]}" |
I've just uploaded this to the queue, mostly just pushed a MP for easier review andlinking to the bug.