[Backport][0.8.4] - Backports for #2288

CHANGELOG-0.8.md

@@ -2,6 +2,10 @@
 
 ## [0.8.4] 2021-0x.xx
 
+### Added
+
+- [#2288](https://github.com/epiphany-platform/epiphany/issues/2288) - Allow to preserve OS images when run 'epicli apply' for existing cluster
+
 ### Fixed
 
 - [#2098](https://github.com/epiphany-platform/epiphany/issues/2098) - The default values can't be changed in cluster config file for virtual machine

core/src/epicli/cli/engine/ApplyEngine.py

@@ -5,7 +5,8 @@
 
 from cli.helpers.Step import Step
 from cli.helpers.doc_list_helpers import select_single
-from cli.helpers.build_saver import save_manifest, get_inventory_path
+from cli.helpers.build_saver import save_manifest, get_inventory_path, get_manifest_path, get_build_path
+from cli.helpers.data_loader import load_manifest_docs
 from cli.helpers.yaml_helpers import safe_load_all
 from cli.helpers.Log import Log
 from cli.engine.providers.provider_class_loader import provider_class_loader
@@ -29,6 +30,7 @@ def __init__(self, input_data):
         self.input_docs = []
         self.configuration_docs = []
         self.infrastructure_docs = []
+        self.manifest_docs = []
 
     def __enter__(self):
         return self
@@ -59,9 +61,12 @@ def process_input_docs(self):
             schema_validator.run()
 
     def process_infrastructure_docs(self):
+        # Load any posible existing manifest docs
+        self.load_manifest_docs()
+
         # Build the infrastructure docs
         with provider_class_loader(self.cluster_model.provider, 'InfrastructureBuilder')(
-                self.input_docs) as infrastructure_builder:
+                self.input_docs, self.manifest_docs) as infrastructure_builder:
             self.infrastructure_docs = infrastructure_builder.run()
 
         # Validate infrastructure documents
@@ -82,16 +87,10 @@ def collect_infrastructure_config(self):
                 [*self.configuration_docs, *self.infrastructure_docs]) as config_collector:
             config_collector.run()
 
-    def validate(self):
-        self.process_input_docs()
-
-        self.process_configuration_docs()
-
-        self.process_infrastructure_docs()
-
-        save_manifest([*self.input_docs, *self.configuration_docs, *self.infrastructure_docs], self.cluster_model.specification.name)
-
-        return 0
+    def load_manifest_docs(self):
+        path_to_manifest = get_manifest_path(self.cluster_model.specification.name)
+        if os.path.isfile(path_to_manifest):
+            self.manifest_docs = load_manifest_docs(get_build_path(self.cluster_model.specification.name))
 
     def assert_no_master_downscale(self):
         components = self.cluster_model.specification.components

core/src/epicli/cli/engine/providers/any/InfrastructureBuilder.py

@@ -3,10 +3,11 @@
 from cli.version import VERSION
 
 class InfrastructureBuilder(Step):
-    def __init__(self, docs):
+    def __init__(self, docs, manifest_docs=[]):
         super().__init__(__name__)
         self.cluster_model = select_single(docs, lambda x: x.kind == 'epiphany-cluster')
         self.docs = docs
+        self.manifest_docs = manifest_docs
 
     def run(self):
         infrastructure_docs = select_all(self.docs, lambda x: x.kind.startswith('infrastructure/'))

core/src/epicli/cli/engine/providers/aws/InfrastructureBuilder.py

@@ -16,13 +16,14 @@
 
 
 class InfrastructureBuilder(Step):
-    def __init__(self, docs):
+    def __init__(self, docs, manifest_docs=[]):
         super().__init__(__name__)
         self.cluster_model = select_single(docs, lambda x: x.kind == 'epiphany-cluster')
         self.cluster_name = self.cluster_model.specification.name.lower()
         self.cluster_prefix = self.cluster_model.specification.prefix.lower()
         self.use_network_security_groups = self.cluster_model.specification.cloud.network.use_network_security_groups
         self.docs = docs
+        self.manifest_docs = manifest_docs
 
     def run(self):
         infrastructure = []
@@ -136,7 +137,7 @@ def get_efs_config(self):
         return efs_config
 
     def get_autoscaling_group(self, component_key, component_value, subnets_to_create, index):
-        autoscaling_group = dict_to_objdict(deepcopy(self.get_virtual_machine(component_value, self.cluster_model, self.docs)))
+        autoscaling_group = dict_to_objdict(deepcopy(self.get_virtual_machine(component_value)))
         autoscaling_group.specification.cluster_name = self.cluster_name
         autoscaling_group.specification.name = resource_name(self.cluster_prefix, self.cluster_name, 'asg' + '-' + str(index), component_key)
         autoscaling_group.specification.count = component_value.count
