Merge ~paul-meyer/cloud-init:fix-ntfs-mount into cloud-init:master

[Paul Meyer]

Azure: Ignore NTFS mount errors when checking ephemeral drive

The Azure data source provides a method to check whether a NTFS partition
on the ephemeral disk is safe for reformatting to ext4. The method checks
to see if there are customer data files on the disk. However, mounting
the partition fails on systems that do not have the capability of
mounting NTFS. Note that in this case, it is also very unlikely that the
NTFS partition would have been used by the system (since it can't mount
it). The only case would be where an update to the system removed the
capability to mount NTFS, the likelihood of which is also very small.
This change allows the reformatting of the ephemeral disk to ext4 on
systems where mounting NTFS is not supported.

The change converts a specific mount error to success in the method cloudinit.sources.DataSourceAzure.can_dev_be_reformatted.

Rationale:
The Azure data source provides a method to check whether a NTFS partition no the ephemeral disk is safe for reformatting to ext4. The method checks to see if there are customer data files on the disk. However, mounting the partition fails on systems that do not have the capability of mounting NTFS. Note that in this case, it is also very unlikely that the NTFS partition would have been used by the system (since is can't mount it). The only case would be where an update to the system removed the capability to mount NTFS, the likelihood of which is also very small.
This change allows the reformatting of the ephemeral disk to ext4 on systems where mounting NTFS is not supported.

Testing:
1. On Azure, boot a VM from image RedHat:RHEL:7-RAW-CI:latest. Verify that /dev/sdb1 is formatted ext4.
2. Update cloud-init (sudo yum update cloud-init)
3. Choose redeploy from the portal (or deallocate and restart VM).
Expected: /dev/sdb1 is formatted ext4
Actual: /dev/sdb1 is formatted ntfs

When this patch is applied between step 2 and 3, the expected result is achieved.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:7c44b975ee546df18910b620f920d673046c4080
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1063/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1063/rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

Just adding the comments we had during IRC discussion:

I'd prefer if we detected that we can't mount ntfs by checking for

a) is there an ntfs kernel module
b) is there a mount.ntfs tool in $PATH

vs. ignoring the error on certain distros;

The general worry is that the NTFS partition could have user-data on it; though I understand that for some distros which don't support NTFS that data wouldn't have come from instances running those distros.

It was also mentioned that images which don't support ntfs, could embed cloud-config, like:

"support_ntfs: false"

That could also be metadata that the AzureDatasource uses to determine if the mount failure is fatal or not.

0f62c80... by Paul Meyer on 2018-04-30

Add _can_mount_ntfs

f7adfcb... by Paul Meyer on 2018-04-30

Only allow NTFS errors to be ignored if cloudinit agrees NTFS is not mountable

[Paul Meyer (paul-meyer)]

> I'd prefer if we detected that we can't mount ntfs by checking for
>
> a) is there an ntfs kernel module
> b) is there a mount.ntfs tool in $PATH
These checks are implemented now.

> It was also mentioned that images which don't support ntfs, could embed cloud-
> config, like:
>
> "support_ntfs: false"
>
> That could also be metadata that the AzureDataSource uses to determine if the
> mount failure is fatal or not.
We don't need this right now, so I left it out.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:f7adfcb61dd194a1f040d061fef301272c47bb43
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1081/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1081/rebuild

review: Needs Fixing (continuous-integration)

dfd94a6... by Paul Meyer on 2018-05-01

fixing up tests

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:dfd94a6af819845c28ab5557f1261ff1a58b1414
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1082/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1082/rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

I think we can narrow down the check to just: 1) is the module loaded 2) do we have a mount.ntfs

I'll ask smoser to comment w.r.t the case where we have mount.ntfs (and module is present but not loaded, the modinfo case returns true) if that's something we care about; unless mount.ntfs will attempt to modprobe the ntfs module then cloud-init isn't currently modprobing that before calling mount.

[Paul Meyer (paul-meyer)]

