Merge lp://qastaging/~raharper/curtin/trunk.fix-iscsi-shutdown into lp://qastaging/~curtin-dev/curtin/trunk

Proposed by Ryan Harper
Status: Merged
Merged at revision: 523
Proposed branch: lp://qastaging/~raharper/curtin/trunk.fix-iscsi-shutdown
Merge into: lp://qastaging/~curtin-dev/curtin/trunk
Diff against target: 298 lines (+227/-5)
4 files modified
curtin/block/iscsi.py (+35/-3)
curtin/commands/install.py (+7/-2)
tests/unittests/test_block_iscsi.py (+181/-0)
tests/vmtests/test_lvm_iscsi.py (+4/-0)
To merge this branch: bzr merge lp://qastaging/~raharper/curtin/trunk.fix-iscsi-shutdown
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
Chad Smith Approve
Nish Aravamudan Pending
Review via email: mp+329777@code.qastaging.launchpad.net

Description of the change

iscsi: use curtin storage config to disconnect iscsi targets

When curtin disconnects from iscsi targets after an install, it does so
after unmounting the targets, however it passed the target dir into the
iscsi disconnect code which then silently failed to find any
session configurations.

We've been lucky as the existing shutdown iscsi service has been
logging out of sessions defined on the host; however on recent
Artful systems, the open-iscsi service is not restarted after
populating /etc/iscsi/nodes directory and since it is not running
in the ephemeral environment the service will not automatically
stop the iscsi devices, causing a hang on shutdown.

To resolve this parse the curtin storage config for iscsi disks,
construct IscsiDisk objects and then call their disconnect() method.
In addition to this logic fix, this branch contains:

- adds a missing Artful LVM Iscsi vmtest class
- Updated logging messages in iscsi paths, including
  a message when iscsi_disconnect doesn't find the
  /etc/iscsi/nodes directory.
- Updated IscsiDisk disconnect with similar logging

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
524. By Ryan Harper

iscsi: disconnect prior to unmounting target

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
525. By Ryan Harper

Check if target is in active session, else skip disconnect

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
526. By Ryan Harper

Add unittests for iscsi.disconnect_target_disks

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
527. By Ryan Harper

Don't rely on ephemeral env, use target iscsi config, disconnect before unmount

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Ryan Harper (raharper) wrote :
Revision history for this message
Scott Moser (smoser) wrote :

Nish,
could you think about this ?
the big change Ryan is making is moving disconnect to happen before unmount rather than after.

that would seem like it could be an issue to me.. if you disconnet a volume and then unmount it could reasonably expect to flush data to that volume.

528. By Ryan Harper

Switch back to unmounting all paths, and then disconnecting iscsi

In general, the typical cleanup of an in-use of block device involves
unmounting the filesystem/block device, and then perform any block
device specific actions. In this case, curtin will recursively unmount
any devices mounted under the install target directory (iscsi disks included)
And then we query the curtin iscsi layer for any connected iscsi devices it
created and invoke disconnect on each device. This should avoid shutting down
any other sessions which may be active but are not created/owned by curtin.

In addition, add some logging to some of the iscsi paths which previously
would silently exit without performing the requested action (disconnect for
example returns if the target iscsi node dir is not present).

529. By Ryan Harper

Don't use iscsi globals, not available across stages

The iscsi global is not available across stages due to use of
subprocess to run curtin commands. Revert back to finding
on-disk sessions that curtin configured to run iscsi disconnects.

Revision history for this message
Ryan Harper (raharper) wrote :
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
530. By Ryan Harper

Parse storage config for reconstructing iscsi disk objects for disconnect operations

531. By Ryan Harper

config is a list

532. By Ryan Harper

Clean up logging messages

533. By Ryan Harper

Add unittest for parsing storage config for iscsi disks

Revision history for this message
Ryan Harper (raharper) wrote :

This branch ran through vmtest running iscsi tests (passes).

https://jenkins.ubuntu.com/server/view/curtin/job/curtin-vmtest-devel-amd64/38/console

Revision history for this message
Chad Smith (chad.smith) wrote :

Looks really good thanks for this (and all the unit tests give me warm fuzzies). +1

Revision history for this message
Ryan Harper (raharper) :
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Chad Smith (chad.smith) wrote :

+1 on this thanks for the comments/discussion

review: Approve
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches