From 7c807560442d1cc88a3dbc4f73947b8d30edf020 Mon Sep 17 00:00:00 2001
From: Alan Somers <asomers@gmail.com>
Date: Tue, 9 Aug 2022 14:32:07 -0600
Subject: [PATCH] zpool.present: correctly handle "feature@" properties

Optional ZFS pool features can have three states: disabled, enabled, and
active.  Enabled means that ZFS will use them if it needs them, but they
haven't changed the on-disk format yet, so they can still be switched
off.  But active features have already changed the on-disk format, so
they can't be switched off.  Disabled features may not be used.

When enabling such a feature via the zpool.present state, treat "active"
as identical to "enabled".

Fixes #62390
---
 changelog/62390.fixed                   |  1 +
 salt/states/zpool.py                    |  9 ++++++++-
 tests/pytests/unit/states/test_zpool.py | 10 +++++++---
 3 files changed, 16 insertions(+), 4 deletions(-)
 create mode 100644 changelog/62390.fixed

diff --git a/changelog/62390.fixed b/changelog/62390.fixed
new file mode 100644
index 00000000000..e5fee65205f
--- /dev/null
+++ b/changelog/62390.fixed
@@ -0,0 +1 @@
+Fix the "zpool.present" state when enabling zpool features that are already active.
diff --git a/salt/states/zpool.py b/salt/states/zpool.py
index 6eac07ada13..9900dde9ea1 100644
--- a/salt/states/zpool.py
+++ b/salt/states/zpool.py
@@ -319,7 +319,14 @@ def present(
                     continue
 
                 # compare current and wanted value
-                if properties_current[prop] != properties[prop]:
+                # Enabled "feature@" properties may report either "enabled" or
+                # "active", depending on whether they're currently in-use.
+                if prop.startswith("feature@") and properties_current[prop] == "active":
+                    effective_property = "enabled"
+                else:
+                    effective_property = properties_current[prop]
+
+                if effective_property != properties[prop]:
                     properties_update.append(prop)
 
         # update pool properties
diff --git a/tests/pytests/unit/states/test_zpool.py b/tests/pytests/unit/states/test_zpool.py
index 3b109761fa4..25d2fd564cf 100644
--- a/tests/pytests/unit/states/test_zpool.py
+++ b/tests/pytests/unit/states/test_zpool.py
@@ -340,7 +340,7 @@ def test_present_update_success(utils_patch):
         "name": "myzpool",
         "result": True,
         "comment": "properties updated",
-        "changes": {"myzpool": {"autoexpand": False}},
+        "changes": {"myzpool": {"autoexpand": False, "feature@bookmarks": "enabled"}},
     }
 
     config = {
@@ -352,6 +352,8 @@ def test_present_update_success(utils_patch):
     ]
     properties = {
         "autoexpand": False,
+        "feature@hole_birth": "enabled",
+        "feature@bookmarks": "enabled",
     }
 
     mock_exists = MagicMock(return_value=True)
@@ -368,7 +370,7 @@ def test_present_update_success(utils_patch):
                 ("dedupditto", "0"),
                 ("dedupratio", "1.00x"),
                 ("autoexpand", True),
-                ("feature@bookmarks", "enabled"),
+                ("feature@bookmarks", "disabled"),
                 ("allocated", 115712),
                 ("guid", 1591906802560842214),
                 ("feature@large_blocks", "enabled"),
@@ -421,7 +423,7 @@ def test_present_update_success(utils_patch):
 
 def test_present_update_nochange_success(utils_patch):
     """
-    Test zpool present with non existing pool
+    Test zpool present with an up-to-date pool
     """
     config = {
         "import": False,
@@ -432,6 +434,8 @@ def test_present_update_nochange_success(utils_patch):
     ]
     properties = {
         "autoexpand": True,
+        "feature@hole_birth": "enabled",
+        "feature@bookmarks": "enabled",
     }
 
     mock_exists = MagicMock(return_value=True)