> unless mount.ntfs will attempt to modprobe the ntfs module then
> cloud-init isn't currently modprobing that before calling mount.

My experience is that if a mount helper needs a module it will modprobe it. However, I can't point to any doc that says that is the contract. I'll wait for you or smoser to respond.

[Scott Moser (smoser)]

I kind of suspect that mount.ntfs would do a modprobe. That seems quite reasonable to me.

that said... There is a ton of happenstance here. I realize that most likely if you have an ntfs filesystem on azure and you can't mount it, then you probably didnt have any data there.

However, there is a failure case. However unlikely..
a.) boot into ubuntu with ntfs support, put a ton of important data on a disk that warns clearly that it is not to be used for important data.
b.) install a custom kernel
c.) reboot
d.) cloud-init can't mount ntfs, so it assumes it should create a new filesystem there.
e.) user screams, and angry tweets about how irresponsible cloud-init developers are, especially @smoser0.

yes... its probably very unlikely.

Better would be if we had information exposed to us from the platform (Azure) that said definitively "we wiped that disk... its empty now".

Without access to that fact, which is known to the platform, we're just doing a bunch of heuristics.

I'm almost more apt to just say that on azure, cloud-init will *always* feel free to destroy an ntfs partition. At least then, we've fixed the problem for the future... no one will ever have an ntfs filesystem there that they put their data on. We can make a config option saying "never-destroy-ntfs" for those people that for some reason did that intentionally, or we can just tell them that there might be better choices for a filesystem.

21ef0d7... by Paul Meyer on 2018-05-12

Review comments - change logic to use config switch

[Paul Meyer (paul-meyer)]

After discussion on IRC, made it so that

a.) if we can mount it, then check for data... don't reformat it if it has files
b.) if we cannot mount it... then check a config variable, if that variable says not to delete, then don't.

If we are able to mount and see that there is data on the disk (in NTFS fs), we'll emit a warning to guide the user to set the config setting.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:21ef0d781cc3fca1da26225d635313db24c6ee31
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1119/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1119/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

some feedback in line.

c4e82b2... by Paul Meyer on 2018-05-17

Review comments

[Paul Meyer (paul-meyer)]

Updated. I wasn't sure how to set the LANG env variable for the mount command, but I hope this is acceptable.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:c4e82b27c2149c000633554c231274bcd3c6e1eb
https://jenkins.ubuntu.com/server/job/cloud-init-ci/8/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/8/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

review: Approve

[Scott Moser (smoser)]

An upstream commit landed for this bug.

To view that commit see the following URL:
https://git.launchpad.net/cloud-init/commit/?id=aa4eeb80

diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index 1b03d46..7007d9e 100644
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -208,6 +208,7 @@ BUILTIN_CLOUD_CONFIG = {
 }
 
 DS_CFG_PATH = ['datasource', DS_NAME]
+DS_CFG_KEY_PRESERVE_NTFS = 'never_destroy_ntfs'
 DEF_EPHEMERAL_LABEL = 'Temporary Storage'
 
 # The redacted password fails to meet password complexity requirements
@@ -394,14 +395,9 @@ class DataSourceAzure(sources.DataSource):
         if found == ddir:
             LOG.debug("using files cached in %s", ddir)
 
-        # azure / hyper-v provides random data here
-        # TODO. find the seed on FreeBSD platform
-        # now update ds_cfg to reflect contents pass in config
-        if not util.is_FreeBSD():
-            seed = util.load_file("/sys/firmware/acpi/tables/OEM0",
-                                  quiet=True, decode=False)
-            if seed:
-                self.metadata['random_seed'] = seed
+        seed = _get_random_seed()
+        if seed:
+            self.metadata['random_seed'] = seed
 
         user_ds_cfg = util.get_cfg_by_path(self.cfg, DS_CFG_PATH, {})
         self.ds_cfg = util.mergemanydict([user_ds_cfg, self.ds_cfg])
@@ -539,7 +535,9 @@ class DataSourceAzure(sources.DataSource):
         return fabric_data
 
     def activate(self, cfg, is_new_instance):
