[Merge] ~robert-ancell/software-properties:esm into software-properties:ubuntu/master

[Chad Smith]

Thanks so much for merging my patch/review suggestion Robert. I did talk with the team and determined that we weren't as close as I thought to releasing the beta flag on esm-apps at the moment. So, we can't strictly rely on only is_current_distro_lts to determine esm-apps availability. We do have to go back to part of your initial branch which calls ua status directly and parses both availability and status out of the UA service.  

Here's the second diff that I've tested on Bionic (substituting stdout=subprocess.PIPE instead of capture_output=True in the subprocess.run command). It's downloadable also as https://paste.ubuntu.com/p/CG5ZXTDvfH/

diff --git a/softwareproperties/gtk/SoftwarePropertiesGtk.py b/softwareproperties/gtk/SoftwarePropertiesGtk.py
index 50007ca..ddc4225 100644
--- a/softwareproperties/gtk/SoftwarePropertiesGtk.py
+++ b/softwareproperties/gtk/SoftwarePropertiesGtk.py
@@ -409,17 +409,17 @@ class SoftwarePropertiesGtk(SoftwareProperties, SimpleGtkbuilderApp):
                 self.dev_box.add(checkbox)
                 checkbox.show()
 
-        if is_current_distro_lts():
-                # All LTS >= Trusty have access to esm-infra and esm-apps
-                esm_available = True
-                esm_enabled = get_ua_service_status("esm-infra") == "enabled" or get_ua_service_status("esm-apps") == "enabled"
-        else:
-                # Non-LTS means no ESM* service
+        status = get_ua_status()
+        if not is_current_distro_lts():
                 esm_available = False
-                esm_enabled = False
+                infra_status = apps_status = "n/a"  # Non-LTS means no ESM* service
+        else:
+                (infra_available, infra_status) = get_ua_service_status("esm-infra", status=status)
+                (apps_available, apps_status) = get_ua_service_status("esm-apps", status=status)
+                esm_available = bool(infra_available or apps_available)
+        esm_enabled = "enabled" in (infra_status, apps_status)
         distro = current_distro()
-        if esm_enabled:
-                status = get_ua_status()
+        if "enabled" in (infra_status, apps_status):
                 eol_text = _("Extended Security Maintenance")
                 # EOL date should probably be UA contract expiry.
                 # This is probably sooner than ESM EOL for the distro and
@@ -449,7 +449,7 @@ class SoftwarePropertiesGtk(SoftwareProperties, SimpleGtkbuilderApp):
                 eol_expiry_text = _("Ends %s - extend or upgrade soon") % eol_date.strftime("%x")
         else:
                 eol_expiry_text = _("Active until %s") % eol_date.strftime("%x")
-        self.label_eol.set_label(eol_expiry_text);
+        self.label_eol.set_label(eol_expiry_text)
         self.label_esm_subscribe.set_markup(
                 "<a href=\"%s\">%s</a>" % (esm_url, _("Extend…"))
         )
diff --git a/softwareproperties/gtk/utils.py b/softwareproperties/gtk/utils.py
index 35b7498..960a8ba 100644
--- a/softwareproperties/gtk/utils.py
+++ b/softwareproperties/gtk/utils.py
@@ -77,11 +77,19 @@ def current_distro():
 def get_ua_status():
     """Return a dict of all UA status information or empty dict on error."""
     # status.json will exist on any attached system or any unattached system
-    # which has already run `ua status`. Don't call ua status directly on
-    # network disconnected machines as it will timeout trying to access
-    # contracts.canonical.com/v1/resources.
-    if not os.path.exists(UA_STATUS_JSON):
+    # which has already run `ua status`. Calling ua status directly on
+    # network disconnected machines will raise a TimeoutException trying to
+    # access contracts.canonical.com/v1/resources.
+    try:
+        # Success writes UA_STATUS_JSON
+        result = subprocess.run(['ua', 'status', '--format=json'], capture_output=True)
+    except Exception as e:
+        print("Failed to call ubuntu advantage client:\n%s" % e)
+        return {}
+    if result.returncode != 0:
+        print("Ubuntu advantage client returned code %d" % result.returncode)
         return {}
+
     with open(UA_STATUS_JSON, "r") as stream:
         status_json = stream.read()
     try:
@@ -91,28 +99,33 @@ def get_ua_status():
         return {}
     return status
 
-def get_ua_service_status(service_name="esm-infra"):
-    """Check `ua status`, returning a string representeing the servide status.
-
-    Generally this will be one of: n/a, enabled, disabled, available
-
-    Return None when service_name is undefined or `ua status` not understood.
+def get_ua_service_status(service_name='esm-infra', status=None):
+    """Get service availability and status for a specific UA service.
+
+    Return a tuple (available, service_status).
+      :boolean available: set True when either:
+        - attached contract is entitled to the service
+        - unattached machine reports service "availability" as "yes"
+      :str service_status: will be one of the following:
+        - "disabled" when the service is available but not active
+        - "enabled" when the service is available and active
+        - "n/a" when the service is not applicable on the environment or not
+          entitled for the attached contract
     """
-    status = get_ua_status()
-
+    if not status:
+        status = get_ua_status()
+    available = False
+    service_status = "n/a"
     for service in status.get("services", []):
         if service.get("name") == service_name:
             if service.get("available"):  # then we are not attached
-                if service.get("available") == "no":
-                    # Available "no" means service is not compatible with
-                    # either platform, release or kernel when machine is
-                    # unattached. So, return n/a for simplicity because that is
-                    # what is returned from "status" if machine were attached.
-                    return "n/a"
-                else:
-                    return "disabled"  # Report not enabled since unattached
-            return service.get("status")  # Will be enabled, disabled or n/a
-    return None
+                if service["available"] == "yes":
+                     available = True
+                     service_status = "disabled"  # Disabled since unattached
+            else:  # attached
+                available = service.get("entitled") == "yes"
+                service_status = service.get("status")  # Will be enabled, disabled or n/a
+    return (available, service_status)
 
 def retry(exceptions, tries=10, delay=0.1, backoff=2):
     """


-- 
https://code.launchpad.net/~robert-ancell/software-properties/+git/software-properties/+merge/400153
Your team Ubuntu Core Development Team is requested to review the proposed merge of ~robert-ancell/software-properties:esm into software-properties:ubuntu/master.