From cd376e990c6c19e8bc168e29f803887de0f37b41 Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Thu, 21 May 2026 11:06:24 +0200 Subject: [PATCH 1/3] feat: drop HELM_IS_INSTALL env-var dependency The HELM_IS_INSTALL env var only branched the final start_pods() call in chartreuse_upgrade.main(). Under ArgoCD the var is always empty (every sync is an apply, with no install/upgrade distinction), and under pure Helm with upgradeBeforeDeployment=true the safe behaviour is already to skip start_pods() and let the orchestrator restart pods after the hook. Always honour that. This removes the dependency cleanly: the tool now behaves identically whether invoked by Helm or ArgoCD. --- src/chartreuse/chartreuse_upgrade.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/chartreuse/chartreuse_upgrade.py b/src/chartreuse/chartreuse_upgrade.py index 630a6ec..3da2d99 100755 --- a/src/chartreuse/chartreuse_upgrade.py +++ b/src/chartreuse/chartreuse_upgrade.py @@ -85,7 +85,9 @@ def main() -> None: "false", "0", ) - HELM_IS_INSTALL: bool = os.environ.get("HELM_IS_INSTALL", "false").lower() not in ("", "false", "0") + # HELM_IS_INSTALL removed: under ArgoCD the env var is always empty, and under + # pure Helm flow with upgradeBeforeDeployment=true the existing pattern was to + # let the orchestrator restart pods after the hook. Always honour that. deployment_manager = KubernetesDeploymentManager(release_name=RELEASE_NAME, use_kubeconfig=None) chartreuse = Chartreuse( @@ -102,7 +104,7 @@ def main() -> None: if not ENABLE_STOP_PODS: return - if UPGRADE_BEFORE_DEPLOYMENT and not HELM_IS_INSTALL: + if UPGRADE_BEFORE_DEPLOYMENT: return try: From 965061467671c98d6d9fab204b3bd5840650bb6e Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Thu, 21 May 2026 11:07:31 +0200 Subject: [PATCH 2/3] test: update fixtures to drop HELM_IS_INSTALL Remove the HELM_IS_INSTALL key from the shared conftest fixture and from every per-test env-setup dict in test_chartreuse_upgrade_extended.py. The test that previously covered the 'UPGRADE_BEFORE_DEPLOYMENT and not HELM_IS_INSTALL' branch is retained but its docstring updated: the assertion (start_pods is not called when UPGRADE_BEFORE_DEPLOYMENT=true) remains valid under the simplified always-upgrade behaviour. --- src/chartreuse/tests/conftest.py | 1 - .../unit_tests/test_chartreuse_upgrade_extended.py | 10 +--------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/src/chartreuse/tests/conftest.py b/src/chartreuse/tests/conftest.py index 1232161..f90dff4 100644 --- a/src/chartreuse/tests/conftest.py +++ b/src/chartreuse/tests/conftest.py @@ -13,7 +13,6 @@ def configure_os_environ_mock(mocker: MockerFixture, additional_environment: dic "CHARTREUSE_RELEASE_NAME": "foo", "RUN_TEST_IN_KIND": os.environ.get("RUN_TEST_IN_KIND", ""), "CHARTREUSE_UPGRADE_BEFORE_DEPLOYMENT": "", - "HELM_IS_INSTALL": "", "CHARTREUSE_MULTI_CONFIG_PATH": "/mock/config.yaml", # Add Kubernetes environment variables to prevent config loading "KUBERNETES_SERVICE_HOST": "localhost", diff --git a/src/chartreuse/tests/unit_tests/test_chartreuse_upgrade_extended.py b/src/chartreuse/tests/unit_tests/test_chartreuse_upgrade_extended.py index 2c82f5e..e11de08 100644 --- a/src/chartreuse/tests/unit_tests/test_chartreuse_upgrade_extended.py +++ b/src/chartreuse/tests/unit_tests/test_chartreuse_upgrade_extended.py @@ -116,7 +116,6 @@ def test_main_multi_database_success(self, mocker: MockerFixture) -> None: "CHARTREUSE_ENABLE_STOP_PODS": "true", "CHARTREUSE_RELEASE_NAME": "test-release", "CHARTREUSE_UPGRADE_BEFORE_DEPLOYMENT": "false", - "HELM_IS_INSTALL": "false", }, ) @@ -166,7 +165,6 @@ def test_main_multi_database_no_migration_needed(self, mocker: MockerFixture) -> "CHARTREUSE_ENABLE_STOP_PODS": "true", "CHARTREUSE_RELEASE_NAME": "test-release", "CHARTREUSE_UPGRADE_BEFORE_DEPLOYMENT": "false", - "HELM_IS_INSTALL": "false", }, ) @@ -234,7 +232,6 @@ def test_main_multi_database_stop_pods_disabled(self, mocker: MockerFixture) -> "CHARTREUSE_ENABLE_STOP_PODS": "false", "CHARTREUSE_RELEASE_NAME": "test-release", "CHARTREUSE_UPGRADE_BEFORE_DEPLOYMENT": "false", - "HELM_IS_INSTALL": "false", }, ) @@ -246,7 +243,7 @@ def test_main_multi_database_stop_pods_disabled(self, mocker: MockerFixture) -> mock_k8s_instance.start_pods.assert_not_called() def test_main_multi_database_upgrade_before_deployment(self, mocker: MockerFixture) -> None: - """Test main function with UPGRADE_BEFORE_DEPLOYMENT and not HELM_IS_INSTALL.""" + """Test main function with UPGRADE_BEFORE_DEPLOYMENT enabled: start_pods should be skipped.""" mocker.patch("chartreuse.chartreuse_upgrade.ensure_safe_run") # Mock file existence for config validation @@ -282,7 +279,6 @@ def test_main_multi_database_upgrade_before_deployment(self, mocker: MockerFixtu "CHARTREUSE_ENABLE_STOP_PODS": "true", "CHARTREUSE_RELEASE_NAME": "test-release", "CHARTREUSE_UPGRADE_BEFORE_DEPLOYMENT": "true", - "HELM_IS_INSTALL": "false", }, ) @@ -334,7 +330,6 @@ def test_main_multi_database_start_pods_failure(self, mocker: MockerFixture) -> "CHARTREUSE_ENABLE_STOP_PODS": "true", "CHARTREUSE_RELEASE_NAME": "test-release", "CHARTREUSE_UPGRADE_BEFORE_DEPLOYMENT": "false", - "HELM_IS_INSTALL": "false", }, ) @@ -390,7 +385,6 @@ def test_main_single_database_success(self, mocker: MockerFixture) -> None: "CHARTREUSE_ENABLE_STOP_PODS": "true", "CHARTREUSE_RELEASE_NAME": "test-release", "CHARTREUSE_UPGRADE_BEFORE_DEPLOYMENT": "false", - "HELM_IS_INSTALL": "false", }, clear=True, ) @@ -450,7 +444,6 @@ def test_main_single_database_default_values(self, mocker: MockerFixture) -> Non "CHARTREUSE_ENABLE_STOP_PODS": "false", "CHARTREUSE_RELEASE_NAME": "test-release", "CHARTREUSE_UPGRADE_BEFORE_DEPLOYMENT": "false", - "HELM_IS_INSTALL": "false", }, clear=True, ) @@ -523,7 +516,6 @@ def test_boolean_parsing_variations(self, mocker: MockerFixture, bool_str: str, "CHARTREUSE_ENABLE_STOP_PODS": bool_str, # Test this boolean parsing "CHARTREUSE_RELEASE_NAME": "test-release", "CHARTREUSE_UPGRADE_BEFORE_DEPLOYMENT": "false", - "HELM_IS_INSTALL": "false", }, clear=True, ) From 340972fb720bc4e42e7f5d79e1aebf167c34af4a Mon Sep 17 00:00:00 2001 From: Arthur Le Roux Date: Thu, 21 May 2026 11:07:43 +0200 Subject: [PATCH 3/3] docs: remove HELM_IS_INSTALL from multi-database.md The env var is no longer consumed by the tool. See the preceding feat commit for context. --- docs/multi-database.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/multi-database.md b/docs/multi-database.md index d715d1f..1bcfe2c 100644 --- a/docs/multi-database.md +++ b/docs/multi-database.md @@ -67,7 +67,6 @@ For multi-database mode: - `CHARTREUSE_ENABLE_STOP_PODS`: Whether to stop pods during migration (optional, default: true) - `CHARTREUSE_RELEASE_NAME`: Kubernetes release name - `CHARTREUSE_UPGRADE_BEFORE_DEPLOYMENT`: Whether to upgrade before deployment (optional, default: false) -- `HELM_IS_INSTALL`: Whether this is a Helm install operation (optional, default: false) ## Usage