Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add system volumes to vitess pods #3959

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add system volumes to vitess pods #3959

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2141f9c
Add system volumes to vitess pods
VinaySagarGonabavi Sep 13, 2024
880e5a4
Set the init_shard to 0 for consistency with existing setup
VinaySagarGonabavi Sep 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
209 changes: 137 additions & 72 deletions paasta_tools/vitesscluster_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import service_configuration_lib
from kubernetes.client import ApiClient
from kubernetes.client import V1VolumeMount

from paasta_tools.kubernetes_tools import KubernetesDeploymentConfig
from paasta_tools.kubernetes_tools import KubernetesDeploymentConfigDict
Expand Down Expand Up @@ -129,13 +130,58 @@ class ResourceConfigDict(TypedDict, total=False):
limits: Dict[str, RequestsDict]


class HostPathVolume(TypedDict, total=False):
path: str
type: str


class KubernetesVolume(TypedDict, total=False):
name: str
hostPath: HostPathVolume
Comment on lines +134 to +141
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm a little surprised that we don't have typed dicts for this type of k8s volume shenanigans - but i guess that's maybe because we never store the volume info as a dict outside of representing the config as seen in soaconfigs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, looks like DockerVolume format is used in the parent classes, and I used this as required for Vitess manifest



# Extra Volumes and Volume Mounts
VTTABLET_EXTRA_VOLUMES: List[KubernetesVolume] = [
KubernetesVolume(
name="vault-secrets",
hostPath=HostPathVolume(path="/nail/etc/vault/all_cas"),
),
KubernetesVolume(
name="vttablet-fake-credentials",
hostPath=HostPathVolume(path="/dev/null"),
),
KubernetesVolume(
name="keyspace-fake-init-script",
hostPath=HostPathVolume(path="/dev/null"),
),
]

VTTABLET_EXTRA_VOLUME_MOUNTS: List[V1VolumeMount] = [
V1VolumeMount(
mount_path="/etc/vault/all_cas",
name="vault-secrets",
read_only=True,
),
V1VolumeMount(
mount_path="/etc/credentials.yaml",
name="vttablet-fake-credentials",
read_only=True,
),
V1VolumeMount(
mount_path="/etc/init_db.sql",
name="keyspace-fake-init-script",
read_only=True,
),
]


class GatewayConfigDict(TypedDict, total=False):
affinity: Dict[str, Any]
extraEnv: List[Union[KVEnvVar, KVEnvVarValueFrom]]
extraFlags: Dict[str, str]
extraLabels: Dict[str, str]
extraVolumeMounts: List[Dict[str, Any]]
extraVolumes: List[Dict[str, Any]]
extraVolumeMounts: List[V1VolumeMount]
extraVolumes: List[KubernetesVolume]
lifecycle: Dict[str, Dict[str, Dict[str, List[str]]]]
replicas: int
resources: Dict[str, Any]
Expand All @@ -156,6 +202,8 @@ class VitessDashboardConfigDict(TypedDict, total=False):
replicas: int
resources: Dict[str, Any]
annotations: Mapping[str, Any]
extraVolumeMounts: List[V1VolumeMount]
extraVolumes: List[KubernetesVolume]


class VtAdminConfigDict(TypedDict, total=False):
Expand All @@ -170,6 +218,8 @@ class VtAdminConfigDict(TypedDict, total=False):
apiResources: Dict[str, Any]
webResources: Dict[str, Any]
annotations: Mapping[str, Any]
extraVolumeMounts: List[V1VolumeMount]
extraVolumes: List[KubernetesVolume]


