Make cluster creation idempotent (#125)

* Make cluster creation idempotent

* Fix a bug in the cluster name validation

* added changelog

* Fix sanity

* Update unit tests

* Update unit tests

* Update unit tests

* Update unit tests

* Fix when cluster_data is not present at all

Author: Thulium-Drake

changelogs/fragments/125-cluster-idempotent.yml

@@ -0,0 +1,3 @@
+---
+minor_changes:
+  - proxmox_cluster - cluster creation has been made idempotent (https://github.com/ansible-collections/community.proxmox/pull/125).

plugins/modules/proxmox_cluster.py

@@ -89,6 +89,7 @@
 """
 
 
+import re
 from ansible.module_utils.basic import AnsibleModule
 from ansible_collections.community.proxmox.plugins.module_utils.proxmox import (
     proxmox_auth_argument_spec, ProxmoxAnsible)
@@ -99,24 +100,29 @@ def cluster_create(self):
         cluster_name = self.module.params.get("cluster_name") or self.module.params.get("api_host")
         payload = {"clustername": cluster_name}
 
+        # Get cluster data
+        cluster_data = self.proxmox_api.cluster.config.totem.get()
+        # If we have data, check if we're already member of the desired cluster or a different one
+        if cluster_data and 'cluster_name' in cluster_data and cluster_data['cluster_name'] is not None:
+            if cluster_data['cluster_name'] == cluster_name:
+                self.module.exit_json(changed=False, msg="Cluster '{}' already present.".format(cluster_name), cluster=cluster_name)
+            else:
+                self.module.fail_json(msg='Error creating cluster: Node is already part of a different cluster - "{}"!'.format(cluster_data['cluster_name']))
+
         if self.module.params.get("link0") is not None:
             payload["link0"] = self.module.params.get("link0")
         if self.module.params.get("link1") is not None:
             payload["link1"] = self.module.params.get("link1")
 
         if self.module.check_mode:
-            cluster_objects = self.proxmox_api.cluster.config.nodes.get()
-            if len(cluster_objects) > 0:
-                self.module.fail_json(msg="Error while creating cluster: Node is already part of a cluster!")
-            else:
-                self.module.exit_json(changed=True, msg="Cluster '{}' would be created (check mode).".format(cluster_name), cluster=cluster_name)
-
-        try:
-            self.proxmox_api.cluster.config.post(**payload)
-        except Exception as e:
-            self.module.fail_json(msg="Error while creating cluster: {}".format(str(e)))
+            self.module.exit_json(changed=True, msg="Cluster '{}' would be created (check mode).".format(cluster_name), cluster=cluster_name)
+        else:
+            try:
+                self.proxmox_api.cluster.config.post(**payload)
+            except Exception as e:
+                self.module.fail_json(msg="Error while creating cluster: {}".format(str(e)))
 
-        self.module.exit_json(changed=True, msg="Cluster '{}' created.".format(cluster_name), cluster=cluster_name)
+            self.module.exit_json(changed=True, msg="Cluster '{}' created.".format(cluster_name), cluster=cluster_name)
 
     def cluster_join(self):
         master_ip = self.module.params.get("master_ip")
@@ -155,13 +161,15 @@ def proxmox_cluster_join_info_argument_spec():
     return dict()
 
 
-def validate_cluster_name(module, cluster_args, min_length=1, max_length=15):
-    if not isinstance(cluster_args['cluster_name'], str):
-        module.fail_json(msg="Cluster name must be a string.")
+def validate_cluster_name(module, min_length=1, max_length=15):
+    cluster_name = module.params.get("cluster_name")
 
-    if not (min_length <= len(cluster_args['cluster_name']) <= max_length):
+    if not (min_length <= len(cluster_name) <= max_length):
         module.fail_json(msg="Cluster name must be between {} and {} characters long.".format(min_length, max_length))
 
+    if not re.match(r"^[a-zA-Z0-9\-]+$", cluster_name):
+        module.fail_json(msg="Cluster name must contain only letters, digits, or hyphens.")
+
 
 def main():
     module_args = proxmox_auth_argument_spec()
@@ -188,7 +196,7 @@ def main():
     )
 
     proxmox = ProxmoxClusterAnsible(module)
-    validate_cluster_name(module, cluster_args)
+    validate_cluster_name(module)
 
     # The Proxmox VE API currently does not support leaving a cluster
     # or removing a node from a cluster. Therefore, we only support creating

tests/unit/plugins/modules/test_proxmox_cluster.py

@@ -107,7 +107,8 @@ def test_cluster_create(mock_api, mock_init, module_args_create):
     mock_api_instance.cluster.config.post.assert_called_once_with(**expected_payload)
 
 
-def test_validate_cluster_name_valid():
+def test_validate_cluster_name_valid(module_args_create):
     module = MagicMock(spec=AnsibleModule)
-    cluster_args = {"cluster_name": "devcluster"}
-    validate_cluster_name(module, cluster_args)
+    module.params = module_args_create
+
+    validate_cluster_name(module)