Remove confusion about upgrade-to-newer-dependencies breeze param (#2…

…3334)

The "upgrade-to-newer-dependencies" in the image can take "false"
"true" and <RANDOM> value (the RANDOM value is used to always
invalidate docker cache).

This has been carried to Breeze command line, but this was a source
of major confusion as the name of the parameter suggest bool value.

The change modifies the parameter to be flag, and when the flag
is set the parameter is set to random during image building.

That should help with recent bug when image was always rebuilt
without checking if it should be rebuilt.

dev/breeze/src/airflow_breeze/build_image/ci/build_ci_params.py

@@ -33,7 +33,7 @@ class BuildCiParams:
     CI build parameters. Those parameters are used to determine command issued to build CI image.
     """
 
-    upgrade_to_newer_dependencies: str = "false"
+    upgrade_to_newer_dependencies: bool = False
     python: str = "3.7"
     airflow_branch: str = AIRFLOW_BRANCH
     build_id: int = 0

dev/breeze/src/airflow_breeze/build_image/prod/build_prod_params.py

@@ -52,7 +52,7 @@ class BuildProdParams:
     empty_image: bool = False
     airflow_is_in_context: bool = False
     install_packages_from_context: bool = False
-    upgrade_to_newer_dependencies: str = "false"
+    upgrade_to_newer_dependencies: bool = False
     airflow_version: str = get_airflow_version()
     python: str = "3.7"
     airflow_branch_for_pypi_preloading: str = AIRFLOW_BRANCH

dev/breeze/src/airflow_breeze/commands/common_options.py

@@ -197,9 +197,8 @@
 option_upgrade_to_newer_dependencies = click.option(
     "-u",
     '--upgrade-to-newer-dependencies',
-    default="false",
-    show_default=True,
-    help='When other than "false", upgrade all PIP packages to latest.',
+    is_flag=True,
+    help='When set, upgrade all PIP packages to latest.',
     envvar='UPGRADE_TO_NEWER_DEPENDENCIES',
 )
 option_additional_extras = click.option(

dev/breeze/src/airflow_breeze/commands/release_management.py

@@ -320,21 +320,20 @@ def generate_constraints(
     set_forced_answer(answer)
     if run_in_parallel:
         given_answer = user_confirm(
-            f"Did you build all CI images {python_versions} with --upgrade-to-newer-dependencies "
-            f"true flag set?",
+            f"Did you build all CI images {python_versions} with --upgrade-to-newer-dependencies flag set?",
             timeout=None,
         )
     else:
         given_answer = user_confirm(
-            f"Did you build CI image {python} with --upgrade-to-newer-dependencies true flag set?",
+            f"Did you build CI image {python} with --upgrade-to-newer-dependencies flag set?",
             timeout=None,
         )
     if given_answer != Answer.YES:
         if run_in_parallel:
             get_console().print("\n[info]Use this command to build the images:[/]\n")
             get_console().print(
                 f"     breeze build-image --run-in-parallel --python-versions '{python_versions}' "
-                f"--upgrade-to-newer-dependencies true\n"
+                f"--upgrade-to-newer-dependencies\n"
             )
         else:
             shell_params = ShellParams(
@@ -343,7 +342,7 @@ def generate_constraints(
             get_console().print("\n[info]Use this command to build the image:[/]\n")
             get_console().print(
                 f"     breeze build-image --python'{shell_params.python}' "
-                f"--upgrade-to-newer-dependencies true\n"
+                f"--upgrade-to-newer-dependencies\n"
             )
         sys.exit(1)
     if run_in_parallel:

dev/breeze/src/airflow_breeze/shell/enter_shell.py

@@ -120,7 +120,7 @@ def run_shell_with_build_image_checks(
     build_ci_image_check_cache = Path(
         BUILD_CACHE_DIR, shell_params.airflow_branch, f".built_{shell_params.python}"
     )
-    ci_image_params = BuildCiParams(python=shell_params.python, upgrade_to_newer_dependencies="false")
+    ci_image_params = BuildCiParams(python=shell_params.python, upgrade_to_newer_dependencies=False)
     if build_ci_image_check_cache.exists():
         get_console().print(f'[info]{shell_params.the_image_type} image already built locally.[/]')
     else:

dev/breeze/src/airflow_breeze/utils/docker_command_utils.py

@@ -18,6 +18,7 @@
 import os
 import re
 import subprocess
+from random import randint
 from typing import Dict, List, Tuple, Union
 
 from airflow_breeze.build_image.ci.build_ci_params import BuildCiParams
@@ -286,7 +287,14 @@ def construct_arguments_for_docker_build_command(
     image_params: Union[BuildCiParams, BuildProdParams], required_args: List[str], optional_args: List[str]
 ) -> List[str]:
     """
-    Constructs docker compose command arguments list based on parameters passed
+    Constructs docker compose command arguments list based on parameters passed. Maps arguments to
+    argument values.
+
+    It maps:
+    * all the truthy/falsy values are converted to "true" / "false" respectively
+    * if upgrade_to_newer_dependencies is set to True, it is replaced by a random string to account
+      for the need of always triggering upgrade for docker build.
+
     :param image_params: parameters of the image
     :param required_args: build argument that are required
     :param optional_args: build arguments that are optional (should not be used if missing or empty)
@@ -295,8 +303,10 @@ def construct_arguments_for_docker_build_command(
 
     def get_env_variable_value(arg_name: str):
         value = str(getattr(image_params, arg_name))
-        value = "true" if value == "True" else value
-        value = "false" if value == "False" else value
+        value = "true" if value.lower() in ["true", "t", "yes", "y"] else value
+        value = "false" if value.lower() in ["false", "f", "no", "n"] else value
+        if arg_name == "upgrade_to_newer_dependencies" and value == "true":
+            value = f"{randint(0, 2**32):x}"
         return value
 
     args_command = []

scripts/ci/selective_ci_checks.sh

@@ -44,13 +44,8 @@ fi
 
 function check_upgrade_to_newer_dependencies_needed() {
     if [[ ${GITHUB_EVENT_NAME=} == 'push' || ${GITHUB_EVENT_NAME=} == "scheduled" ]]; then
-        # Trigger upgrading to latest constraints when we are in push or schedule event. We use the
-        # random string so that it always triggers rebuilding layer in the docker image
-        # Each build that upgrades to latest constraints will get truly latest constraints, not those
-        # cached in the image because docker layer will get invalidated.
-        # This upgrade_to_newer_dependencies variable can later be overridden
-        # in case we find that any of the setup.* files changed (see below)
-        upgrade_to_newer_dependencies="${RANDOM}"
+        # Trigger upgrading to latest constraints when we are in push or schedule event
+        upgrade_to_newer_dependencies="true"
     fi
 }
 
@@ -358,9 +353,8 @@ function check_if_setup_files_changed() {
 
     if [[ $(count_changed_files) != "0" ]]; then
         # In case the setup files changed, we automatically force upgrading to newer dependencies
-        # no matter what was set before. We set it to random number to make sure that it will be
-        # always invalidating the layer in Docker that triggers installing the dependencies
-        upgrade_to_newer_dependencies="${RANDOM}"
+        # no matter what was set before.
+        upgrade_to_newer_dependencies="true"
     fi
     start_end::group_end
 }