Merge ~cgrabowski/maas:handle_different_channel_check_for_missing_channel into maas:master

[Christian Grabowski]

build channel from version, like we do for target version, when update origin is missing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b handle_different_channel_check_for_missing_channel lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: bec34b10b30f9ecb5059aa09521eb962e14d6bf9

review: Approve

[Alberto Donato (ack)]

We should probably add another entry to VERSION_ISSUES for this specific case where a controller has no update_origin (and don't report the DIFFERENT_CHANNEL one in that case).

While the current message is misleading, I think it's correct to report lack of an update origin as an issue as that means the controller will never get updates (which has security-related implications).

review: Needs Fixing

[Christian Grabowski (cgrabowski)]

> We should probably add another entry to VERSION_ISSUES for this specific case
> where a controller has no update_origin (and don't report the
> DIFFERENT_CHANNEL one in that case).
>
> While the current message is misleading, I think it's correct to report lack
> of an update origin as an issue as that means the controller will never get
> updates (which has security-related implications).

That makes sense. I was thinking with regards to the scenario in the bug report where you purposely download the snap into an air-gapped env, but I suppose we should still flag that as it won't get updates.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b handle_different_channel_check_for_missing_channel lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/11903/console
COMMIT: 79e8a3992f42f1c5a725cfddbbf8dcf4a14c4042

review: Needs Fixing

[Alberto Donato (ack)]

Thanks for the changes, looks good!

One nitpick inline

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b handle_different_channel_check_for_missing_channel lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/11914/console
COMMIT: c48ef59394ae046e4d4f82b8edc3e88eecec45b9

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b handle_different_channel_check_for_missing_channel lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: e551d2bf200baa28510c7bd52c70fd98ecb097ba

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b handle_different_channel_check_for_missing_channel lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/11916/consoleText

diff --git a/src/maasserver/models/controllerinfo.py b/src/maasserver/models/controllerinfo.py
index 16906e1..6e5c55c 100644
--- a/src/maasserver/models/controllerinfo.py
+++ b/src/maasserver/models/controllerinfo.py
@@ -48,6 +48,7 @@ class VERSION_ISSUES(Enum):
 
     DIFFERENT_CHANNEL = "different-channel"
     DIFFERENT_COHORT = "different-cohort"
+    MISSING_CHANNEL = "missing-channel"
 
 
 class ControllerInfoManager(Manager):
@@ -163,10 +164,13 @@ class ControllerInfo(CleanSave, TimestampedModel):
         """Return a list of version-related issues compared to the target version."""
         issues = []
         if self.install_type == CONTROLLER_INSTALL_TYPE.SNAP:
-            if (
-                SnapChannel.from_string(self.update_origin)
-                != target.snap_channel
-            ):
+            if self.update_origin:
+                snap_channel = SnapChannel.from_string(self.update_origin)
+            else:
+                snap_channel = None
+                issues.append(VERSION_ISSUES.MISSING_CHANNEL.value)
+
+            if snap_channel and snap_channel != target.snap_channel:
                 issues.append(VERSION_ISSUES.DIFFERENT_CHANNEL.value)
             if self.snap_cohort != target.snap_cohort:
                 issues.append(VERSION_ISSUES.DIFFERENT_COHORT.value)
@@ -198,6 +202,16 @@ def get_maas_version() -> Optional[MAASVersion]:
     return versions[0][1]
 
 
+def channel_from_version(version) -> SnapChannel:
+    """ channel from version constructs a SnapChannel from a MAAS version """
+    risk_map = {"alpha": "edge", "beta": "beta", "rc": "candidate"}
+    risk = risk_map.get(version.qualifier_type, "stable")
+    return SnapChannel(
+        track=f"{version.major}.{version.minor}",
+        risk=risk,
+    )
+
+
 def get_target_version() -> Optional[TargetVersion]:
     """Get the target version for the deployment."""
     highest_version = None
@@ -264,12 +278,7 @@ def get_target_version() -> Optional[TargetVersion]:
             )
     else:
         # compose the channel from the target version
-        risk_map = {"alpha": "edge", "beta": "beta", "rc": "candidate"}
-        risk = risk_map.get(version.qualifier_type, "stable")
-        snap_channel = SnapChannel(
-            track=f"{version.major}.{version.minor}",
-            risk=risk,
-        )
+        snap_channel = channel_from_version(version)
 
     # report a cohort only if all controllers with the target version are on
     # the same cohort (or have no cohort)
diff --git a/src/maasserver/models/tests/test_controllerinfo.py b/src/maasserver/models/tests/test_controllerinfo.py
index d736cf2..1e979c1 100644
--- a/src/maasserver/models/tests/test_controllerinfo.py
+++ b/src/maasserver/models/tests/test_controllerinfo.py
@@ -393,6 +393,28 @@ class TestControllerInfo(MAASServerTestCase):
             ],
         )
 
+    def test_get_version_issue_flags_missing_channel(self):
+        target_version = TargetVersion(
+            version=MAASVersion.from_string("3.1.0-222-g.cafecafe"),
+            snap_channel=SnapChannel.from_string("3.1/stable"),
+        )
+        controller = factory.make_RegionRackController()
+        ControllerInfo.objects.set_versions_info(
+            controller,
+            SnapVersionsInfo(
+                current={
+                    "revision": "1234",
+                    "version": "3.1.0-222-g.cafecafe",
+                },
+            ),
+        )
+        self.assertEqual(
+            controller.info.get_version_issues(target_version),
+            [
+                VERSION_ISSUES.MISSING_CHANNEL.value,
+            ],
+        )
+
 
 class TestGetTargetVersion(MAASServerTestCase):
     def test_empty(self):