Add unmanaged tablet flag at vttablet level

[beingnoble03]

Description

Adds a --unmanaged flag for vttablet, which Indicates an unmanaged tablet, i.e. using an external mysql-compatible database. It will help add both better visibility and user experience while setting up unmanaged tablets.

Related Issue(s)

• Fixes: #12092

Checklist

• [x] "Backport to:" labels have been added if this change should be back-ported to release branches
• [x] If this change is to be back-ported to previous releases, a justification is included in the PR description
• [ ] Tests were added or are not required
• [ ] Did the new or modified tests pass consistently locally and on CI?
• [ ] Documentation was added or is not required

[vitess-bot[bot]]

Review Checklist

Hello reviewers! :wave: Please follow this checklist when reviewing this Pull Request.

General

• [ ] Ensure that the Pull Request has a descriptive title.
• [ ] Ensure there is a link to an issue (except for internal cleanup and flaky test fixes), new features should have an RFC that documents use cases and test cases.

Tests

• [ ] Bug fixes should have at least one unit or end-to-end test, enhancement and new features should have a sufficient number of tests.

Documentation

• [ ] Apply the release notes (needs details) label if users need to know about this change.
• [ ] New features should be documented.
• [ ] There should be some code comments as to why things are implemented the way they are.
• [ ] There should be a comment at the top of each new or modified test to explain what the test does.

New flags

• [ ] Is this flag really necessary?
• [ ] Flag names must be clear and intuitive, use dashes (-), and have a clear help text.

If a workflow is added or modified:

• [ ] Each item in Jobs should be named in order to mark it as required.
• [ ] If the workflow needs to be marked as required, the maintainer team must be notified.

Backward compatibility

• [ ] Protobuf changes should be wire-compatible.
• [ ] Changes to _vt tables and RPCs need to be backward compatible.
• [ ] RPC changes should be compatible with vitess-operator
• [ ] If a flag is removed, then it should also be removed from vitess-operator and arewefastyet, if used there.
• [ ] vtctl command output order should be stable and awk-able.

[beingnoble03]

@GuptaManan100 can you please have a look?

[rohit-nayak-ps]

Do we have end to end tests for unmanaged tablets @rohit-nayak-ps? If we do, we should add this flag there to make sure that the validations are correct.

We don't explicitly setup unmanaged tablets in e2e tests currently. @beingnoble03, how did you test these changes?

[rohit-nayak-ps]

Thanks for this, @beingnoble03 . I wanted to note that this PR doesn't fix https://github.com/vitessio/vitess/issues/12092 entirely. These have not been addressed:

* Add validations to check that incompatible flags are not defined
* Display this in relevant vttablet APIs and expose in /debug/vars

@deepthi, one of the reasons that issue was originally created was, iirc, that incompatible flags were being specified when vttablets were started which did not apply and were causing either errors or unexpected behavior. (I forget the details). If we have (or can enumerate) the list of such flags, we can add it to the validations added in this PR.

Aside: We also need to update/refresh the unmanaged tablet documentation. We don't provide a proper motivation and examples for the use of unmanaged tablets as far as I can find. Also https://vitess.io/docs/18.0/user-guides/configuration-advanced/unmanaged-tablet/ is still referring to the --init_populate_metadata flag.

[beingnoble03]

Do we have end to end tests for unmanaged tablets @rohit-nayak-ps? If we do, we should add this flag there to make sure that the validations are correct.

We don't explicitly setup unmanaged tablets in e2e tests currently. @beingnoble03, how did you test these changes?

@rohit-nayak-ps thanks for the review. I changed examples/common/scripts/vttablet-up.sh script accordingly (adding --unmanaged flag and testing some of the validations I've added) to test the changes and running the 101_initial_cluster.sh script.

[beingnoble03]

Thanks for this, @beingnoble03 . I wanted to note that this PR doesn't fix #12092 entirely. These have not been addressed:

* Add validations to check that incompatible flags are not defined
* Display this in relevant vttablet APIs and expose in /debug/vars

@deepthi, one of the reasons that issue was originally created was, iirc, that incompatible flags were being specified when vttablets were started which did not apply and were causing either errors or unexpected behavior. (I forget the details). If we have (or can enumerate) the list of such flags, we can add it to the validations added in this PR.

Aside: We also need to update/refresh the unmanaged tablet documentation. We don't provide a proper motivation and examples for the use of unmanaged tablets as far as I can find. Also https://vitess.io/docs/18.0/user-guides/configuration-advanced/unmanaged-tablet/ is still referring to the --init_populate_metadata flag.

Yes, This PR needs to address these points too. I added some validations which I thought were necessary, but if we can enumerate a list of incompatible flags, I can proceed further adding checks for these flags.

I think we can update the docs, in a separate follow-up PR.

[beingnoble03]

Thanks for this, @beingnoble03 . I wanted to note that this PR doesn't fix #12092 entirely. These have not been addressed:

* Add validations to check that incompatible flags are not defined
* Display this in relevant vttablet APIs and expose in /debug/vars

@deepthi, one of the reasons that issue was originally created was, iirc, that incompatible flags were being specified when vttablets were started which did not apply and were causing either errors or unexpected behavior. (I forget the details). If we have (or can enumerate) the list of such flags, we can add it to the validations added in this PR.

Aside: We also need to update/refresh the unmanaged tablet documentation. We don't provide a proper motivation and examples for the use of unmanaged tablets as far as I can find. Also https://vitess.io/docs/18.0/user-guides/configuration-advanced/unmanaged-tablet/ is still referring to the --init_populate_metadata flag.

@deepthi can you please look into this? So that we can proceed with adding required validations for the flag. Thanks.

[deepthi]

@deepthi can you please look into this? So that we can proceed with adding required validations for the flag. Thanks.

@GuptaManan100 and I discussed this, he will come back with the list of related flags and what values are expected so that the necessary validations can be added.

@ajm188 if you have opinions/guidelines on a standard way of implementing relationships between flags, please do chime in.

[ajm188]

@ajm188 if you have opinions/guidelines on a standard way of implementing relationships between flags, please do chime in.

Suggest using the different Mark* functions where possible (e.g. MarkFlagsMutuallyExclusive, MarkFlagsOneRequired, and so on), and where those aren't powerful enough, doing the validation in code in a PreRunE function for that command.

[beingnoble03]

@GuptaManan100 and I discussed this, he will come back with the list of related flags and what values are expected so that the necessary validations can be added.

@GuptaManan100 could you please look into this?

[beingnoble03]

@deepthi @GuptaManan100 is there any other validation which we need to include here?

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 81.81818% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 65.64%. Comparing base (f889134) to head (e957a3b).

Files	Patch %	Lines
go/vt/mysqlctl/mysqld.go	50.00%	3 Missing :warning:
go/vt/vttablet/tabletserver/tabletenv/config.go	88.88%	3 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #14871      +/-   ##
==========================================
- Coverage   65.65%   65.64%   -0.01%     
==========================================
  Files        1563     1563              
  Lines      194395   194427      +32     
==========================================
+ Hits       127627   127632       +5     
- Misses      66768    66795      +27     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[GuptaManan100]

Rest looks good to me, we would need to add summary changes for the same for 20.0.0. @deepthi Do you have any other change in mind?

[ajm188]

@GuptaManan100 we should also add some unit tests for verifyUnmanagedTabletConfig

[beingnoble03]

@GuptaManan100 @ajm188 added required tests for verifyUnmanagedTabletConfig

[GuptaManan100]

I'll do the website docs update by EOD.

Update - Here is the docs PR - https://github.com/vitessio/website/pull/1702