-        address_ephemeral_resize(is_new_instance=is_new_instance)
+        address_ephemeral_resize(is_new_instance=is_new_instance,
+                                 preserve_ntfs=self.ds_cfg.get(
+                                     DS_CFG_KEY_PRESERVE_NTFS, False))
         return
 
     @property
@@ -583,17 +581,29 @@ def _has_ntfs_filesystem(devpath):
     return os.path.realpath(devpath) in ntfs_devices
 
 
-def can_dev_be_reformatted(devpath):
-    """Determine if block device devpath is newly formatted ephemeral.
+def can_dev_be_reformatted(devpath, preserve_ntfs):
+    """Determine if the ephemeral drive at devpath should be reformatted.
 
-    A newly formatted disk will:
+    A fresh ephemeral disk is formatted by Azure and will:
       a.) have a partition table (dos or gpt)
       b.) have 1 partition that is ntfs formatted, or
           have 2 partitions with the second partition ntfs formatted.
           (larger instances with >2TB ephemeral disk have gpt, and will
            have a microsoft reserved partition as part 1.  LP: #1686514)
       c.) the ntfs partition will have no files other than possibly
-          'dataloss_warning_readme.txt'"""
+          'dataloss_warning_readme.txt'
+
+    User can indicate that NTFS should never be destroyed by setting
+    DS_CFG_KEY_PRESERVE_NTFS in dscfg.
+    If data is found on NTFS, user is warned to set DS_CFG_KEY_PRESERVE_NTFS
+    to make sure cloud-init does not accidentally wipe their data.
+    If cloud-init cannot mount the disk to check for data, destruction
+    will be allowed, unless the dscfg key is set."""
+    if preserve_ntfs:
+        msg = ('config says to never destroy NTFS (%s.%s), skipping checks' %
+               (".".join(DS_CFG_PATH), DS_CFG_KEY_PRESERVE_NTFS))
+        return False, msg
+
     if not os.path.exists(devpath):
         return False, 'device %s does not exist' % devpath
 
@@ -626,18 +636,27 @@ def can_dev_be_reformatted(devpath):
     bmsg = ('partition %s (%s) on device %s was ntfs formatted' %
             (cand_part, cand_path, devpath))
     try:
-        file_count = util.mount_cb(cand_path, count_files)
+        file_count = util.mount_cb(cand_path, count_files, mtype="ntfs",
+                                   update_env_for_mount={'LANG': 'C'})
     except util.MountFailedError as e:
+        if "mount: unknown filesystem type 'ntfs'" in str(e):
+            return True, (bmsg + ' but this system cannot mount NTFS,'
+                          ' assuming there are no important files.'
+                          ' Formatting allowed.')
         return False, bmsg + ' but mount of %s failed: %s' % (cand_part, e)
 
     if file_count != 0:
