Add system volumes to vitess pods #3959
Conversation
VinaySagarGonabavi
requested review from
nemacysts,
jfongatyelp,
matt-ullmer and
alexanderjulianmartinez
VinaySagarGonabavi
force-pushed
the
u/gonabavi/DREIMP-10984_add_default_volumes
branch
from
75bf9a4
to
2141f9c
Compare
| class HostPathVolume(TypedDict, total=False): | ||
| path: str | ||
| type: str | ||
|
|
||
|
|
||
| class KubernetesVolume(TypedDict, total=False): | ||
| name: str | ||
| hostPath: HostPathVolume |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Yeah, looks like DockerVolume format is used in the parent classes, and I used this as required for Vitess manifest
| @@ -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( | |||
There was a problem hiding this comment.
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)
| for volume_mount in VTTABLET_EXTRA_VOLUME_MOUNTS: | ||
| extra_volume_mounts.append( | ||
| api_client.sanitize_for_serialization(volume_mount) | ||
| ) |
There was a problem hiding this comment.
should we store this in the final serialized format instead of having to convert before using them?
There was a problem hiding this comment.
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,
},
There was a problem hiding this comment.
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
Additional changes needed locally to test
Modified
k8s_itests/deployments/paasta/fake_etc_paasta/volumes.jsonto the devc versionUpdated
paasta_tools/contrib/create_paasta_playground.pyto not modify the paasta config locally https://fluffy.yelpcorp.com/i/BhR02T8wwShMWjvK38JPptz9B2cX0c3g.html