Correctly parse cell names with dashes in tablet aliases

[hkdsun]

Signed-off-by: Hormoz Kheradmand hormoz.kheradmand@shopify.com

Description

Closes #6041

vttablet fails to parse tablet-aliases with more than one - in the cell name.

This is especially problematic because cells are often named after GCP/AWS zones, which are often valid DNS-1123 labels (consists of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character. see similar validation in the kubernetes/kubernetes repository)

Since this parser has been out in the wild for a while, I am not sure how strict we want to make the format. I chose to include the capital alphabetic characters in addition to ., _, and of course -.

I am happy to change the expected format to be any more or any less strict, as maintainers see fit.

Checklist

• Tests were added or are not required
• Documentation was added or is not required

Deployment Notes

There are chances of invalid cell names out in the wild the more strict we make the expected format.

@deepthi @gedgar @harshit-gangal
cc @jeremycole @acharis

[rafael]

Hello @hkdsun , @jeremycole ! Good to see you in Vitess land 🎉

How cell is inferred in tablets is something that has bugged for a long time. Thank you for fixing this. In my view, this will work and after addressing that outstanding comments we should proceed with the approach proposed here.

Having said this, there is an alternative approach that we could consider that I would like us to consider. All other vitess components are explicit about which cell they are registering to. I can't think of a good reason why that shouldn't be the case here as well. Curious if @deepthi or @sougou remember something that I might be missing. If we can make it explicit, I think we can do something as follows:

1. Add two flags to the tablet (-cell and -uid).
2. If those attributes are not set, default to existent behavior.
3. If they are set, they take precedence and we don't have to rely on processing the tablet_path to infer what portion is the cell and which one is the uid.
4. Eventually deprecate tablet_path.

In the long term, this might be a better approach. Of course, that will increase the scope of what you are actually trying to fix. It will not only mean updating o tablet, but other places where this assumption is also being made (e.g some of the vtctld operation like reparents).

I think we can merge this, but I would be curious of people thoughts on the alternate approach.

[hkdsun]

Thanks for the comments everyone and the excellent context, Rafael.

Using separate -uid and -cell arguments makes a lot of sense to me, though I can only commit to this simple fix at the moment for the stage of the project we are in – happy to reconsider in the future.

I would still love to have this PR be merged if we agree on it as a good short-term fix.

[deepthi]

Thanks for the comments everyone and the excellent context, Rafael.

Using separate -uid and -cell arguments makes a lot of sense to me, though I can only commit to this simple fix at the moment for the stage of the project we are in – happy to reconsider in the future.

I would still love to have this PR be merged if we agree on it as a good short-term fix.

I like the long-term approach proposed by @rafael but understand your position too. We can merge this as-is and revisit later.

[rafael]

I will create an issue to keep track of the long term fix.