From ef02f7c73f6721fbfe632021e0c33513a83bb6ad Mon Sep 17 00:00:00 2001 From: knatsu-summer Date: Tue, 27 Aug 2024 19:33:05 +0900 Subject: [PATCH 1/6] only use replace_resource if resource already exists Signed-off-by: knatsu-summer --- oper8/deploy_manager/openshift_deploy_manager.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/oper8/deploy_manager/openshift_deploy_manager.py b/oper8/deploy_manager/openshift_deploy_manager.py index ac886d7..7592939 100644 --- a/oper8/deploy_manager/openshift_deploy_manager.py +++ b/oper8/deploy_manager/openshift_deploy_manager.py @@ -920,8 +920,10 @@ def _apply(self, resource_definition, method: DeployMethod): # If the resource requires a replace operation then use put. Otherwise use # server side apply if ( - req_replace or method is DeployMethod.REPLACE - ) and method != DeployMethod.UPDATE: + (req_replace or method is DeployMethod.REPLACE) + and method != DeployMethod.UPDATE + and current != {} + ): apply_res = self._replace_resource( resource_definition, ) From 05467f34fc5da147261ea6a430f68330efdaf3c5 Mon Sep 17 00:00:00 2001 From: knatsu-summer Date: Tue, 27 Aug 2024 20:06:13 +0900 Subject: [PATCH 2/6] update pytest Signed-off-by: knatsu-summer --- tests/deploy_manager/test_openshift_deploy_manager.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/deploy_manager/test_openshift_deploy_manager.py b/tests/deploy_manager/test_openshift_deploy_manager.py index 1118461..25b66af 100644 --- a/tests/deploy_manager/test_openshift_deploy_manager.py +++ b/tests/deploy_manager/test_openshift_deploy_manager.py @@ -222,6 +222,15 @@ def track_apply(resource_definition): dm._apply_resource = track_apply + # Use apply instead of replace when first deploying + success, changed = dm.deploy( + resource_definitions=make_obj_states(end_cluster_state), + method=DeployMethod.REPLACE, + ) + assert success + assert changed + assert len(replace_called) == 0 and len(apply_called) == 1 + success, changed = dm.deploy( resource_definitions=make_obj_states(end_cluster_state), method=DeployMethod.REPLACE, From f7ceb8b99975fa0f5ed5d6af015718e79ef81415 Mon Sep 17 00:00:00 2001 From: knatsu-summer Date: Wed, 28 Aug 2024 20:18:58 +0900 Subject: [PATCH 3/6] update unit test for replace option Signed-off-by: knatsu-summer --- .../test_openshift_deploy_manager.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/deploy_manager/test_openshift_deploy_manager.py b/tests/deploy_manager/test_openshift_deploy_manager.py index 25b66af..ed5b4ca 100644 --- a/tests/deploy_manager/test_openshift_deploy_manager.py +++ b/tests/deploy_manager/test_openshift_deploy_manager.py @@ -197,11 +197,17 @@ def test_deploy_method_resource(): """ start_cluster_state = {"test": {"Foo": {"foo.bar.com/v1": {}}}} end_cluster_state = {"test": {"Foo": {"foo.bar.com/v1": {"bar": {}}}}} + replace_apply_resource = { + "kind": "Foo", + "apiVersion": "foo.bar.com/v1", + "metadata": {"name": "bar", "namespace": "test"}, + "spec": {"some": "key_1"}, + } end_apply_resource = { "kind": "Foo", "apiVersion": "foo.bar.com/v1", "metadata": {"name": "bar", "namespace": "test"}, - "spec": {"some": "key"}, + "spec": {"some": "key_2"}, } dm = setup_testable_manager(cluster_state=start_cluster_state) dm._requires_replace = lambda *args, **kwargs: True @@ -232,19 +238,18 @@ def track_apply(resource_definition): assert len(replace_called) == 0 and len(apply_called) == 1 success, changed = dm.deploy( - resource_definitions=make_obj_states(end_cluster_state), - method=DeployMethod.REPLACE, + resource_definitions=[replace_apply_resource], method=DeployMethod.REPLACE, ) assert success assert changed - assert len(replace_called) == 1 and len(apply_called) == 0 + assert len(replace_called) == 1 and len(apply_called) == 1 success, changed = dm.deploy( resource_definitions=[end_apply_resource], method=DeployMethod.UPDATE ) assert success assert changed - assert len(replace_called) == 1 and len(apply_called) == 1 + assert len(replace_called) == 1 and len(apply_called) == 2 def test_deploy_method_update_resource(): From 4175299b4411e46568701c1ee3e60d92f5141a27 Mon Sep 17 00:00:00 2001 From: knatsu-summer Date: Wed, 28 Aug 2024 21:04:09 +0900 Subject: [PATCH 4/6] fix format Signed-off-by: knatsu-summer --- tests/deploy_manager/test_openshift_deploy_manager.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/deploy_manager/test_openshift_deploy_manager.py b/tests/deploy_manager/test_openshift_deploy_manager.py index ed5b4ca..eb242aa 100644 --- a/tests/deploy_manager/test_openshift_deploy_manager.py +++ b/tests/deploy_manager/test_openshift_deploy_manager.py @@ -238,7 +238,8 @@ def track_apply(resource_definition): assert len(replace_called) == 0 and len(apply_called) == 1 success, changed = dm.deploy( - resource_definitions=[replace_apply_resource], method=DeployMethod.REPLACE, + resource_definitions=[replace_apply_resource], + method=DeployMethod.REPLACE, ) assert success assert changed From a2622cf6e57fbef48da4d30954c7382dd09acdf8 Mon Sep 17 00:00:00 2001 From: knatsu-summer Date: Wed, 28 Aug 2024 21:12:55 +0900 Subject: [PATCH 5/6] fix format Signed-off-by: knatsu-summer --- tests/deploy_manager/test_openshift_deploy_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/deploy_manager/test_openshift_deploy_manager.py b/tests/deploy_manager/test_openshift_deploy_manager.py index eb242aa..22b2dca 100644 --- a/tests/deploy_manager/test_openshift_deploy_manager.py +++ b/tests/deploy_manager/test_openshift_deploy_manager.py @@ -238,7 +238,7 @@ def track_apply(resource_definition): assert len(replace_called) == 0 and len(apply_called) == 1 success, changed = dm.deploy( - resource_definitions=[replace_apply_resource], + resource_definitions=[replace_apply_resource], method=DeployMethod.REPLACE, ) assert success From 6c2845f67fc053ad0b6de1884e4c7121de14c8bd Mon Sep 17 00:00:00 2001 From: Gabe Goodhart Date: Wed, 28 Aug 2024 10:22:09 -0600 Subject: [PATCH 6/6] test: Bug fix in PUT implementation in kub_mock The update body was accidentally being discarded in favor of the current cluster state. Branch: deploymeshod_replace Signed-off-by: Gabe Goodhart --- oper8/test_helpers/kub_mock.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/oper8/test_helpers/kub_mock.py b/oper8/test_helpers/kub_mock.py index ec480e1..df52c61 100644 --- a/oper8/test_helpers/kub_mock.py +++ b/oper8/test_helpers/kub_mock.py @@ -791,9 +791,9 @@ def current_state_put(self, api_endpoint, api_version, kind, body, is_status=Fal content.update({"status": body.get("status", {})}) updated_content = content else: - if "status" in content: - del content["status"] - updated_content = content + if "status" in body: + del body["status"] + updated_content = body log.debug3( "Updating [%s/%s/%s/%s] with body: %s", namespace, @@ -832,6 +832,7 @@ def current_state_patch(self, api_endpoint, api_version, kind, body): name=name, ) log.debug3("Current Content: %s", content) + log.debug3("Update body: %s", body) # If the content has a status code, unpack it status_code = 200