From e18969058acfd118d8767038266a9a320b71e4b0 Mon Sep 17 00:00:00 2001 From: mabulgu Date: Fri, 31 May 2024 23:20:52 +0300 Subject: [PATCH] replace commands with api call for replace --- Dockerfile | 2 +- README.md | 4 ++-- kfk/commands/clusters.py | 17 +++++++------ kfk/commands/connect/clusters.py | 12 ++++------ kfk/commands/connect/connectors.py | 15 +++++------- kfk/commands/topics.py | 15 +++++------- kfk/commands/users.py | 15 +++++------- kfk/config.py | 2 +- kfk/kubectl_command_builder.py | 8 ------- kfk/kubernetes_commons.py | 26 ++++++++++++++++++++ pyproject.toml | 2 +- tests/test_topics_command.py | 38 ++++++++++++++++++++---------- 12 files changed, 87 insertions(+), 69 deletions(-) diff --git a/Dockerfile b/Dockerfile index 0d28ff4..93c4866 100644 --- a/Dockerfile +++ b/Dockerfile @@ -4,7 +4,7 @@ RUN apk add --update \ curl \ && rm -rf /var/cache/apk/* RUN adduser -D kfkuser -RUN pip install strimzi-kafka-cli==0.1.0a79 +RUN pip install strimzi-kafka-cli==0.1.0a80 USER kfkuser RUN mkdir /home/kfkuser/.kube RUN curl https://raw.githubusercontent.com/SystemCraftsman/strimzi-kafka-cli/main/tests/files/yaml/kubeconfig -o /home/kfkuser/.kube/config diff --git a/README.md b/README.md index 8003241..2452531 100644 --- a/README.md +++ b/README.md @@ -84,10 +84,10 @@ Please see [pyproject.toml](https://github.com/SystemCraftsman/strimzi-kafka-cli kfk --version ``` -If you want to change the `kubectl` and `Strimzi resources` folder, you can simply change their version with the help of some environment variables in order to let Strimzi Kafka CLI download the version you want, or change the PATH of any if you want to use a custom kubectl or Strimzi binary folder. Current versions are recommended, so use these environment variables at your own risk. +You can change where you want to locate the `kubectl`, `Strimzi resources`, or `Strimzi CLI` files/folders. You can use the following environment variables: **STRIMZI_KAFKA_CLI_BASE_PATH:** Set this if you want to have a custom Strimzi Kafka CLI folder. It is `~/.strimzi-kafka-cli` as default. -**STRIMZI_KAFKA_CLI_STRIMZI_PATH:** Set this if you want to use a custom Strimzi/AMQ Streams. +**STRIMZI_KAFKA_CLI_STRIMZI_PATH:** Set this if you want to use a custom Strimzi/AMQ Streams. We only recommend this when using AMQ Streams instead of Strimzi. **STRIMZI_KAFKA_CLI_KUBECTL_PATH:** Set this if you want to use a custom kubectl. diff --git a/kfk/commands/clusters.py b/kfk/commands/clusters.py index 081aa20..41061b6 100644 --- a/kfk/commands/clusters.py +++ b/kfk/commands/clusters.py @@ -15,7 +15,11 @@ ) from kfk.config import STRIMZI_PATH, STRIMZI_VERSION from kfk.kubectl_command_builder import Kubectl -from kfk.kubernetes_commons import create_using_yaml, delete_using_yaml +from kfk.kubernetes_commons import ( + create_using_yaml, + delete_using_yaml, + replace_using_yaml, +) from kfk.messages import Messages from kfk.option_extensions import NotRequiredIf @@ -202,14 +206,9 @@ def alter(cluster, replicas, zk_replicas, config, delete_config, namespace): cluster_yaml = yaml.dump(cluster_dict) cluster_temp_file = create_temp_file(cluster_yaml) - os.system( - Kubectl() - .apply() - .from_file("{cluster_temp_file_path}") - .namespace(namespace) - .build() - .format(cluster_temp_file_path=cluster_temp_file.name) - ) + + replace_using_yaml(cluster_temp_file.name, namespace) + cluster_temp_file.close() else: os.system(Kubectl().edit().kafkas(cluster).namespace(namespace).build()) diff --git a/kfk/commands/connect/clusters.py b/kfk/commands/connect/clusters.py index b6ed060..42c1541 100644 --- a/kfk/commands/connect/clusters.py +++ b/kfk/commands/connect/clusters.py @@ -31,6 +31,7 @@ create_using_yaml, delete_object, delete_using_yaml, + replace_using_yaml, ) from kfk.messages import Errors, Messages from kfk.utils import is_valid_url @@ -362,14 +363,9 @@ def alter(cluster, replicas, config_file, namespace): cluster_yaml = yaml.dump(cluster_dict) cluster_temp_file = create_temp_file(cluster_yaml) - os.system( - Kubectl() - .replace() - .from_file("{cluster_temp_file_path}") - .namespace(namespace) - .build() - .format(cluster_temp_file_path=cluster_temp_file.name) - ) + + replace_using_yaml(cluster_temp_file.name, namespace) + cluster_temp_file.close() else: os.system(Kubectl().edit().kafkaconnects(cluster).namespace(namespace).build()) diff --git a/kfk/commands/connect/connectors.py b/kfk/commands/connect/connectors.py index ed3daf5..5a8b537 100644 --- a/kfk/commands/connect/connectors.py +++ b/kfk/commands/connect/connectors.py @@ -16,7 +16,11 @@ from kfk.config import STRIMZI_PATH, STRIMZI_VERSION from kfk.constants import SpecialTexts from kfk.kubectl_command_builder import Kubectl -from kfk.kubernetes_commons import create_using_yaml, delete_using_yaml +from kfk.kubernetes_commons import ( + create_using_yaml, + delete_using_yaml, + replace_using_yaml, +) CONNECTOR_SKIPPED_PROPERTIES = ( SpecialTexts.CONNECTOR_NAME, @@ -199,14 +203,7 @@ def alter(config_file, cluster, namespace): connector_yaml = yaml.dump(connector_dict) connector_temp_file = create_temp_file(connector_yaml) - os.system( - Kubectl() - .replace() - .from_file("{topic_temp_file_path}") - .namespace(namespace) - .build() - .format(topic_temp_file_path=connector_temp_file.name) - ) + replace_using_yaml(connector_temp_file.name, namespace) connector_temp_file.close() diff --git a/kfk/commands/topics.py b/kfk/commands/topics.py index 34a69c8..d573e7a 100644 --- a/kfk/commands/topics.py +++ b/kfk/commands/topics.py @@ -16,7 +16,11 @@ from kfk.config import STRIMZI_PATH, STRIMZI_VERSION from kfk.constants import KAFKA_PORT from kfk.kubectl_command_builder import Kubectl -from kfk.kubernetes_commons import create_using_yaml, delete_using_yaml +from kfk.kubernetes_commons import ( + create_using_yaml, + delete_using_yaml, + replace_using_yaml, +) from kfk.option_extensions import NotRequiredIf, RequiredIf @@ -249,14 +253,7 @@ def alter( topic_yaml = yaml.dump(topic_dict) topic_temp_file = create_temp_file(topic_yaml) - os.system( - Kubectl() - .apply() - .from_file("{topic_temp_file_path}") - .namespace(namespace) - .build() - .format(topic_temp_file_path=topic_temp_file.name) - ) + replace_using_yaml(topic_temp_file.name, namespace) topic_temp_file.close() diff --git a/kfk/commands/users.py b/kfk/commands/users.py index 35fe46e..10ec0fa 100644 --- a/kfk/commands/users.py +++ b/kfk/commands/users.py @@ -14,7 +14,11 @@ ) from kfk.config import STRIMZI_PATH, STRIMZI_VERSION from kfk.kubectl_command_builder import Kubectl -from kfk.kubernetes_commons import create_using_yaml, delete_using_yaml +from kfk.kubernetes_commons import ( + create_using_yaml, + delete_using_yaml, + replace_using_yaml, +) from kfk.option_extensions import NotRequiredIf, RequiredIf from kfk.utils import snake_to_camel_case @@ -306,14 +310,7 @@ def alter( user_yaml = yaml.dump(user_dict) user_temp_file = create_temp_file(user_yaml) - os.system( - Kubectl() - .apply() - .from_file("{user_temp_file_path}") - .namespace(namespace) - .build() - .format(user_temp_file_path=user_temp_file.name) - ) + replace_using_yaml(user_temp_file.name, namespace) user_temp_file.close() diff --git a/kfk/config.py b/kfk/config.py index 8a13ab7..55deedf 100644 --- a/kfk/config.py +++ b/kfk/config.py @@ -3,7 +3,7 @@ import sys from pathlib import Path -STRIMZI_VERSION = "0.40.0" +STRIMZI_VERSION = "0.41.0" KUBECTL_VERSION = "v1.29.3" diff --git a/kfk/kubectl_command_builder.py b/kfk/kubectl_command_builder.py index 6da9241..f5b2966 100644 --- a/kfk/kubectl_command_builder.py +++ b/kfk/kubectl_command_builder.py @@ -16,14 +16,6 @@ def get(self): self.cmd_str = self.cmd_str + SPACE + "get" return self - def apply(self): - self.cmd_str = self.cmd_str + SPACE + "apply" - return self - - def replace(self): - self.cmd_str = self.cmd_str + SPACE + "replace" - return self - def describe(self): self.cmd_str = self.cmd_str + SPACE + "describe" return self diff --git a/kfk/kubernetes_commons.py b/kfk/kubernetes_commons.py index c4afbea..d292aa7 100644 --- a/kfk/kubernetes_commons.py +++ b/kfk/kubernetes_commons.py @@ -89,6 +89,17 @@ def delete_using_yaml(file_path, namespace): ) +def replace_using_yaml(file_path, namespace): + _operate_using_yaml( + api_client, + file_path, + "replace", + yaml_objects=None, + verbose=True, + namespace=namespace, + ) + + def _operate_using_yaml( k8s_client, yaml_file=None, @@ -242,6 +253,21 @@ def _delete_using_yaml_object(k8s_api, yml_object, object_type, **kwargs): _delete_object(k8s_api, name, object_type, **kwargs) +@yaml_object_argument_filter +def _replace_using_yaml_object(k8s_api, yml_object, object_type, **kwargs): + if hasattr(k8s_api, f"replace_namespaced_{object_type}"): + if "namespace" in yml_object["metadata"]: + namespace = yml_object["metadata"]["namespace"] + kwargs["namespace"] = namespace + resp = getattr(k8s_api, f"replace_namespaced_{object_type}")( + body=yml_object, **kwargs + ) + else: + kwargs.pop("namespace", None) + resp = getattr(k8s_api, f"replace_{object_type}")(body=yml_object, **kwargs) + return resp + + def _delete_object(k8s_api, name, object_type, delete_options_version="V1", **kwargs): try: if hasattr(k8s_api, f"delete_namespaced_{object_type}"): diff --git a/pyproject.toml b/pyproject.toml index 9be71af..fb459df 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "strimzi-kafka-cli" -version = "0.1.0-alpha79" +version = "0.1.0-alpha80" description = "Command Line Interface for Strimzi Kafka Operator" authors = [{ name = "Aykut Bulgu", email = "aykut@systemcraftsman.com" }] readme = "README.md" diff --git a/tests/test_topics_command.py b/tests/test_topics_command.py index e00ed9f..e7aaf44 100644 --- a/tests/test_topics_command.py +++ b/tests/test_topics_command.py @@ -184,6 +184,8 @@ def test_create_topic(self, mock_create_using_yaml, mock_create_temp_file): result_topic_yaml = mock_create_temp_file.call_args[0][0] assert expected_topic_yaml == result_topic_yaml + mock_create_using_yaml.assert_called_once() + @mock.patch("kfk.commands.topics.create_temp_file") @mock.patch("kfk.commands.topics.create_using_yaml") def test_create_topic_with_config( @@ -256,9 +258,9 @@ def test_delete_topic(self, mock_delete_using_yaml): @mock.patch("kfk.commands.topics.create_temp_file") @mock.patch("kfk.commons.get_resource_yaml") - @mock.patch("kfk.commands.topics.os") + @mock.patch("kfk.commands.topics.replace_using_yaml") def test_alter_topic_with_no_params( - self, mock_os, mock_get_resource_yaml, mock_create_temp_file + self, mock_replace_using_yaml, mock_get_resource_yaml, mock_create_temp_file ): with open("tests/files/yaml/topic.yaml") as file: expected_topic_yaml = file.read() @@ -283,11 +285,13 @@ def test_alter_topic_with_no_params( result_topic_yaml = mock_create_temp_file.call_args[0][0] assert expected_topic_yaml == result_topic_yaml + mock_replace_using_yaml.assert_called_once() + @mock.patch("kfk.commands.topics.create_temp_file") @mock.patch("kfk.commons.get_resource_yaml") - @mock.patch("kfk.commands.topics.os") + @mock.patch("kfk.commands.topics.replace_using_yaml") def test_alter_topic_without_config( - self, mock_os, mock_get_resource_yaml, mock_create_temp_file + self, mock_replace_using_yaml, mock_get_resource_yaml, mock_create_temp_file ): with open("tests/files/yaml/topic.yaml") as file: topic_yaml = file.read() @@ -318,11 +322,13 @@ def test_alter_topic_without_config( result_topic_yaml = mock_create_temp_file.call_args[0][0] assert expected_topic_yaml == result_topic_yaml + mock_replace_using_yaml.assert_called_once() + @mock.patch("kfk.commands.topics.create_temp_file") @mock.patch("kfk.commons.get_resource_yaml") - @mock.patch("kfk.commands.topics.os") + @mock.patch("kfk.commands.topics.replace_using_yaml") def test_alter_topic_with_config( - self, mock_os, mock_get_resource_yaml, mock_create_temp_file + self, mock_replace_using_yaml, mock_get_resource_yaml, mock_create_temp_file ): with open("tests/files/yaml/topic.yaml") as file: topic_yaml = file.read() @@ -355,11 +361,13 @@ def test_alter_topic_with_config( result_topic_yaml = mock_create_temp_file.call_args[0][0] assert expected_topic_yaml == result_topic_yaml + mock_replace_using_yaml.assert_called_once() + @mock.patch("kfk.commands.topics.create_temp_file") @mock.patch("kfk.commons.get_resource_yaml") - @mock.patch("kfk.commands.topics.os") + @mock.patch("kfk.commands.topics.replace_using_yaml") def test_alter_topic_with_two_configs( - self, mock_os, mock_get_resource_yaml, mock_create_temp_file + self, mock_replace_using_yaml, mock_get_resource_yaml, mock_create_temp_file ): with open("tests/files/yaml/topic.yaml") as file: topic_yaml = file.read() @@ -394,11 +402,13 @@ def test_alter_topic_with_two_configs( result_topic_yaml = mock_create_temp_file.call_args[0][0] assert expected_topic_yaml == result_topic_yaml + mock_replace_using_yaml.assert_called_once() + @mock.patch("kfk.commands.topics.create_temp_file") @mock.patch("kfk.commons.get_resource_yaml") - @mock.patch("kfk.commands.topics.os") + @mock.patch("kfk.commands.topics.replace_using_yaml") def test_alter_topic_with_two_configs_delete_one_config( - self, mock_os, mock_get_resource_yaml, mock_create_temp_file + self, mock_replace_using_yaml, mock_get_resource_yaml, mock_create_temp_file ): with open("tests/files/yaml/topic_with_two_configs.yaml") as file: topic_yaml = file.read() @@ -427,11 +437,13 @@ def test_alter_topic_with_two_configs_delete_one_config( result_topic_yaml = mock_create_temp_file.call_args[0][0] assert expected_topic_yaml == result_topic_yaml + mock_replace_using_yaml.assert_called_once() + @mock.patch("kfk.commands.topics.create_temp_file") @mock.patch("kfk.commons.get_resource_yaml") - @mock.patch("kfk.commands.topics.os") + @mock.patch("kfk.commands.topics.replace_using_yaml") def test_alter_topic_with_two_configs_delete_two_configs( - self, mock_os, mock_get_resource_yaml, mock_create_temp_file + self, mock_replace_using_yaml, mock_get_resource_yaml, mock_create_temp_file ): with open("tests/files/yaml/topic_with_two_configs.yaml") as file: topic_yaml = file.read() @@ -461,3 +473,5 @@ def test_alter_topic_with_two_configs_delete_two_configs( expected_topic_yaml = file.read() result_topic_yaml = mock_create_temp_file.call_args[0][0] assert expected_topic_yaml == result_topic_yaml + + mock_replace_using_yaml.assert_called_once()