@@ -245,6 +246,36 @@ def add_security_rules_inbound_efs(self, infrastructure, security_group):
             rules.append(objdict_to_dict(rule))
         security_group.specification.rules = rules
 
+    def get_virtual_machine(self, component_value):
+        machine_selector = component_value.machine
+        model_with_defaults = select_first(self.docs, lambda x: x.kind == 'infrastructure/virtual-machine' and
+                                                                 x.name == machine_selector)
+
+        # Merge with defaults
+        if model_with_defaults is None:
+            model_with_defaults = merge_with_defaults(self.cluster_model.provider, 'infrastructure/virtual-machine',
+                                                      machine_selector, self.docs)
+
+        # finally check if we are trying to re-apply a configuration.
+        if self.manifest_docs:
+            manifest_vm_config = select_first(self.manifest_docs, lambda x: x.name == machine_selector and x.kind == 'infrastructure/virtual-machine')
+            manifest_firstvm_config = select_first(self.manifest_docs, lambda x: x.kind == 'infrastructure/virtual-machine')
+
+            if manifest_vm_config  is not None and model_with_defaults.specification.os_full_name == manifest_vm_config.specification.os_full_name:
+                return model_with_defaults
+
+            if model_with_defaults.specification.os_full_name == manifest_firstvm_config.specification.os_full_name:
+                return model_with_defaults
+
+            self.logger.warning(f"Re-applying a different OS image might lead to data loss and/or other issues. Preserving the existing OS image used for VM definition '{machine_selector}'.")
+
+            if manifest_vm_config  is not None:
+                model_with_defaults.specification.os_full_name = manifest_vm_config.specification.os_full_name
+            else:
+                model_with_defaults.specification.os_full_name = manifest_firstvm_config.specification.os_full_name
+
+        return model_with_defaults
+
     @staticmethod
     def efs_add_mount_target_config(efs_config, subnet):
         target = select_first(efs_config.specification.mount_targets,
@@ -275,17 +306,6 @@ def get_config_or_default(docs, kind):
             config['version'] = VERSION
         return config
 
-    @staticmethod
-    def get_virtual_machine(component_value, cluster_model, docs):
-        machine_selector = component_value.machine
-        model_with_defaults = select_first(docs, lambda x: x.kind == 'infrastructure/virtual-machine' and
-                                                                 x.name == machine_selector)
-        if model_with_defaults is None:
-            model_with_defaults = merge_with_defaults(cluster_model.provider, 'infrastructure/virtual-machine',
-                                                      machine_selector, docs)
-
-        return model_with_defaults
-
     @staticmethod
     def rule_exists_in_list(rule_list, rule_to_check):
         for rule in rule_list:

core/src/epicli/cli/engine/providers/azure/InfrastructureBuilder.py

@@ -12,7 +12,7 @@
 from cli.version import VERSION
 
 class InfrastructureBuilder(Step):
-    def __init__(self, docs):
+    def __init__(self, docs, manifest_docs=[]):
         super().__init__(__name__)
         self.cluster_model = select_single(docs, lambda x: x.kind == 'epiphany-cluster')
         self.cluster_name = self.cluster_model.specification.name.lower()
@@ -22,6 +22,7 @@ def __init__(self, docs):
         self.use_network_security_groups = self.cluster_model.specification.cloud.network.use_network_security_groups
         self.use_public_ips = self.cluster_model.specification.cloud.use_public_ips
         self.docs = docs
+        self.manifest_docs = manifest_docs
 
     def run(self):
         infrastructure = []
@@ -42,7 +43,7 @@ def run(self):
 
             # The vm config also contains some other stuff we use for network and security config.
             # So get it here and pass it allong.
-            vm_config = self.get_virtual_machine(component_value, self.cluster_model, self.docs)
+            vm_config = self.get_virtual_machine(component_value)
 
             # If there are no security groups Ansible provisioning will fail because
             # SSH is not allowed then with public IPs on Azure.
@@ -206,21 +207,40 @@ def get_vm(self, component_key, component_value, vm_config, availability_set, ne
             vm.specification.availability_set_name = availability_set.specification.name
         return vm
 
+    def get_virtual_machine(self, component_value):
+        machine_selector = component_value.machine
+        model_with_defaults = select_first(self.docs, lambda x: x.kind == 'infrastructure/virtual-machine' and
+                                                                 x.name == machine_selector)
+
+        # Merge with defaults
+        if model_with_defaults is None:
+            model_with_defaults = merge_with_defaults(self.cluster_model.provider, 'infrastructure/virtual-machine',
+                                                      machine_selector, self.docs)
+
+        # finally check if we are trying to re-apply a configuration.
+        if self.manifest_docs:
+            manifest_vm_config = select_first(self.manifest_docs, lambda x: x.name == machine_selector and x.kind == 'infrastructure/virtual-machine')
+            manifest_firstvm_config = select_first(self.manifest_docs, lambda x: x.kind == 'infrastructure/virtual-machine')
+
+            if manifest_vm_config  is not None and model_with_defaults.specification.storage_image_reference == manifest_vm_config.specification.storage_image_reference:
+                return model_with_defaults
+
+            if model_with_defaults.specification.storage_image_reference == manifest_firstvm_config.specification.storage_image_reference:
+                return model_with_defaults
+
+            self.logger.warning(f"Re-applying a different OS image might lead to data loss and/or other issues. Preserving the existing OS image used for VM definition '{machine_selector}'.")
+
+            if manifest_vm_config  is not None:
+                model_with_defaults.specification.storage_image_reference = dict_to_objdict(deepcopy(manifest_vm_config.specification.storage_image_reference))
+            else:
+                model_with_defaults.specification.storage_image_reference = dict_to_objdict(deepcopy(manifest_firstvm_config.specification.storage_image_reference))
+
+        return model_with_defaults
+
     @staticmethod
     def get_config_or_default(docs, kind):
         config = select_first(docs, lambda x: x.kind == kind)
         if config is None:
             config = load_yaml_obj(types.DEFAULT, 'azure', kind)
             config['version'] = VERSION
         return config
-
-    @staticmethod
-    def get_virtual_machine(component_value, cluster_model, docs):
-        machine_selector = component_value.machine
-        model_with_defaults = select_first(docs, lambda x: x.kind == 'infrastructure/virtual-machine' and
-                                                                 x.name == machine_selector)
-        if model_with_defaults is None:
-            model_with_defaults = merge_with_defaults(cluster_model.provider, 'infrastructure/virtual-machine',
-                                                      machine_selector, docs)
-
-        return model_with_defaults

core/src/epicli/cli/epicli.py

@@ -93,9 +93,6 @@ def debug_level(x):
     upgrade_parser(subparsers)
     delete_parser(subparsers)
     test_parser(subparsers)
-    '''
-    validate_parser(subparsers)
-    '''
     backup_parser(subparsers)
     recovery_parser(subparsers)
 
@@ -246,23 +243,6 @@ def run_test(args):
     sub_parser.set_defaults(func=run_test)
 
 
-'''
-def validate_parser(subparsers):
-    sub_parser = subparsers.add_parser('verify', description='Validates the configuration from file by executing a dry '
-                                                             'run without changing the physical '
-                                                             'infrastructure/configuration')
-    sub_parser.add_argument('-f', '--file', dest='file', type=str,
-                            help='File with infrastructure/configuration definitions to use.')
-
-    def run_validate(args):
-        adjust_paths_from_file(args)
-        with ApplyEngine(args) as engine:
-            return engine.validate()
-
-    sub_parser.set_defaults(func=run_validate)    
-'''
-
-
 def backup_parser(subparsers):
     """Configure and execute backup of cluster components."""
 

core/src/epicli/cli/helpers/build_saver.py

@@ -88,6 +88,10 @@ def get_inventory_path(cluster_name):
     return os.path.join(get_build_path(cluster_name), INVENTORY_FILE_NAME)
 
 
+def get_manifest_path(cluster_name):
+    return os.path.join(get_build_path(cluster_name), MANIFEST_FILE_NAME)
+
+
 def get_inventory_path_for_build(build_directory):
     build_version = check_build_output_version(build_directory)
     inventory =  os.path.join(build_directory, INVENTORY_FILE_NAME)

core/src/epicli/tests/engine/providers/azure/test_AzureConfigBuilder.py

@@ -68,7 +68,7 @@ def test_get_public_ip_should_set_proper_values_to_model():
     component_value = dict_to_objdict({
         'machine': 'kubernetes-master-machine'
     })
-    vm_config = builder.get_virtual_machine(component_value, cluster_model, [])
+    vm_config = builder.get_virtual_machine(component_value)
 
     actual = builder.get_public_ip('kubernetes_master', component_value, vm_config, 1)
 
@@ -84,7 +84,7 @@ def test_get_network_interface_should_set_proper_values_to_model():
     component_value = dict_to_objdict({
         'machine': 'kubernetes-master-machine'
     })
-    vm_config = builder.get_virtual_machine(component_value, cluster_model, [])
+    vm_config = builder.get_virtual_machine(component_value)
 
     actual = builder.get_network_interface(
                                 'kubernetes_master',
@@ -119,6 +119,7 @@ def get_cluster_model(address_pool='10.22.0.0/22', cluster_name='EpiphanyTestClu
     cluster_model = dict_to_objdict({
         'kind': 'epiphany-cluster',
         'provider': 'azure',
+        'name': 'default',
         'specification': {
             'name': cluster_name,
             'prefix': 'prefix',