Merge pull request #18 from caruhome/fix/force-flag-semantics

Fix/force flag semantics

misc/README.md

@@ -9,7 +9,6 @@ A little helper to create an Upparat job.
                                  S3_BUCKET --arn-s3-role ARN_S3_ROLE --arn-iot
                                  ARN_IOT [--job-id JOB_ID] [--thing THING]
                                  [--group GROUP] [--force] [--dry-run]
-                                 [--create-legacy-job]
                                  [--target-selection {SNAPSHOT,CONTINUOUS}]
 
     Create Upparat Jobs.
@@ -22,9 +21,8 @@ A little helper to create an Upparat job.
     --job-id JOB_ID             A unique job id (if not provided, a generated UUID4).
     --thing THING               Thing(s) that should be updated. Or: cat things | ./upparat_create_job.py
     --group GROUP               Group(s) that should be included in this job.
-    --force                     Force the update (ignore hooks).
+    --force                     Force the update (ignore version & pre-download hooks).
     --dry-run                   Dry run / simulate what would happen.
-    --create-legacy-job         Legacy flag for Upparat versions <1.5 where the force flag was a string.
     --target-selection          Change targetSelection of job.{SNAPSHOT,CONTINUOUS}
 
 ### Examples
@@ -39,7 +37,7 @@ A little helper to create an Upparat job.
     --group mygroup \
     --thing mything
 
-*Output:*
+_Output:_
 
     Running Upparat Job Creator \o/
 
@@ -55,7 +53,7 @@ A little helper to create an Upparat job.
 
     ✓ Job 'my-group-and-thing-rollout-to-1.2' created!
 
-*You can also pipe a list into `./upparat_create_job.py` with things:*
+_You can also pipe a list into `./upparat_create_job.py` with things:_
 
     cat things | ./upparat_create_job.py (...)
 

misc/upparat_create_job.py

@@ -57,22 +57,14 @@ def parse_arguments(args):
         "--force",
         action="store_true",
         default=False,
-        help="Force the update (ignore hooks).",
+        help="Force the update (ignore version & pre-download hooks).",
     )
     parser.add_argument(
         "--dry-run",
         action="store_true",
         default=False,
         help="Dry run / simulate what would happen.",
     )
-    # FIXME: Temporary flag since we still have older Upparat instances running
-    # where the --force flag is a string ("True") instead of boolean.
-    parser.add_argument(
-        "--create-legacy-job",
-        action="store_true",
-        default=False,
-        help="Legacy flag for Upparat versions <1.5 where the force flag was a string.",
-    )
     parser.add_argument(
         "--target-selection",
         choices=["SNAPSHOT", "CONTINUOUS"],
@@ -151,10 +143,7 @@ def main(arguments):
 
     s3_object_url = f"https://{arguments.s3_bucket}.s3.amazonaws.com/{arguments.file}"
 
-    # FIXME: See --legacy flag. Remove this in the near future!
-    legacy_job = arguments.create_legacy_job
-    force = arguments.force if not legacy_job else str(arguments.force)
-    force_pretty = force if not legacy_job else f"'{force}' (→ legacy job)"
+    force = arguments.force
 
     document = create_job_document(
         s3_object_url=s3_object_url, version=arguments.version, force=force
@@ -164,7 +153,7 @@ def main(arguments):
     print(f" → Job: {job_id}")
     print(f" → File: {s3_object_url}")
     print(f" → Version: {arguments.version}")
-    print(f" → Force: {force_pretty}")
+    print(f" → Force: {force}")
     print(f" → ARN S3 role: {arguments.arn_s3_role}")
     print(f" → ARN IoT: {arguments.arn_iot}")
     print(f" → Things: {thing_names}")

src/upparat/statemachine/verify_installation.py

@@ -29,7 +29,7 @@ def __init__(self):
         super().__init__()
 
     def on_enter(self, state, event):
-        if self.job.force or not settings.hooks.version:
+        if not settings.hooks.version:
             logger.info("Skip version check")
             self.job_succeeded(JobSuccessStatus.COMPLETE_NO_VERSION_CHECK.value)
             self.publish(pysm.Event(JOB_INSTALLATION_COMPLETE))

src/upparat/statemachine/verify_job.py

@@ -38,13 +38,18 @@ def __init__(self):
 
     def on_enter(self, state, event):
         if self.job.status == JobStatus.QUEUED.value:
-            if self.job.force or not settings.hooks.version:
-                logger.info("Skip version check")
+            version_hook = settings.hooks.version
+            force = self.job.force
+
+            if force or not version_hook:
+                logger.info(
+                    f"Skip version check [force={force}, {version_hook if version_hook else 'no-hook'}]"  # noqa
+                )
                 return self._job_verified()
+
             logger.debug("Start version check")
-            # Start version check
             self.stop_version_hook = run_hook(
-                settings.hooks.version, self.root_machine.inbox, args=[self.job.meta]
+                version_hook, self.root_machine.inbox, args=[self.job.meta]
             )
 
         elif self.job.status == JobStatus.IN_PROGRESS.value:

tests/statemachine/verify_installation_test.py

@@ -76,36 +76,12 @@ def test_on_enter_no_hook(verify_installation_state):
     )
 
 
-def test_on_enter_force_job(verify_installation_state):
-    state, inbox, mqtt_client, _, _ = verify_installation_state
-
-    settings.hooks.version = "./version.sh"
-    state.job = create_job_with(force=True)
-
-    state.on_enter(None, None)
-
-    assert inbox.qsize() == 1
-    published_event = inbox.get_nowait()
-    assert published_event.name == JOB_INSTALLATION_COMPLETE
-
-    mqtt_client.publish.assert_called_once_with(
-        f"$aws/things/{settings.broker.thing_name}/jobs/{state.job.id_}/update",
-        json.dumps(
-            {
-                "status": JobStatus.SUCCEEDED.value,
-                "statusDetails": {
-                    "state": JobSuccessStatus.COMPLETE_NO_VERSION_CHECK.value,
-                    "message": "none",
-                },
-            }
-        ),
-    )
-
-
-def test_on_enter_hook_executed(verify_installation_state):
+@pytest.mark.parametrize("force", [True, False])
+def test_on_enter_hook_executed(verify_installation_state, force):
     state, inbox, _, _, run_hook = verify_installation_state
 
     settings.hooks.version = "./version.sh"
+    state.job = create_job_with(force=force)
 
     state.on_enter(None, None)