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.

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

Mount srv-configs onto vtgate pods #3950

Merged

Conversation

VinaySagarGonabavi
Copy link
Contributor

@VinaySagarGonabavi VinaySagarGonabavi commented Sep 9, 2024

The cloudmap scripts expect srv-configs volume to be mounted on vtgate pods

Copy link
Member

@nemacysts nemacysts left a comment

Choose a reason for hiding this comment

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

i think we'll also need to setup the service account creation bits + make these pods use the paasta-created pod identity service account for pod identity to work (and afaik, we still need the fs_group config) - i'm not sure that adding the iam annotation on your CRs will work (unless that's what flink is doing for pod identity)

@VinaySagarGonabavi
Copy link
Contributor Author

i think we'll also need to setup the service account creation bits + make these pods use the paasta-created pod identity service account for pod identity to work (and afaik, we still need the fs_group config) - i'm not sure that adding the iam annotation on your CRs will work (unless that's what flink is doing for pod identity)

Added the service account creation bit, please take another look. Any ideas on how to add the service context? There is no PodSpec/PodTemplateSpec to be provided in the CRD https://github.yelpcorp.com/misc/compute-infra-k8s/blob/master/clusters/eks-infrastage/90-vitess-operator/eks/40-crd-planetscale.com-vitessclusters.yaml

@VinaySagarGonabavi VinaySagarGonabavi force-pushed the u/gonabavi/DREIMP-10984_add_iam_role_annotation branch from 8a1b340 to bb18c44 Compare September 10, 2024 03:04
@VinaySagarGonabavi VinaySagarGonabavi force-pushed the u/gonabavi/DREIMP-10984_add_iam_role_annotation branch from bb18c44 to d835427 Compare September 12, 2024 04:08
@VinaySagarGonabavi VinaySagarGonabavi changed the title Add iam role annotation to vitess pods Mount srv-configs onto vtgate pods Sep 12, 2024
@VinaySagarGonabavi VinaySagarGonabavi merged commit 9f85147 into master Sep 12, 2024
10 checks passed
paasta_tools/vitesscluster_tools.py Outdated Show resolved Hide resolved
paasta_tools/vitesscluster_tools.py Outdated Show resolved Hide resolved
@@ -287,6 +289,28 @@ def get_cell_config(
"mysql_auth_vault_tls_ca": f"/etc/vault/all_cas/acm-privateca-{region}.crt",
"mysql_auth_vault_ttl": "60s",
},
extraVolumeMounts=[
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 mount the default volumes defined in SystemPaastaConfig? or are we sure we only need/want srv-configs? (i.e., this isn't mounting the /nail/etc/{habitat,runtimeenv,superregion,etc} files)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nemacysts This is following the same pattern suggested earlier for vttablet volume mounts https://github.com/Yelp/paasta/blob/master/paasta_tools/vitesscluster_tools.py#L477-L516 . If there is an alternate/recommended way to do this?

For vtgates, we need the /nail/srv/configs/dre_service_discovery.yaml file as a part of lifecycle hook scripts

i.e., this isn't mounting the /nail/etc/{habitat,runtimeenv,superregion,etc} files
Wouldn't the volumes mounted from the kube machine host paths be already having habit/runtimeenv/superregion specific srv-configs?

Copy link
Member

@nemacysts nemacysts Sep 13, 2024

Choose a reason for hiding this comment

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

@VinaySagarGonabavi system_paasta_config.get_volumes()will read /etc/paasta/volumes.json and return some default mounts that are usually needed/helpful to have - that said, if you only need srv-configs this is fine

Wouldn't the volumes mounted from the kube machine host paths be already having habit/runtimeenv/superregion specific srv-configs?

yes, this is unrelated to srv-configs - having these files inside your pods would allow for things like the register/deregister scripts to discover where a pod is running rather than having to configure those lifecycle hooks region parameters inside paasta :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll update the script to find the region based on where it's running. The reliance on srv-configs is to find the cloudmap service id stored in srv-configs https://github.yelpcorp.com/sysgit/srv-configs/blob/master/ecosystem/devc/dre_service_discovery.yaml#L23

Copy link
Contributor Author

Choose a reason for hiding this comment

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

++. Let me also add system_paasta_config.get_volumes() so we can find /nail/etc/region etc.,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants