make Cloud::vm_config() handle per-VM vm_overrides according to inlin…

…e docs

changelog/64610.fixed.md

@@ -0,0 +1 @@
+changed vm_config() to deep-merge vm_overrides of specific VM, instead of simple-merging the whole vm_overrides

salt/cloud/__init__.py

@@ -59,17 +59,17 @@ def _call(queue, args, kwargs):
             trace = traceback.format_exc()
             queue.put("KEYBOARDINT")
             queue.put("Keyboard interrupt")
-            queue.put("{}\n{}\n".format(ex, trace))
+            queue.put(f"{ex}\n{trace}\n")
         except Exception as ex:  # pylint: disable=broad-except
             trace = traceback.format_exc()
             queue.put("ERROR")
             queue.put("Exception")
-            queue.put("{}\n{}\n".format(ex, trace))
+            queue.put(f"{ex}\n{trace}\n")
         except SystemExit as ex:
             trace = traceback.format_exc()
             queue.put("ERROR")
             queue.put("System exit")
-            queue.put("{}\n{}\n".format(ex, trace))
+            queue.put(f"{ex}\n{trace}\n")
         return ret
 
     return _call
@@ -149,7 +149,7 @@ def enter_mainloop(
             " we bail out".format(target)
         )
         log.error(msg)
-        raise SaltCloudSystemExit("Exception caught\n{}".format(msg))
+        raise SaltCloudSystemExit(f"Exception caught\n{msg}")
     elif mapped_args is not None:
         iterable = [[queue, [arg], kwargs] for arg in mapped_args]
         ret = pool.map(func=target, iterable=iterable)
@@ -160,12 +160,12 @@ def enter_mainloop(
         if test in ["ERROR", "KEYBOARDINT"]:
             type_ = queue.get()
             trace = queue.get()
-            msg = "Caught {}, terminating workers\n".format(type_)
-            msg += "TRACE: {}\n".format(trace)
+            msg = f"Caught {type_}, terminating workers\n"
+            msg += f"TRACE: {trace}\n"
             log.error(msg)
             pool.terminate()
             pool.join()
-            raise SaltCloudSystemExit("Exception caught\n{}".format(msg))
+            raise SaltCloudSystemExit(f"Exception caught\n{msg}")
         elif test in ["END"] or (callback and callback(test)):
             pool.close()
             pool.join()
@@ -198,7 +198,7 @@ def __init__(self, path=None, opts=None, config_dir=None, pillars=None):
             for name, profile in pillars.pop("profiles", {}).items():
                 provider = profile["provider"].split(":")[0]
                 driver = next(iter(self.opts["providers"][provider].keys()))
-                profile["provider"] = "{}:{}".format(provider, driver)
+                profile["provider"] = f"{provider}:{driver}"
                 profile["profile"] = name
                 self.opts["profiles"].update({name: profile})
                 self.opts["providers"][provider][driver]["profiles"].update(
@@ -391,7 +391,7 @@ def create(self, provider, names, **kwargs):
         mapper = salt.cloud.Map(self._opts_defaults())
         providers = self.opts["providers"]
         if provider in providers:
-            provider += ":{}".format(next(iter(providers[provider].keys())))
+            provider += f":{next(iter(providers[provider].keys()))}"
         else:
             return False
         if isinstance(names, str):
@@ -432,7 +432,7 @@ def extra_action(self, names, provider, action, **kwargs):
         mapper = salt.cloud.Map(self._opts_defaults())
         providers = mapper.map_providers_parallel()
         if provider in providers:
-            provider += ":{}".format(next(iter(providers[provider].keys())))
+            provider += f":{next(iter(providers[provider].keys()))}"
         else:
             return False
         if isinstance(names, str):
@@ -517,7 +517,7 @@ def get_configured_providers(self):
         for alias, drivers in self.opts["providers"].items():
             if len(drivers) > 1:
                 for driver in drivers:
-                    providers.add("{}:{}".format(alias, driver))
+                    providers.add(f"{alias}:{driver}")
                 continue
             providers.add(alias)
         return providers
@@ -608,7 +608,7 @@ def map_providers(self, query="list_nodes", cached=False):
         pmap = {}
         for alias, drivers in self.opts["providers"].items():
             for driver, details in drivers.items():
-                fun = "{}.{}".format(driver, query)
+                fun = f"{driver}.{query}"
                 if fun not in self.clouds:
                     log.error("Public cloud provider %s is not available", driver)
                     continue
@@ -658,11 +658,11 @@ def map_providers_parallel(self, query="list_nodes", cached=False):
                 # for minimum information, Otherwise still use query param.
                 if (
                     opts.get("selected_query_option") is None
-                    and "{}.list_nodes_min".format(driver) in self.clouds
+                    and f"{driver}.list_nodes_min" in self.clouds
                 ):
                     this_query = "list_nodes_min"
 
-                fun = "{}.{}".format(driver, this_query)
+                fun = f"{driver}.{this_query}"
                 if fun not in self.clouds:
                     log.error("Public cloud provider %s is not available", driver)
                     continue
@@ -770,7 +770,7 @@ def _optimize_providers(self, providers):
                 provider_by_driver[name][alias] = data
 
         for driver, providers_data in provider_by_driver.items():
-            fun = "{}.optimize_providers".format(driver)
+            fun = f"{driver}.optimize_providers"
             if fun not in self.clouds:
                 log.debug("The '%s' cloud driver is unable to be optimized.", driver)
 
@@ -800,7 +800,7 @@ def location_list(self, lookup="all"):
             return data
 
         for alias, driver in lookups:
-            fun = "{}.avail_locations".format(driver)
+            fun = f"{driver}.avail_locations"
             if fun not in self.clouds:
                 # The capability to gather locations is not supported by this
                 # cloud module
@@ -841,7 +841,7 @@ def image_list(self, lookup="all"):
             return data
 
         for alias, driver in lookups:
-            fun = "{}.avail_images".format(driver)
+            fun = f"{driver}.avail_images"
             if fun not in self.clouds:
                 # The capability to gather images is not supported by this
                 # cloud module
@@ -881,7 +881,7 @@ def size_list(self, lookup="all"):
             return data
 
         for alias, driver in lookups:
-            fun = "{}.avail_sizes".format(driver)
+            fun = f"{driver}.avail_sizes"
             if fun not in self.clouds:
                 # The capability to gather sizes is not supported by this
                 # cloud module
@@ -1016,7 +1016,7 @@ def destroy(self, names, cached=False):
         else:
             log.info("Destroying in non-parallel mode.")
             for alias, driver, name in vms_to_destroy:
-                fun = "{}.destroy".format(driver)
+                fun = f"{driver}.destroy"
                 with salt.utils.context.func_globals_inject(
                     self.clouds[fun], __active_provider_name__=":".join([alias, driver])
                 ):
@@ -1049,7 +1049,7 @@ def destroy(self, names, cached=False):
             key_file = os.path.join(
                 self.opts["pki_dir"], "minions", minion_dict.get("id", name)
             )
-            globbed_key_file = glob.glob("{}.*".format(key_file))
+            globbed_key_file = glob.glob(f"{key_file}.*")
 
             if not os.path.isfile(key_file) and not globbed_key_file:
                 # There's no such key file!? It might have been renamed
@@ -1089,25 +1089,25 @@ def destroy(self, names, cached=False):
             )
             while True:
                 for idx, filename in enumerate(globbed_key_file):
-                    print(" {}: {}".format(idx, os.path.basename(filename)))
+                    print(f" {idx}: {os.path.basename(filename)}")
                 selection = input("Which minion key should be deleted(number)? ")
                 try:
                     selection = int(selection)
                 except ValueError:
-                    print("'{}' is not a valid selection.".format(selection))
+                    print(f"'{selection}' is not a valid selection.")
 
                 try:
                     filename = os.path.basename(globbed_key_file.pop(selection))
                 except Exception:  # pylint: disable=broad-except
                     continue
 
-                delete = input("Delete '{}'? [Y/n]? ".format(filename))
+                delete = input(f"Delete '{filename}'? [Y/n]? ")
                 if delete == "" or delete.lower().startswith("y"):
                     salt.utils.cloud.remove_key(self.opts["pki_dir"], filename)
-                    print("Deleted '{}'".format(filename))
+                    print(f"Deleted '{filename}'")
                     break
 
-                print("Did not delete '{}'".format(filename))
+                print(f"Did not delete '{filename}'")
                 break
 
         if names and not processed:
@@ -1137,7 +1137,7 @@ def reboot(self, names):
                 if node in names:
                     acts[prov].append(node)
         for prov, names_ in acts.items():
-            fun = "{}.reboot".format(prov)
+            fun = f"{prov}.reboot"
             for name in names_:
                 ret.append({name: self.clouds[fun](name)})
 
@@ -1154,7 +1154,7 @@ def create(self, vm_, local_master=True, sync_sleep=3):
         )
 
         alias, driver = vm_["provider"].split(":")