+        LOG.warning("it looks like you're using NTFS on the ephemeral disk, "
+                    'to ensure that filesystem does not get wiped, set '
+                    '%s.%s in config', '.'.join(DS_CFG_PATH),
+                    DS_CFG_KEY_PRESERVE_NTFS)
         return False, bmsg + ' but had %d files on it.' % file_count
 
     return True, bmsg + ' and had no important files. Safe for reformatting.'
 
 
 def address_ephemeral_resize(devpath=RESOURCE_DISK_PATH, maxwait=120,
-                             is_new_instance=False):
+                             is_new_instance=False, preserve_ntfs=False):
     # wait for ephemeral disk to come up
     naplen = .2
     missing = util.wait_for_files([devpath], maxwait=maxwait, naplen=naplen,
@@ -653,7 +672,7 @@ def address_ephemeral_resize(devpath=RESOURCE_DISK_PATH, maxwait=120,
     if is_new_instance:
         result, msg = (True, "First instance boot.")
     else:
-        result, msg = can_dev_be_reformatted(devpath)
+        result, msg = can_dev_be_reformatted(devpath, preserve_ntfs)
 
     LOG.debug("reformattable=%s: %s", result, msg)
     if not result:
@@ -967,6 +986,18 @@ def _check_freebsd_cdrom(cdrom_dev):
     return False
 
 
+def _get_random_seed():
+    """Return content random seed file if available, otherwise,
+       return None."""
+    # azure / hyper-v provides random data here
+    # TODO. find the seed on FreeBSD platform
+    # now update ds_cfg to reflect contents pass in config
+    if util.is_FreeBSD():
+        return None
+    return util.load_file("/sys/firmware/acpi/tables/OEM0",
+                          quiet=True, decode=False)
+
+
 def list_possible_azure_ds_devs():
     devlist = []
     if util.is_FreeBSD():
diff --git a/cloudinit/util.py b/cloudinit/util.py
index edfedc7..653ed6e 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -1581,7 +1581,8 @@ def mounts():
     return mounted
 
 
-def mount_cb(device, callback, data=None, rw=False, mtype=None, sync=True):
+def mount_cb(device, callback, data=None, rw=False, mtype=None, sync=True,
+             update_env_for_mount=None):
     """
     Mount the device, call method 'callback' passing the directory
     in which it was mounted, then unmount.  Return whatever 'callback'
@@ -1643,7 +1644,7 @@ def mount_cb(device, callback, data=None, rw=False, mtype=None, sync=True):
                         mountcmd.extend(['-t', mtype])
                     mountcmd.append(device)
                     mountcmd.append(tmpd)
-                    subp(mountcmd)
+                    subp(mountcmd, update_env=update_env_for_mount)
                     umount = tmpd  # This forces it to be unmounted (when set)
                     mountpoint = tmpd
                     break
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index 26e8d7d..e82716e 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -1,10 +1,10 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
 from cloudinit import helpers
-from cloudinit.util import b64e, decode_binary, load_file, write_file
 from cloudinit.sources import DataSourceAzure as dsaz
-from cloudinit.util import find_freebsd_part
-from cloudinit.util import get_path_dev_freebsd
+from cloudinit.util import (b64e, decode_binary, load_file, write_file,
+                            find_freebsd_part, get_path_dev_freebsd,
+                            MountFailedError)
 from cloudinit.version import version_string as vs
 from cloudinit.tests.helpers import (CiTestCase, TestCase, populate_dir, mock,
                                      ExitStack, PY26, SkipTest)
@@ -95,6 +95,8 @@ class TestAzureDataSource(CiTestCase):
         self.patches = ExitStack()
         self.addCleanup(self.patches.close)
 
+        self.patches.enter_context(mock.patch.object(dsaz, '_get_random_seed'))
+
         super(TestAzureDataSource, self).setUp()
 
     def apply_patches(self, patches):
@@ -335,6 +337,18 @@ fdescfs            /dev/fd          fdescfs rw              0 0
         self.assertTrue(ret)
         self.assertEqual(data['agent_invoked'], '_COMMAND')
 
+    def test_sys_cfg_set_never_destroy_ntfs(self):
+        sys_cfg = {'datasource': {'Azure': {
+            'never_destroy_ntfs': 'user-supplied-value'}}}
+        data = {'ovfcontent': construct_valid_ovf_env(data={}),
+                'sys_cfg': sys_cfg}
+
+        dsrc = self._get_ds(data)
+        ret = self._get_and_setup(dsrc)
+        self.assertTrue(ret)
+        self.assertEqual(dsrc.ds_cfg.get(dsaz.DS_CFG_KEY_PRESERVE_NTFS),
+                         'user-supplied-value')
+
     def test_username_used(self):
         odata = {'HostName': "myhost", 'UserName': "myuser"}
         data = {'ovfcontent': construct_valid_ovf_env(data=odata)}
@@ -676,6 +690,8 @@ class TestAzureBounce(CiTestCase):
                               mock.MagicMock(return_value={})))
         self.patches.enter_context(
             mock.patch.object(dsaz.util, 'which', lambda x: True))
+        self.patches.enter_context(
+            mock.patch.object(dsaz, '_get_random_seed'))
 
         def _dmi_mocks(key):
             if key == 'system-uuid':
@@ -957,7 +973,9 @@ class TestCanDevBeReformatted(CiTestCase):
             # return sorted by partition number
             return sorted(ret, key=lambda d: d[0])
 
-        def mount_cb(device, callback):
+        def mount_cb(device, callback, mtype, update_env_for_mount):
+            self.assertEqual('ntfs', mtype)
+            self.assertEqual('C', update_env_for_mount.get('LANG'))
             p = self.tmp_dir()
             for f in bypath.get(device).get('files', []):
                 write_file(os.path.join(p, f), content=f)
@@ -988,14 +1006,16 @@ class TestCanDevBeReformatted(CiTestCase):
                     '/dev/sda2': {'num': 2},
                     '/dev/sda3': {'num': 3},
                 }}})
-        value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
+        value, msg = dsaz.can_dev_be_reformatted("/dev/sda",
+                                                 preserve_ntfs=False)
         self.assertFalse(value)
         self.assertIn("3 or more", msg.lower())
 
     def test_no_partitions_is_false(self):
         """A disk with no partitions can not be formatted."""
         self.patchup({'/dev/sda': {}})
-        value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
+        value, msg = dsaz.can_dev_be_reformatted("/dev/sda",
+                                                 preserve_ntfs=False)
         self.assertFalse(value)
         self.assertIn("not partitioned", msg.lower())
 
@@ -1007,7 +1027,8 @@ class TestCanDevBeReformatted(CiTestCase):
                     '/dev/sda1': {'num': 1},
                     '/dev/sda2': {'num': 2, 'fs': 'ext4', 'files': []},
                 }}})
-        value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
+        value, msg = dsaz.can_dev_be_reformatted("/dev/sda",
+                                                 preserve_ntfs=False)
         self.assertFalse(value)
         self.assertIn("not ntfs", msg.lower())
 
@@ -1020,7 +1041,8 @@ class TestCanDevBeReformatted(CiTestCase):
                     '/dev/sda2': {'num': 2, 'fs': 'ntfs',
                                   'files': ['secret.txt']},
                 }}})
-        value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
+        value, msg = dsaz.can_dev_be_reformatted("/dev/sda",
+                                                 preserve_ntfs=False)
         self.assertFalse(value)
         self.assertIn("files on it", msg.lower())
 
@@ -1032,7 +1054,8 @@ class TestCanDevBeReformatted(CiTestCase):
                     '/dev/sda1': {'num': 1},
                     '/dev/sda2': {'num': 2, 'fs': 'ntfs', 'files': []},
                 }}})
-        value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
+        value, msg = dsaz.can_dev_be_reformatted("/dev/sda",
+                                                 preserve_ntfs=False)
         self.assertTrue(value)
         self.assertIn("safe for", msg.lower())
 
@@ -1043,7 +1066,8 @@ class TestCanDevBeReformatted(CiTestCase):
                 'partitions': {
                     '/dev/sda1': {'num': 1, 'fs': 'zfs'},
                 }}})
-        value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
+        value, msg = dsaz.can_dev_be_reformatted("/dev/sda",
+                                                 preserve_ntfs=False)
         self.assertFalse(value)
         self.assertIn("not ntfs", msg.lower())
 
@@ -1055,9 +1079,14 @@ class TestCanDevBeReformatted(CiTestCase):
                     '/dev/sda1': {'num': 1, 'fs': 'ntfs',
                                   'files': ['file1.txt', 'file2.exe']},
                 }}})
-        value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
-        self.assertFalse(value)
-        self.assertIn("files on it", msg.lower())
+        with mock.patch.object(dsaz.LOG, 'warning') as warning:
+            value, msg = dsaz.can_dev_be_reformatted("/dev/sda",
+                                                     preserve_ntfs=False)
+            wmsg = warning.call_args[0][0]
+            self.assertIn("looks like you're using NTFS on the ephemeral disk",
+                          wmsg)
+            self.assertFalse(value)
+            self.assertIn("files on it", msg.lower())
 
     def test_one_partition_ntfs_empty_is_true(self):
         """1 mountable ntfs partition and no files can be formatted."""
@@ -1066,7 +1095,8 @@ class TestCanDevBeReformatted(CiTestCase):
                 'partitions': {
                     '/dev/sda1': {'num': 1, 'fs': 'ntfs', 'files': []}
                 }}})
-        value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
+        value, msg = dsaz.can_dev_be_reformatted("/dev/sda",
+                                                 preserve_ntfs=False)
         self.assertTrue(value)
         self.assertIn("safe for", msg.lower())
 
@@ -1078,7 +1108,8 @@ class TestCanDevBeReformatted(CiTestCase):
                     '/dev/sda1': {'num': 1, 'fs': 'ntfs',
                                   'files': ['dataloss_warning_readme.txt']}
                 }}})
-        value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
+        value, msg = dsaz.can_dev_be_reformatted("/dev/sda",
+                                                 preserve_ntfs=False)
         self.assertTrue(value)
         self.assertIn("safe for", msg.lower())
 
@@ -1093,7 +1124,8 @@ class TestCanDevBeReformatted(CiTestCase):
                         'num': 1, 'fs': 'ntfs', 'files': [self.warning_file],
                         'realpath': '/dev/sdb1'}
                 }}})
-        value, msg = dsaz.can_dev_be_reformatted(epath)
+        value, msg = dsaz.can_dev_be_reformatted(epath,
+                                                 preserve_ntfs=False)
         self.assertTrue(value)
         self.assertIn("safe for", msg.lower())
 
@@ -1112,10 +1144,49 @@ class TestCanDevBeReformatted(CiTestCase):
                     epath + '-part3': {'num': 3, 'fs': 'ext',
                                        'realpath': '/dev/sdb3'}
                 }}})
-        value, msg = dsaz.can_dev_be_reformatted(epath)
+        value, msg = dsaz.can_dev_be_reformatted(epath,
+                                                 preserve_ntfs=False)
         self.assertFalse(value)
         self.assertIn("3 or more", msg.lower())
 
+    def test_ntfs_mount_errors_true(self):
+        """can_dev_be_reformatted does not fail if NTFS is unknown fstype."""
+        self.patchup({
+            '/dev/sda': {
+                'partitions': {
+                    '/dev/sda1': {'num': 1, 'fs': 'ntfs', 'files': []}
+                }}})
+
+        err = ("Unexpected error while running command.\n",
+               "Command: ['mount', '-o', 'ro,sync', '-t', 'auto', ",
+               "'/dev/sda1', '/fake-tmp/dir']\n"
+               "Exit code: 32\n"
+               "Reason: -\n"
+               "Stdout: -\n"
+               "Stderr: mount: unknown filesystem type 'ntfs'")
+        self.m_mount_cb.side_effect = MountFailedError(
+            'Failed mounting %s to %s due to: %s' %
+            ('/dev/sda', '/fake-tmp/dir', err))
+
+        value, msg = dsaz.can_dev_be_reformatted('/dev/sda',
+                                                 preserve_ntfs=False)
+        self.assertTrue(value)
+        self.assertIn('cannot mount NTFS, assuming', msg)
+
+    def test_never_destroy_ntfs_config_false(self):
+        """Normally formattable situation with never_destroy_ntfs set."""
+        self.patchup({
+            '/dev/sda': {
+                'partitions': {
+                    '/dev/sda1': {'num': 1, 'fs': 'ntfs',
+                                  'files': ['dataloss_warning_readme.txt']}
+                }}})
+        value, msg = dsaz.can_dev_be_reformatted("/dev/sda",
+                                                 preserve_ntfs=True)
+        self.assertFalse(value)
+        self.assertIn("config says to never destroy NTFS "
+                      "(datasource.Azure.never_destroy_ntfs)", msg)
+
 
 class TestAzureNetExists(CiTestCase):