class VtTabletDict(TypedDict, total=False):
Expand All @@ -184,8 +234,8 @@ class TabletPoolDict(TypedDict, total=False):
affinity: Dict[str, Any]
extraLabels: Dict[str, str]
extraEnv: List[Union[KVEnvVar, KVEnvVarValueFrom]]
extraVolumeMounts: List[Dict[str, Any]]
extraVolumes: List[Dict[str, Any]]
extraVolumeMounts: List[V1VolumeMount]
extraVolumes: List[KubernetesVolume]
replicas: int
vttablet: VtTabletDict
externalDatastore: Dict[str, Any]
Expand Down Expand Up @@ -239,7 +289,8 @@ def get_cell_config(
labels: Dict[str, str],
node_affinity: dict,
annotations: Mapping[str, Any],
aws_region: str,
extra_volumes: List[KubernetesVolume],
extra_volume_mounts: List[V1VolumeMount],
) -> CellConfigDict:
"""
get vtgate config
Expand All @@ -266,7 +317,7 @@ def get_cell_config(
"command": [
"/bin/sh",
"-c",
f"/cloudmap/scripts/deregister_from_cloudmap.sh vtgate-{cell} {aws_region}",
f"/cloudmap/scripts/deregister_from_cloudmap.sh vtgate-{cell}",
]
}
},
Expand All @@ -275,7 +326,7 @@ def get_cell_config(
"command": [
"/bin/sh",
"-c",
f"/cloudmap/scripts/register_to_cloudmap.sh vtgate-{cell} {aws_region}",
f"/cloudmap/scripts/register_to_cloudmap.sh vtgate-{cell}",
]
}
},
Expand All @@ -289,28 +340,8 @@ def get_cell_config(
"mysql_auth_vault_tls_ca": f"/etc/vault/all_cas/acm-privateca-{region}.crt",
"mysql_auth_vault_ttl": "60s",
},
extraVolumeMounts=[
{
"mountPath": "/nail/srv",
"name": "srv-configs",
"readOnly": True,
},
{
"mountPath": "/nail/etc/srv-configs",
"name": "etc-srv-configs",
"readOnly": True,
},
],
extraVolumes=[
{
"name": "srv-configs",
"hostPath": {"path": "/nail/srv"},
},
{
"name": "etc-srv-configs",
"hostPath": {"path": "/nail/etc/srv-configs"},
},
],
extraVolumeMounts=extra_volume_mounts,
extraVolumes=extra_volumes,
extraLabels=labels,
replicas=replicas,
resources={
Expand All @@ -331,6 +362,8 @@ def get_vitess_dashboard_config(
labels: Dict[str, str],
node_affinity: dict,
annotations: Mapping[str, Any],
extra_volumes: List[KubernetesVolume],
extra_volume_mounts: List[V1VolumeMount],
) -> VitessDashboardConfigDict:
"""
get vtctld config
Expand All @@ -353,6 +386,8 @@ def get_vitess_dashboard_config(
extraEnv=updated_vtctld_extra_env,
extraFlags=VTCTLD_EXTRA_FLAGS,
extraLabels=labels,
extraVolumeMounts=extra_volume_mounts,
extraVolumes=extra_volumes,
replicas=replicas,
resources={
"requests": requests,
Expand All @@ -370,6 +405,8 @@ def get_vt_admin_config(
labels: Dict[str, str],
node_affinity: dict,
annotations: Mapping[str, Any],
extra_volumes: List[KubernetesVolume],
extra_volume_mounts: List[V1VolumeMount],
) -> VtAdminConfigDict:
"""
get vtadmin config
Expand All @@ -385,6 +422,8 @@ def get_vt_admin_config(
extraLabels=labels,
extraFlags=VTADMIN_EXTRA_FLAGS,
extraEnv=env,
extraVolumeMounts=extra_volume_mounts,
extraVolumes=extra_volumes,
replicas=replicas,
readOnly=False,
apiResources={
Expand Down Expand Up @@ -415,6 +454,8 @@ def get_tablet_pool_config(
labels: Dict[str, str],
node_affinity: dict,
annotations: Mapping[str, Any],
extra_volumes: List[KubernetesVolume],
extra_volume_mounts: List[V1VolumeMount],
) -> TabletPoolDict:
"""
get vttablet config
Expand Down Expand Up @@ -474,46 +515,8 @@ def get_tablet_pool_config(
affinity={"nodeAffinity": node_affinity},
extraLabels=labels,
extraEnv=updated_vttablet_extra_env,
extraVolumeMounts=[
{
"mountPath": "/etc/vault/all_cas",
"name": "vault-secrets",
"readOnly": True,
},
{
"mountPath": "/nail/srv",
"name": "srv-configs",
"readOnly": True,
},
{
"mountPath": "/nail/etc/srv-configs",
"name": "etc-srv-configs",
"readOnly": True,
},
{
"mountPath": "etc/credentials.yaml",
"name": "vttablet-fake-credentials",
"readOnly": True,
},
{
"mountPath": "/etc/init_db.sql",
"name": "keyspace-fake-init-script",
"readOnly": True,
},
],
extraVolumes=[
{"name": "vault-secrets", "hostPath": {"path": "/nail/etc/vault/all_cas"}},
{
"name": "srv-configs",
"hostPath": {"path": "/nail/srv"},
},
{
"name": "etc-srv-configs",
"hostPath": {"path": "/nail/etc/srv-configs"},
},
{"name": "vttablet-fake-credentials", "hostPath": {"path": "/dev/null"}},
{"name": "keyspace-fake-init-script", "hostPath": {"path": "/dev/null"}},
],
extraVolumeMounts=extra_volume_mounts,
extraVolumes=extra_volumes,
replicas=replicas,
vttablet={
"extraFlags": vttablet_extra_flags,
Expand Down Expand Up @@ -551,6 +554,8 @@ def get_keyspaces_config(
labels: Dict[str, str],
node_affinity: dict,
annotations: Mapping[str, Any],
extra_volumes: List[KubernetesVolume],
extra_volume_mounts: List[V1VolumeMount],
) -> List[KeyspaceConfigDict]:
"""
get vitess keyspace config
Expand Down Expand Up @@ -613,6 +618,8 @@ def get_keyspaces_config(
labels,
node_affinity,
annotations,
extra_volumes,
extra_volume_mounts,
)
for cell in cells
]
Expand Down Expand Up @@ -775,16 +782,50 @@ def get_global_lock_server(self) -> Dict[str, Dict[str, str]]:
}
}

def get_kubernetes_volumes(self) -> List[KubernetesVolume]:
system_volumes = load_system_paasta_config().get_volumes()
docker_volumes = self.get_volumes(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should hardcode passing in uses_bulkdata_default=False to all the self.get_volumes() calls so that we skip adding bulkdata mounts (that y'all don't need)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking again at it, the get_volumes method of InstanceConfig has the uses_bulkdata_default argument but not SystemPaastaConfig one

system_volumes=system_volumes,
)
kubernetes_volumes = [
KubernetesVolume(
name=self.get_docker_volume_name(docker_volume),
hostPath=HostPathVolume(
path=docker_volume["hostPath"],
),
)
for docker_volume in docker_volumes
]
return kubernetes_volumes

def get_kubernetes_volume_mounts(
self,
kubernetes_volumes: List[KubernetesVolume],
) -> List[V1VolumeMount]:
api_client = ApiClient()
volume_mounts = [
api_client.sanitize_for_serialization(
V1VolumeMount(
mount_path=kubernetes_volume["hostPath"]["path"],
name=kubernetes_volume["name"],
read_only=True,
)
)
for kubernetes_volume in kubernetes_volumes
]
return volume_mounts

def get_cells(self) -> List[CellConfigDict]:
cells = self.config_dict.get("cells")
region = self.get_region()
aws_region = self.get_aws_region()
vtgate_resources = self.config_dict.get("vtgate_resources")

formatted_env = self.get_env_variables()
labels = self.get_labels()
node_affinity = self.get_vitess_node_affinity()
annotations = self.get_annotations()
extra_volumes = self.get_kubernetes_volumes()
extra_volume_mounts = self.get_kubernetes_volume_mounts(extra_volumes)

return [
get_cell_config(
Expand All @@ -795,7 +836,8 @@ def get_cells(self) -> List[CellConfigDict]:
labels,
node_affinity,
annotations,
aws_region,
extra_volumes,
extra_volume_mounts,
)
for cell in cells
]
Expand All @@ -809,6 +851,8 @@ def get_vitess_dashboard(self) -> VitessDashboardConfigDict:
labels = self.get_labels()
node_affinity = self.get_vitess_node_affinity()
annotations = self.get_annotations()
extra_volumes = self.get_kubernetes_volumes()
extra_volume_mounts = self.get_kubernetes_volume_mounts(extra_volumes)

return get_vitess_dashboard_config(
cells,
Expand All @@ -818,6 +862,8 @@ def get_vitess_dashboard(self) -> VitessDashboardConfigDict:
labels,
node_affinity,
annotations,
extra_volumes,
extra_volume_mounts,
)

def get_vtadmin(self) -> VtAdminConfigDict:
Expand All @@ -828,9 +874,18 @@ def get_vtadmin(self) -> VtAdminConfigDict:
labels = self.get_labels()
node_affinity = self.get_vitess_node_affinity()
annotations = self.get_annotations()
extra_volumes = self.get_kubernetes_volumes()
extra_volume_mounts = self.get_kubernetes_volume_mounts(extra_volumes)

return get_vt_admin_config(
cells, vtadmin_resources, formatted_env, labels, node_affinity, annotations
cells,
vtadmin_resources,
formatted_env,
labels,
node_affinity,
annotations,
extra_volumes,
extra_volume_mounts,
)

def get_keyspaces(self) -> List[KeyspaceConfigDict]:
Expand All @@ -843,6 +898,14 @@ def get_keyspaces(self) -> List[KeyspaceConfigDict]:
labels = self.get_labels()
node_affinity = self.get_vitess_node_affinity()
annotations = self.get_annotations()
extra_volumes = self.get_kubernetes_volumes()
extra_volume_mounts = self.get_kubernetes_volume_mounts(extra_volumes)
api_client = ApiClient()
extra_volumes.extend(VTTABLET_EXTRA_VOLUMES)
for volume_mount in VTTABLET_EXTRA_VOLUME_MOUNTS:
extra_volume_mounts.append(
api_client.sanitize_for_serialization(volume_mount)
)
Comment on lines +906 to +909
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we store this in the final serialized format instead of having to convert before using them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the final format in which we expect it to be part of the spec, for reference in tests

{
                                            "mountPath": "/etc/vault/all_cas",
                                            "name": "vault-secrets",
                                            "readOnly": True,
                                        },

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, do you mean to store the global variable for VTTABLET_EXTRA_VOLUME_MOUNTS in a serialized format instead of List[V1VolumeMount]? I did that to avoid mypy errors when performing extra_volume_mounts.append operation


return get_keyspaces_config(
cells,
Expand All @@ -853,6 +916,8 @@ def get_keyspaces(self) -> List[KeyspaceConfigDict]:
labels,
node_affinity,
annotations,
extra_volumes,
extra_volume_mounts,
)

def get_update_strategy(self) -> Dict[str, str]:
Expand Down
Loading
Loading