-        fun = "{}.create".format(driver)
+        fun = f"{driver}.create"
         if fun not in self.clouds:
             log.error(
                 "Creating '%s' using '%s' as the provider "
@@ -1219,7 +1219,7 @@ def create(self, vm_, local_master=True, sync_sleep=3):
 
         try:
             alias, driver = vm_["provider"].split(":")
-            func = "{}.create".format(driver)
+            func = f"{driver}.create"
             with salt.utils.context.func_globals_inject(
                 self.clouds[fun], __active_provider_name__=":".join([alias, driver])
             ):
@@ -1308,12 +1308,12 @@ def vm_config(name, main, provider, profile, overrides):
         :param dict main: The main cloud config
         :param dict provider: The provider config
         :param dict profile: The profile config
-        :param dict overrides: The vm's config overrides
+        :param dict overrides: a special dict that carries per-node options overrides (see CloudConfig:profile() documentation)
         """
         vm = main.copy()
         vm = salt.utils.dictupdate.update(vm, provider)
         vm = salt.utils.dictupdate.update(vm, profile)
-        vm.update(overrides)
+        vm = salt.utils.dictupdate.update(vm, overrides.get(name, {}))
         vm["name"] = name
         return vm
 
@@ -1356,7 +1356,7 @@ def run_profile(self, profile, names, vm_overrides=None):
         handle them
         """
         if profile not in self.opts["profiles"]:
-            msg = "Profile {} is not defined".format(profile)
+            msg = f"Profile {profile} is not defined"
             log.error(msg)
             return {"Error": msg}
 
@@ -1395,7 +1395,7 @@ def run_profile(self, profile, names, vm_overrides=None):
             if name in vms:
                 prov = vms[name]["provider"]
                 driv = vms[name]["driver"]
-                msg = "{} already exists under {}:{}".format(name, prov, driv)
+                msg = f"{name} already exists under {prov}:{driv}"
                 log.error(msg)
                 ret[name] = {"Error": msg}
                 continue
@@ -1541,14 +1541,12 @@ def do_function(self, prov, func, kwargs):
             raise SaltCloudSystemExit(
                 "More than one results matched '{}'. Please specify one of: {}".format(
                     prov,
-                    ", ".join(
-                        ["{}:{}".format(alias, driver) for (alias, driver) in matches]
-                    ),
+                    ", ".join([f"{alias}:{driver}" for (alias, driver) in matches]),
                 )
             )
 
         alias, driver = matches.pop()
-        fun = "{}.{}".format(driver, func)
+        fun = f"{driver}.{func}"
         if fun not in self.clouds:
             raise SaltCloudSystemExit(
                 "The '{}' cloud provider alias, for the '{}' driver, does "
@@ -1572,7 +1570,7 @@ def __filter_non_working_providers(self):
         """
         for alias, drivers in self.opts["providers"].copy().items():
             for driver in drivers.copy():
-                fun = "{}.get_configured_provider".format(driver)
+                fun = f"{driver}.get_configured_provider"
                 if fun not in self.clouds:
                     # Mis-configured provider that got removed?
                     log.warning(
@@ -1897,7 +1895,7 @@ def map_data(self, cached=False):
                     "The required profile, '{}', defined in the map "
                     "does not exist. The defined nodes, {}, will not "
                     "be created.".format(
-                        profile_name, ", ".join("'{}'".format(node) for node in nodes)
+                        profile_name, ", ".join(f"'{node}'" for node in nodes)
                     )
                 )
                 log.error(msg)
@@ -1930,7 +1928,7 @@ def map_data(self, cached=False):
 
                 # Update profile data with the map overrides
                 for setting in ("grains", "master", "minion", "volumes", "requires"):
-                    deprecated = "map_{}".format(setting)
+                    deprecated = f"map_{setting}"
                     if deprecated in overrides:
                         log.warning(
                             "The use of '%s' on the '%s' mapping has "

tests/pytests/unit/cloud/test_cloud.py

@@ -77,6 +77,7 @@ def test_vm_config_merger():
     Validate the vm's config is generated correctly.
 
     https://github.com/saltstack/salt/issues/49226
+    https://github.com/saltstack/salt/issues/64610
     """
     main = {
         "minion": {"master": "172.31.39.213"},
@@ -100,6 +101,10 @@ def test_vm_config_merger():
         "image": "ami-0a1fbca0e5b419fd1",
         "size": "t2.micro",
     }
+    vm_overrides = {
+        "test_vm": {"grains": {"meh2": "newbar", "meh3": "foo"}},
+        "other_vm": {"grains": {"meh1": "notused"}},
+    }
     expected = {
         "minion": {"master": "172.31.39.213"},
         "log_file": "var/log/salt/cloud.log",
@@ -108,7 +113,8 @@ def test_vm_config_merger():
         "grains": {
             "foo1": "bar",
             "foo2": "bang",
-            "meh2": "bar",
+            "meh3": "foo",
+            "meh2": "newbar",
             "meh1": "foo",
         },
         "availability_zone": "us-west-2b",
@@ -122,7 +128,67 @@ def test_vm_config_merger():
         "size": "t2.micro",
         "name": "test_vm",
     }
-    vm = Cloud.vm_config("test_vm", main, provider, profile, {})
+    vm = Cloud.vm_config("test_vm", main, provider, profile, vm_overrides)
+    assert expected == vm
+
+
+@pytest.mark.slow_test
+def test_vm_config_merger_nooverridevalue():
+    """
+    Validate the vm's config is generated correctly, even if no
+    applicable values are in the vm_override structure
+
+    https://github.com/saltstack/salt/issues/64610
+    """
+    main = {
+        "minion": {"master": "172.31.39.213"},
+        "log_file": "var/log/salt/cloud.log",
+        "pool_size": 10,
+    }
+    provider = {
+        "private_key": "dwoz.pem",
+        "grains": {"foo1": "bar", "foo2": "bang"},
+        "availability_zone": "us-west-2b",
+        "driver": "ec2",
+        "ssh_interface": "private_ips",
+        "ssh_username": "admin",
+        "location": "us-west-2",
+    }
+    profile = {
+        "profile": "default",
+        "grains": {"meh2": "bar", "meh1": "foo"},
+        "provider": "ec2-default:ec2",
+        "ssh_username": "admin",
+        "image": "ami-0a1fbca0e5b419fd1",
+        "size": "t2.micro",
+    }
+    vm_overrides = {
+        "test_vm": {"grains": {"meh2": "newbar", "meh3": "foo"}},
+        "other_vm": {"grains": {"meh1": "notused"}},
+    }
+    expected = {
+        "minion": {"master": "172.31.39.213"},
+        "log_file": "var/log/salt/cloud.log",
+        "pool_size": 10,
+        "private_key": "dwoz.pem",
+        "grains": {
+            "foo1": "bar",
+            "foo2": "bang",
+            "meh2": "bar",
+            "meh1": "foo",
+        },
+        "availability_zone": "us-west-2b",
+        "driver": "ec2",
+        "ssh_interface": "private_ips",
+        "ssh_username": "admin",
+        "location": "us-west-2",
+        "profile": "default",
+        "provider": "ec2-default:ec2",
+        "image": "ami-0a1fbca0e5b419fd1",
+        "size": "t2.micro",
+        "name": "test_vm2",
+    }
+    vm = Cloud.vm_config("test_vm2", main, provider, profile, vm_overrides)
     assert expected == vm