For k8s, use provisioner-managed volumes when an absolute host path i…

…s not specified. (#741)

In kind, when we bind-mount a host directory it is first mounted into the kind container at /mnt, then into the pod at the desired location.

We accidentally picked this up for full-blown k8s, and were creating volumes at /mnt.  This changes the behavior for both kind and regular k8s so that bind mounts are only allowed if a fully-qualified path is specified.  If no path is specified at all, a default storageClass is assumed to be present, and the volume managed by a provisioner.

Eg, for kind, the default provisioner is: https://github.com/rancher/local-path-provisioner

```
stack: test
deploy-to: k8s-kind
config:
  test-variable-1: test-value-1
network:
  ports:
    test:
     - '80'
volumes:
  # this will be bind-mounted to a host-path
  test-data-bind: /srv/data
  # this will be managed by the k8s node
  test-data-auto:
configmaps:
  test-config: ./configmap/test-config
```

Reviewed-on: https://git.vdb.to/cerc-io/stack-orchestrator/pulls/741
Co-authored-by: Thomas E Lackey <[email protected]>
Co-committed-by: Thomas E Lackey <[email protected]>

stack_orchestrator/constants.py

@@ -27,6 +27,12 @@
 deploy_to_key = "deploy-to"
 network_key = "network"
 http_proxy_key = "http-proxy"
-image_resigtry_key = "image-registry"
+image_registry_key = "image-registry"
+configmaps_key = "configmaps"
+resources_key = "resources"
+volumes_key = "volumes"
+security_key = "security"
+annotations_key = "annotations"
+labels_key = "labels"
 kind_config_filename = "kind-config.yml"
 kube_config_filename = "kubeconfig.yml"

stack_orchestrator/data/compose/docker-compose-test.yml

@@ -7,11 +7,13 @@ services:
       CERC_TEST_PARAM_1: ${CERC_TEST_PARAM_1:-FAILED}
       CERC_TEST_PARAM_2: "CERC_TEST_PARAM_2_VALUE"
     volumes:
-      - test-data:/data
+      - test-data-bind:/data
+      - test-data-auto:/data2
       - test-config:/config:ro
     ports:
       - "80"
 
 volumes:
-  test-data:
+  test-data-bind:
+  test-data-auto:
   test-config:

stack_orchestrator/data/container-build/cerc-test-container/run.sh

@@ -1,19 +1,38 @@
 #!/usr/bin/env bash
 set -e
+
 if [ -n "$CERC_SCRIPT_DEBUG" ]; then
   set -x
 fi
-# Test if the container's filesystem is old (run previously) or new
-EXISTSFILENAME=/data/exists
+
 echo "Test container starting"
-if [[ -f "$EXISTSFILENAME" ]];
-then
-    TIMESTAMP=`cat $EXISTSFILENAME`
-    echo "Filesystem is old, created: $TIMESTAMP" 
+
+DATA_DEVICE=$(df | grep "/data$" | awk '{ print $1 }')
+if [[ -n "$DATA_DEVICE" ]]; then
+  echo "/data: MOUNTED dev=${DATA_DEVICE}"
+else
+  echo "/data: not mounted"
+fi
+
+DATA2_DEVICE=$(df | grep "/data2$" | awk '{ print $1 }')
+if [[ -n "$DATA_DEVICE" ]]; then
+  echo "/data2: MOUNTED dev=${DATA2_DEVICE}"
 else
-    echo "Filesystem is fresh"
-    echo `date` > $EXISTSFILENAME
+  echo "/data2: not mounted"
 fi
+
+# Test if the container's filesystem is old (run previously) or new
+for d in /data /data2; do
+  if [[ -f "$d/exists" ]];
+  then
+      TIMESTAMP=`cat $d/exists`
+      echo "$d filesystem is old, created: $TIMESTAMP"
+  else
+      echo "$d filesystem is fresh"
+      echo `date` > $d/exists
+  fi
+done
+
 if [ -n "$CERC_TEST_PARAM_1" ]; then
   echo "Test-param-1: ${CERC_TEST_PARAM_1}"
 fi

stack_orchestrator/deploy/deployment_create.py

@@ -27,6 +27,7 @@
 from stack_orchestrator.util import (get_stack_file_path, get_parsed_deployment_spec, get_parsed_stack_config,
                                      global_options, get_yaml, get_pod_list, get_pod_file_path, pod_has_scripts,
                                      get_pod_script_paths, get_plugin_code_paths, error_exit, env_var_map_from_file)
+from stack_orchestrator.deploy.spec import Spec
 from stack_orchestrator.deploy.deploy_types import LaconicStackSetupCommand
 from stack_orchestrator.deploy.deployer_factory import getDeployerConfigGenerator
 from stack_orchestrator.deploy.deployment_context import DeploymentContext
@@ -111,6 +112,7 @@ def _create_bind_dir_if_relative(volume, path_string, compose_dir):
 
 # See: https://stackoverflow.com/questions/45699189/editing-docker-compose-yml-with-pyyaml
 def _fixup_pod_file(pod, spec, compose_dir):
+    deployment_type = spec[constants.deploy_to_key]
     # Fix up volumes
     if "volumes" in spec:
         spec_volumes = spec["volumes"]
@@ -119,27 +121,34 @@ def _fixup_pod_file(pod, spec, compose_dir):
             for volume in pod_volumes.keys():
                 if volume in spec_volumes:
                     volume_spec = spec_volumes[volume]
-                    volume_spec_fixedup = volume_spec if Path(volume_spec).is_absolute() else f".{volume_spec}"
-                    _create_bind_dir_if_relative(volume, volume_spec, compose_dir)
-                    new_volume_spec = {"driver": "local",
-                                       "driver_opts": {
-                                           "type": "none",
-                                           "device": volume_spec_fixedup,
-                                           "o": "bind"
-                                       }
-                                       }
-                    pod["volumes"][volume] = new_volume_spec
+                    if volume_spec:
+                        volume_spec_fixedup = volume_spec if Path(volume_spec).is_absolute() else f".{volume_spec}"
+                        _create_bind_dir_if_relative(volume, volume_spec, compose_dir)
+                        # this is Docker specific
+                        if spec.is_docker_deployment():
+                            new_volume_spec = {
+                                "driver": "local",
+                                "driver_opts": {
+                                    "type": "none",
+                                    "device": volume_spec_fixedup,
+                                    "o": "bind"
+                                }
+                            }
+                            pod["volumes"][volume] = new_volume_spec
 
     # Fix up configmaps
-    if "configmaps" in spec:
-        spec_cfgmaps = spec["configmaps"]
-        if "volumes" in pod:
-            pod_volumes = pod["volumes"]
-            for volume in pod_volumes.keys():
-                if volume in spec_cfgmaps:
-                    volume_cfg = spec_cfgmaps[volume]
-                    # Just make the dir (if necessary)
-                    _create_bind_dir_if_relative(volume, volume_cfg, compose_dir)
+    if constants.configmaps_key in spec:
+        if spec.is_kubernetes_deployment():
+            spec_cfgmaps = spec[constants.configmaps_key]
+            if "volumes" in pod:
+                pod_volumes = pod[constants.volumes_key]
+                for volume in pod_volumes.keys():
+                    if volume in spec_cfgmaps:
+                        volume_cfg = spec_cfgmaps[volume]
+                        # Just make the dir (if necessary)
+                        _create_bind_dir_if_relative(volume, volume_cfg, compose_dir)
+        else:
+            print(f"Warning: ConfigMaps not supported for {deployment_type}")
 
     # Fix up ports
     if "network" in spec and "ports" in spec["network"]:
@@ -323,7 +332,7 @@ def init_operation(deploy_command_context, stack, deployer_type, config,
         if image_registry is None:
             error_exit("--image-registry must be supplied with --deploy-to k8s")
         spec_file_content.update({constants.kube_config_key: kube_config})
-        spec_file_content.update({constants.image_resigtry_key: image_registry})
+        spec_file_content.update({constants.image_registry_key: image_registry})
     else:
         # Check for --kube-config supplied for non-relevant deployer types
         if kube_config is not None:
@@ -358,10 +367,16 @@ def init_operation(deploy_command_context, stack, deployer_type, config,
         volume_descriptors = {}
         configmap_descriptors = {}
         for named_volume in named_volumes["rw"]:
-            volume_descriptors[named_volume] = f"./data/{named_volume}"
+            if "k8s" in deployer_type:
+                volume_descriptors[named_volume] = None
+            else:
+                volume_descriptors[named_volume] = f"./data/{named_volume}"
         for named_volume in named_volumes["ro"]:
-            if "k8s" in deployer_type and "config" in named_volume:
-                configmap_descriptors[named_volume] = f"./data/{named_volume}"
+            if "k8s" in deployer_type:
+                if "config" in named_volume:
+                    configmap_descriptors[named_volume] = f"./configmaps/{named_volume}"
+                else:
+                    volume_descriptors[named_volume] = None
             else:
                 volume_descriptors[named_volume] = f"./data/{named_volume}"
         if volume_descriptors:
@@ -406,6 +421,17 @@ def _create_deployment_file(deployment_dir: Path):
         output_file.write(f"{constants.cluster_id_key}: {cluster}\n")
 
 
+def _check_volume_definitions(spec):
+    if spec.is_kubernetes_deployment():
+        for volume_name, volume_path in spec.get_volumes().items():
+            if volume_path:
+                if not os.path.isabs(volume_path):
+                    raise Exception(
+                        f"Relative path {volume_path} for volume {volume_name} not "
+                        f"supported for deployment type {spec.get_deployment_type()}"
+                    )
+
+
 @click.command()
 @click.option("--spec-file", required=True, help="Spec file to use to create this deployment")
 @click.option("--deployment-dir", help="Create deployment files in this directory")
@@ -421,7 +447,8 @@ def create(ctx, spec_file, deployment_dir, network_dir, initial_peers):
 # The init command's implementation is in a separate function so that we can
 # call it from other commands, bypassing the click decoration stuff
 def create_operation(deployment_command_context, spec_file, deployment_dir, network_dir, initial_peers):
-    parsed_spec = get_parsed_deployment_spec(spec_file)
+    parsed_spec = Spec(os.path.abspath(spec_file), get_parsed_deployment_spec(spec_file))
+    _check_volume_definitions(parsed_spec)
     stack_name = parsed_spec["stack"]
     deployment_type = parsed_spec[constants.deploy_to_key]
     stack_file = get_stack_file_path(stack_name)

stack_orchestrator/deploy/images.py

@@ -46,7 +46,7 @@ def push_images_operation(command_context: DeployCommandContext, deployment_cont
     cluster_context = command_context.cluster_context
     images: Set[str] = images_for_deployment(cluster_context.compose_files)
     # Tag the images for the remote repo
-    remote_repo_url = deployment_context.spec.obj[constants.image_resigtry_key]
+    remote_repo_url = deployment_context.spec.obj[constants.image_registry_key]
     docker = DockerClient()
     for image in images:
         if _image_needs_pushed(image):

stack_orchestrator/deploy/k8s/cluster_info.py

@@ -21,7 +21,7 @@
 from stack_orchestrator.opts import opts
 from stack_orchestrator.util import env_var_map_from_file
 from stack_orchestrator.deploy.k8s.helpers import named_volumes_from_pod_files, volume_mounts_for_service, volumes_for_pod_files
-from stack_orchestrator.deploy.k8s.helpers import get_node_pv_mount_path
+from stack_orchestrator.deploy.k8s.helpers import get_kind_pv_bind_mount_path
 from stack_orchestrator.deploy.k8s.helpers import envs_from_environment_variables_map, envs_from_compose_file, merge_envs
 from stack_orchestrator.deploy.deploy_util import parsed_pod_files_map_from_file_names, images_for_deployment
 from stack_orchestrator.deploy.deploy_types import DeployEnvVars
@@ -171,21 +171,33 @@ def get_pvcs(self):
             print(f"Spec Volumes: {spec_volumes}")
             print(f"Named Volumes: {named_volumes}")
             print(f"Resources: {resources}")
-        for volume_name in spec_volumes:
+        for volume_name, volume_path in spec_volumes.items():
             if volume_name not in named_volumes:
                 if opts.o.debug:
                     print(f"{volume_name} not in pod files")
                 continue
+
+            labels = {
+                "app": self.app_name,
+                "volume-label": f"{self.app_name}-{volume_name}"
+            }
+            if volume_path:
+                storage_class_name = "manual"
+                k8s_volume_name = f"{self.app_name}-{volume_name}"
+            else:
+                # These will be auto-assigned.
+                storage_class_name = None
+                k8s_volume_name = None
+
             spec = client.V1PersistentVolumeClaimSpec(
                 access_modes=["ReadWriteOnce"],
-                storage_class_name="manual",
+                storage_class_name=storage_class_name,
                 resources=to_k8s_resource_requirements(resources),
-                volume_name=f"{self.app_name}-{volume_name}"
+                volume_name=k8s_volume_name
             )
             pvc = client.V1PersistentVolumeClaim(
-                metadata=client.V1ObjectMeta(name=f"{self.app_name}-{volume_name}",
-                                             labels={"volume-label": f"{self.app_name}-{volume_name}"}),
-                spec=spec,
+                metadata=client.V1ObjectMeta(name=f"{self.app_name}-{volume_name}", labels=labels),
+                spec=spec
             )
             result.append(pvc)
         return result
@@ -226,16 +238,32 @@ def get_pvs(self):
         resources = self.spec.get_volume_resources()
         if not resources:
             resources = DEFAULT_VOLUME_RESOURCES
-        for volume_name in spec_volumes:
+        for volume_name, volume_path in spec_volumes.items():
+            # We only need to create a volume if it is fully qualified HostPath.
+            # Otherwise, we create the PVC and expect the node to allocate the volume for us.
+            if not volume_path:
+                if opts.o.debug:
+                    print(f"{volume_name} does not require an explicit PersistentVolume, since it is not a bind-mount.")
+                continue
+
             if volume_name not in named_volumes:
                 if opts.o.debug:
                     print(f"{volume_name} not in pod files")
                 continue
+
+            if not os.path.isabs(volume_path):
+                print(f"WARNING: {volume_name}:{volume_path} is not absolute, cannot bind volume.")
+                continue
+
+            if self.spec.is_kind_deployment():
+                host_path = client.V1HostPathVolumeSource(path=get_kind_pv_bind_mount_path(volume_name))
+            else:
+                host_path = client.V1HostPathVolumeSource(path=volume_path)
             spec = client.V1PersistentVolumeSpec(
                 storage_class_name="manual",
                 access_modes=["ReadWriteOnce"],
                 capacity=to_k8s_resource_requirements(resources).requests,
-                host_path=client.V1HostPathVolumeSource(path=get_node_pv_mount_path(volume_name))
+                host_path=host_path
             )
             pv = client.V1PersistentVolume(
                 metadata=client.V1ObjectMeta(name=f"{self.app_name}-{volume_name}",

stack_orchestrator/deploy/k8s/deploy_k8s.py

@@ -88,6 +88,16 @@ def _create_volume_data(self):
             if opts.o.debug:
                 print(f"Sending this pv: {pv}")
             if not opts.o.dry_run:
+                try:
+                    pv_resp = self.core_api.read_persistent_volume(name=pv.metadata.name)
+                    if pv_resp:
+                        if opts.o.debug:
+                            print("PVs already present:")
+                            print(f"{pv_resp}")
+                        continue
+                except:  # noqa: E722
+                    pass
+
                 pv_resp = self.core_api.create_persistent_volume(body=pv)
                 if opts.o.debug:
                     print("PVs created:")
@@ -100,6 +110,17 @@ def _create_volume_data(self):
                 print(f"Sending this pvc: {pvc}")
 
             if not opts.o.dry_run:
+                try:
+                    pvc_resp = self.core_api.read_namespaced_persistent_volume_claim(
+                        name=pvc.metadata.name, namespace=self.k8s_namespace)
+                    if pvc_resp:
+                        if opts.o.debug:
+                            print("PVCs already present:")
+                            print(f"{pvc_resp}")
+                        continue
+                except:  # noqa: E722
+                    pass
+
                 pvc_resp = self.core_api.create_namespaced_persistent_volume_claim(body=pvc, namespace=self.k8s_namespace)
                 if opts.o.debug:
                     print("PVCs created:")
@@ -181,33 +202,35 @@ def up(self, detach, services):
     def down(self, timeout, volumes):  # noqa: C901
         self.connect_api()
         # Delete the k8s objects
-        # Create the host-path-mounted PVs for this deployment
-        pvs = self.cluster_info.get_pvs()
-        for pv in pvs:
-            if opts.o.debug:
-                print(f"Deleting this pv: {pv}")
-            try:
-                pv_resp = self.core_api.delete_persistent_volume(name=pv.metadata.name)
+
+        if volumes:
+            # Create the host-path-mounted PVs for this deployment
+            pvs = self.cluster_info.get_pvs()
+            for pv in pvs:
                 if opts.o.debug:
-                    print("PV deleted:")
-                    print(f"{pv_resp}")
-            except client.exceptions.ApiException as e:
-                _check_delete_exception(e)
+                    print(f"Deleting this pv: {pv}")
+                try:
+                    pv_resp = self.core_api.delete_persistent_volume(name=pv.metadata.name)
+                    if opts.o.debug:
+                        print("PV deleted:")
+                        print(f"{pv_resp}")
+                except client.exceptions.ApiException as e:
+                    _check_delete_exception(e)
 
-        # Figure out the PVCs for this deployment
-        pvcs = self.cluster_info.get_pvcs()
-        for pvc in pvcs:
-            if opts.o.debug:
-                print(f"Deleting this pvc: {pvc}")
-            try:
-                pvc_resp = self.core_api.delete_namespaced_persistent_volume_claim(
-                    name=pvc.metadata.name, namespace=self.k8s_namespace
-                )
+            # Figure out the PVCs for this deployment
+            pvcs = self.cluster_info.get_pvcs()
+            for pvc in pvcs:
                 if opts.o.debug:
-                    print("PVCs deleted:")
-                    print(f"{pvc_resp}")
-            except client.exceptions.ApiException as e:
-                _check_delete_exception(e)
+                    print(f"Deleting this pvc: {pvc}")
+                try:
+                    pvc_resp = self.core_api.delete_namespaced_persistent_volume_claim(
+                        name=pvc.metadata.name, namespace=self.k8s_namespace
+                    )
+                    if opts.o.debug:
+                        print("PVCs deleted:")
+                        print(f"{pvc_resp}")
+                except client.exceptions.ApiException as e:
+                    _check_delete_exception(e)
 
         # Figure out the ConfigMaps for this deployment
         cfg_maps = self.cluster_info.get_configmaps()