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

slack-vitess-r14.0.5-dsdefense: backport vitessio/vitess#10619 pflag migration #13910

Conversation

timvaillancourt
Copy link
Contributor

Description

This PR backports #10619 to make backports >= v15 much easier to do

Related Issue(s)

#10619

Checklist

  • "Backport to:" labels have been added if this change should be back-ported
  • Tests were added or are not required
  • Did the new or modified tests pass consistently locally and on the CI
  • Documentation was added or is not required

Deployment Notes

frouioui and others added 30 commits June 7, 2022 16:24
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
* docs: added to the release notes

Signed-off-by: Andres Taylor <[email protected]>

* Add name to static check workflow

Signed-off-by: Rohit Nayak <[email protected]>

Co-authored-by: Rohit Nayak <[email protected]>
…0453) (vitessio#10466)

* fix: change planner_version to planner-version everywhere

Signed-off-by: Andres Taylor <[email protected]>

* fix: actually change the planner version on vtgate after checking

Signed-off-by: Andres Taylor <[email protected]>

* fix: move CheckPlannerVersionFlag out from vtgate

Signed-off-by: Andres Taylor <[email protected]>

* fix: move the global plannerVersion to be a field on the executor. think global, act local, yo

Signed-off-by: Andres Taylor <[email protected]>

* fix: use the planner in the session first

Signed-off-by: Andres Taylor <[email protected]>

* test: use DEFAULT instead of 0

Signed-off-by: Andres Taylor <[email protected]>

* fix: re-add the planner-version flag to vtcombo

Signed-off-by: Andres Taylor <[email protected]>
* Add PingTablet to mock vtctld

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for PingTablet

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for RefreshState

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for RefreshTabletReplicationSource

Signed-off-by: Andrew Mason <[email protected]>

* Add RunHealthCheck to mock vtctld

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for RunHealthCheck

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for SetReadOnly

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for SetReadWrite

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for StartReplication

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for StopReplication

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for TabletExternallyPromoted

Signed-off-by: Andrew Mason <[email protected]>
* add sharding column name/type deprecation to release notes summary

Signed-off-by: deepthi <[email protected]>

* deprecation: mark sharding-column-name and sharding-column-type for CreateKeyspace command as deprecated

Signed-off-by: deepthi <[email protected]>
* [vtadmin] Add authz tests for remaining non-schema related actions (vitessio#10481)

* Add authz tests for EmergencyFailoverShard

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for PlannedFailoverShard

Signed-off-by: Andrew Mason <[email protected]>

* Add ValidateKeyspace to mock vtctld

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for ValidateKeyspace

Signed-off-by: Andrew Mason <[email protected]>

* Add ValidateSchemaKeyspace to mock vtctld

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for ValidateSchemaKeyspace

Signed-off-by: Andrew Mason <[email protected]>

* Add ValidateVersionKeyspace to mock vtctld

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for ValidateVersionKeyspace

Signed-off-by: Andrew Mason <[email protected]>

* fixup mock data sorting

Signed-off-by: Andrew Mason <[email protected]>

* [vtadmin] Add schema-related authz tests (vitessio#10486)

* Update template to support more tablet fields

Signed-off-by: Andrew Mason <[email protected]>

* Update template, add full srvvschema payload, add schema payload, add authz tests for VTExplain

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for GetSchema

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for GetSchemas

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for FindSchema

Signed-off-by: Andrew Mason <[email protected]>

* Add authz tests for ReloadSchemas

Signed-off-by: Andrew Mason <[email protected]>
…io#10502) (vitessio#10509)

* Update docs for backup commands

Signed-off-by: Andrew Mason <[email protected]>

* Update docs for cell commands

Signed-off-by: Andrew Mason <[email protected]>

* Update docs for keyspace commands

Signed-off-by: Andrew Mason <[email protected]>

* Update docs for legacy-shim command

Signed-off-by: Andrew Mason <[email protected]>

* Update docs, add aliases for reparent commands

Signed-off-by: Andrew Mason <[email protected]>

* Update docs for root command

Signed-off-by: Andrew Mason <[email protected]>

* Update docs for routing rule commands

Signed-off-by: Andrew Mason <[email protected]>

* Update docs for schema commands

Signed-off-by: Andrew Mason <[email protected]>

* Update docs for serving graph commands

Signed-off-by: Andrew Mason <[email protected]>

* Update docs for shard commands

Signed-off-by: Andrew Mason <[email protected]>

* Update docs for tablet commands

Signed-off-by: Andrew Mason <[email protected]>

* Update docs for vschema commands

Signed-off-by: Andrew Mason <[email protected]>

* Update docs for workflow commands

Signed-off-by: Andrew Mason <[email protected]>

* Remove references to deprecated flags

Signed-off-by: Andrew Mason <[email protected]>

* fixup! Update docs for keyspace commands

Signed-off-by: Andrew Mason <[email protected]>

* Remove reparent command aliases

Signed-off-by: Andrew Mason <[email protected]>

* Specify that `--server` is required

Signed-off-by: Andrew Mason <[email protected]>
… and move it to places which will only be called less frequently (vitessio#10516) (vitessio#10518)

Signed-off-by: Manan Gupta <[email protected]>
… and vitessio#10514 (vitessio#10517)

* Fix parsing of CAST() statements (vitessio#10512)

* Fix parsing of CAST() statements

CAST() was treated as an alias for CONVERT() but with slightly different
syntax.

This is also described in the documentation at
https://dev.mysql.com/doc/refman/8.0/en/cast-functions.html,
specifically:

With CAST(expr AS type syntax, the CAST() function takes an expression of
any type and produces a result value of the specified type. This operation
may also be expressed as CONVERT(expr, type), which is equivalent. If expr
is NULL, CAST() returns NULL.

This is wrong sadly. CAST() is not equivalent to CONVERT(), specifically
in the context of a CREATE TABLE. For JSON keys, the ARRAY attribute is
possible on a CAST(), but that is not accepted for a CONVERT().

The difference in parsing also shows in MySQL:

```
mysql> select convert(json_keys(c), char(64) array) from t;
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'array) from t' at line 1
mysql> select cast(json_keys(c) as char(64) array) from t;
ERROR 1235 (42000): This version of MySQL doesn't yet support 'Use of CAST( .. AS .. ARRAY) outside of functional index in CREATE(non-SELECT)/ALTER TABLE or in general expressions'
```

Here the first statement can't be parsed at all. The second is properly
parsed, but ARRAY is not allowed in the context of CAST() in this
situation (and only in a CREATE TABLE).

This means we should really treat these as two separate expressions and
don't store them both in the same structure. The change here creates a
separate CAST structure, removes the ARRAY option from CONVERT and
updates the grammar and all tests accordingly.

Signed-off-by: Dirkjan Bussink <[email protected]>

* Handle new cast expression in evalengine and planbuilder

Signed-off-by: Dirkjan Bussink <[email protected]>

* evalengine: do not duplicate CAST/CONVERT translation

Signed-off-by: Vicent Marti <[email protected]>

Co-authored-by: Vicent Marti <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>

* Add back unary single column expression check (vitessio#10514)

This was accidentally removed in
vitessio#10512 but it shouldn't have
been.

Signed-off-by: Dirkjan Bussink <[email protected]>

Co-authored-by: Dirkjan Bussink <[email protected]>
Co-authored-by: Vicent Marti <[email protected]>
…#10508) (vitessio#10528)

* addition of the release_notes_label workflow

Signed-off-by: Florent Poinsard <[email protected]>

* addition of a helper step in the release_notes_label to guide users

Signed-off-by: Florent Poinsard <[email protected]>

* addition of a set of rules for release_notes_labels's pull_request target

Signed-off-by: Florent Poinsard <[email protected]>

* remove the previous workflow that used to check the release notes labels

Signed-off-by: Florent Poinsard <[email protected]>

* Change the review check list and enforce the use of type/component labels

Signed-off-by: Florent Poinsard <[email protected]>

* Fixed syntax in check release notes label workflow

Signed-off-by: Florent Poinsard <[email protected]>
…#10532)

* generate the release changelog as a second document

Signed-off-by: Florent Poinsard <[email protected]>

* include every PR in the release notes

Signed-off-by: Florent Poinsard <[email protected]>

* Moved changelog metrics away from the changelog and added summary for v15

Signed-off-by: Florent Poinsard <[email protected]>
…0553)

* test: added failing e2e test

Signed-off-by: Harshit Gangal <[email protected]>

* fix: handle empty row for scalar aggregation and also olap engine for scalar aggregation

Signed-off-by: Harshit Gangal <[email protected]>

* test: update plan test output

Signed-off-by: Harshit Gangal <[email protected]>

* test: e2e test for empty rows in non-scalar aggregates

Signed-off-by: Harshit Gangal <[email protected]>

Co-authored-by: Florent Poinsard <[email protected]>

Co-authored-by: Florent Poinsard <[email protected]>
* docs: add blurb about gen4 being the new planner

Signed-off-by: Andres Taylor <[email protected]>

* docs: release notes

Signed-off-by: Andres Taylor <[email protected]>
…itessio#10464) (vitessio#10546)

* Adds RPCs to vttablet that vtorc requires (vitessio#10464)

* feat: add vttablet rpc to reset replication parameters

Signed-off-by: Manan Gupta <[email protected]>

* feat: added end to end testing for the rpc and fixed bug

Signed-off-by: Manan Gupta <[email protected]>

* feat: fix typing error

Signed-off-by: Manan Gupta <[email protected]>

* feat: add basic full status rpc functionality and add test for it

Signed-off-by: Manan Gupta <[email protected]>

* feat: add all the fields needed in full status

Signed-off-by: Manan Gupta <[email protected]>

* test: moved the test to reparent tests and improved it

Signed-off-by: Manan Gupta <[email protected]>

* feat: bug fix for no replication status and no primary status

Signed-off-by: Manan Gupta <[email protected]>

* feat: add version to the full status output

Signed-off-by: Manan Gupta <[email protected]>

* feat: add binlog information to full status

Signed-off-by: Manan Gupta <[email protected]>

* docs: fix the comment explaining the binlog information

Signed-off-by: Manan Gupta <[email protected]>

* feat: add semi-sync statuses to full status

Signed-off-by: Manan Gupta <[email protected]>

* feat: call the correct command

Signed-off-by: Manan Gupta <[email protected]>

* feat: add server uuid and id to full status

Signed-off-by: Manan Gupta <[email protected]>

* feat: make server_id a uint32 to accept the correct range of values

Signed-off-by: Manan Gupta <[email protected]>

* feat: add few more fields to the full status like version comment, semi-sync settings, binlog_row_image

Signed-off-by: Manan Gupta <[email protected]>

* feat: generate vtadmin proto files

Signed-off-by: Manan Gupta <[email protected]>

* test: add assertion to check binlog row format is read correctly

Signed-off-by: Manan Gupta <[email protected]>

* feat: split GTID mode in its own function because mariadb doesn't support it

Signed-off-by: Manan Gupta <[email protected]>

* feat: fix parsing of empty mariadb gtid set

Signed-off-by: Manan Gupta <[email protected]>

* docs: add doucmentation for existing fields in ReplicationStatus

Signed-off-by: Manan Gupta <[email protected]>

* feat: add relay log file position to the replication status output

Signed-off-by: Manan Gupta <[email protected]>

* test: augmented full status test to check all the different positions stored

Signed-off-by: Manan Gupta <[email protected]>

* feat: add additional fields to replication status and read source user

Signed-off-by: Manan Gupta <[email protected]>

* feat: read sql delay from show replica status output

Signed-off-by: Manan Gupta <[email protected]>

* feat: read ssl allowed from show replica status output

Signed-off-by: Manan Gupta <[email protected]>

* feat: read has replication filters from show replica status output

Signed-off-by: Manan Gupta <[email protected]>

* feat: read auto position and using gtid from show replica status output

Signed-off-by: Manan Gupta <[email protected]>

* feat: add replication lag unknown too to replication status

Signed-off-by: Manan Gupta <[email protected]>

* feat: return nils from replication and primary postiion if it is not present

Signed-off-by: Manan Gupta <[email protected]>

* feat: rename FileRelayLogPosition in replication status output to RelayLogSourceBinLogEquivalentPosition and augment test to make sure rpc changes are backward compatible

Signed-off-by: Manan Gupta <[email protected]>

* feat: update vtadmin proto files

Signed-off-by: Manan Gupta <[email protected]>

* refactor: rename BinLog to binlog in renamed proto field

Signed-off-by: Manan Gupta <[email protected]>

* lint: add a new line to proto file

Signed-off-by: Manan Gupta <[email protected]>
…io#10562)

* VReplication: more unrecoverable error codes

Signed-off-by: Shlomi Noach <[email protected]>

* correct path for test files

Signed-off-by: Shlomi Noach <[email protected]>
* test: reproduce the panic as a unit test

Signed-off-by: Manan Gupta <[email protected]>

* feat: check ev is not nil before using its fields

Signed-off-by: Manan Gupta <[email protected]>

* feat: increase timeout of LockShard and wait replicas in VTOrc default config

Signed-off-by: Manan Gupta <[email protected]>
…#10535) (vitessio#10570)

The canonical form of printing the vitess migration syntax would upcase
the UUID value, but it needs to be passed through unchanged.

This in turn led me to testing comments in a query as well and it turns
out we wrongly change the case there as well.

Signed-off-by: Dirkjan Bussink <[email protected]>

Co-authored-by: Dirkjan Bussink <[email protected]>
…sist and unrecoverable errors (vitessio#10573)

* Fail VReplication workflows on errors that persist and unrecoverable errors (vitessio#10429)

* Fail workflow if same error persists too long. Fail for unrecoverable errors also in non-online ddl workflows

Signed-off-by: Rohit Nayak <[email protected]>

* Update max time default to 15m, was 1m for testing purposes

Signed-off-by: Rohit Nayak <[email protected]>

* Leverage vterrors for Equals; attempt to address my own nits

Signed-off-by: Matt Lord <[email protected]>

* sanity: validate range of vreplication_retry_delay and of vreplication_max_time_to_retry_on_error

Signed-off-by: Shlomi Noach <[email protected]>

* Fix flags test

Signed-off-by: Rohit Nayak <[email protected]>

* Remove leftover log.Flush()

Signed-off-by: Rohit Nayak <[email protected]>

* Revert validations min/max settings on retry delay since it is breaking unit tests that set the value to a very small value

Signed-off-by: Rohit Nayak <[email protected]>

* captilize per request

Signed-off-by: Shlomi Noach <[email protected]>

Co-authored-by: Matt Lord <[email protected]>
Co-authored-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>

* fix TestHelpOutput

Signed-off-by: Shlomi Noach <[email protected]>

* spaces, not tabs

Signed-off-by: Shlomi Noach <[email protected]>

Co-authored-by: Rohit Nayak <[email protected]>
Co-authored-by: Matt Lord <[email protected]>
…sts (vitessio#10522)

* Take into account `github.ref` when doing upgrade-downgrade tests (vitessio#10504)

* Take into account the github.ref variable when doing upgrade-downgrade tests

Signed-off-by: Florent Poinsard <[email protected]>

* Changed upgrade-downgrade workflows' syntax to use 'previous' instead of 'latest'

Signed-off-by: Florent Poinsard <[email protected]>

* Fixed syntax error in backup upgrade downgrade workflow

Signed-off-by: Florent Poinsard <[email protected]>

* Fixed static_checks_etc workflow

Signed-off-by: Florent Poinsard <[email protected]>
…hannel might block the vstream thread if target channel goes away: context was not being checked then. Fix health stream goroutine leak. (vitessio#10576)

Signed-off-by: Rohit Nayak <[email protected]>
* add-vtadmin-docker-image

Signed-off-by: Léopold Jacquot <[email protected]>

* env as function, update tests and code

Signed-off-by: Andrew Mason <[email protected]>
Signed-off-by: Léopold Jacquot <[email protected]>

* feat: fix remaining env usages to be function calls

Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Léopold Jacquot <[email protected]>

* add vtadmin build result in the bootstrap image

Signed-off-by: Léopold Jacquot <[email protected]>

* fix vtadmin web cleaning

Signed-off-by: Léopold Jacquot <[email protected]>

* add vtadmin docker image entrypoint

Signed-off-by: Léopold Jacquot <[email protected]>

* add the possibility to custom vtadmin web port in Docker image

Signed-off-by: Léopold Jacquot <[email protected]>

* fix vtadmin docker port

Signed-off-by: Léopold Jacquot <[email protected]>

* fix vtadmin entrypoint

Signed-off-by: Léopold Jacquot <[email protected]>

* set default vtadmin docker user as vitess

Signed-off-by: Léopold Jacquot <[email protected]>

* do not build vtadmin frontend in the bootstrap image

Signed-off-by: Léopold Jacquot <[email protected]>

* build vtadmin frontend only in vtadmin docker image

Signed-off-by: Léopold Jacquot <[email protected]>

* vtadmin replace sed and custom entrypoint with config.js file

Signed-off-by: Léopold Jacquot <[email protected]>

* remove vitess web files in lite docker image

Signed-off-by: Léopold Jacquot <[email protected]>

* move vtadmin config into a specific directory

Signed-off-by: Léopold Jacquot <[email protected]>

* restore vtadmin web file in mysql57 lite docker image

Signed-off-by: Léopold Jacquot <[email protected]>

Co-authored-by: Andrew Mason <[email protected]>
Co-authored-by: Manan Gupta <[email protected]>

Co-authored-by: Léopold Jacquot <[email protected]>
Co-authored-by: Andrew Mason <[email protected]>
Co-authored-by: Manan Gupta <[email protected]>
* CherryPick: VDiff2: Support Resuming VDiffs (vitessio#10497)

And fix a number of bugs discovered related to incorrect VDiff summary handling and other more minor things.

Signed-off-by: Matt Lord <[email protected]>
timvaillancourt and others added 8 commits July 3, 2023 20:50
* txthrottler: further code cleanup



* Fix bad merge resolution



---------

Signed-off-by: Tim Vaillancourt <[email protected]>
…let type (vitessio#12174) (#95)

* Add flag to select tx throttler tablet type (vitessio#12174)

Signed-off-by: Tim Vaillancourt <[email protected]>

* Remove mistaken git add

Signed-off-by: Tim Vaillancourt <[email protected]>

---------

Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Priya Bibra <[email protected]>
Signed-off-by: 'Priya Bibra' <[email protected]>
This is a backport of upstreamed vitessio#13526

Signed-off-by: Eduardo J. Ortega U <[email protected]>
* BackportAdd dry-run mode to the TxThrottler

This is a backport of upstreamed vitessio#13604

Signed-off-by: Eduardo J. Ortega U <[email protected]>
* Cleanup usage of go.rice in favor of go:embed (vitessio#10956)

* Cleanup usage of go.rice in favor of go:embed

The usage of go.rice predates the availability of go:embed, but we
should switch to using go:embed instead to ship specific assets like
config files that we need.

go.rice is also incompatible with Go 1.19 and while it might see a fix
in the future, it seems better to go with the recommended Go approach
that is available these days.

Signed-off-by: Dirkjan Bussink <[email protected]>

* Move vtctld to also use `go embed` instead of go.rice

Signed-off-by: Dirkjan Bussink <[email protected]>

* Remove last rice-box related comments

Signed-off-by: Dirkjan Bussink <[email protected]>

* Remove config moving

This right now breaks building the actual tests since the tests might
also end up loading the regular code which has a `go embed` and refers
to the package with the config embeds.

This doesn't mean that the config isn't properly included in the
binaries. Also with using `go embed` we have a build time dependency on
the files and we always know the latest is included, so we don't have
the issue of potentially outdated files either.

All in all, it seems simplest to remove this logic and trust that Go
itself works as advertised.

Signed-off-by: Dirkjan Bussink <[email protected]>

* fix vtrootbin

Signed-off-by: 'Stanislav Maksimov' <[email protected]>

* update the build to 1.19.10

Signed-off-by: 'Stanislav Maksimov' <[email protected]>

* update the workflows to 1.19.10

Signed-off-by: 'Stanislav Maksimov' <[email protected]>

* update the docker image to 1.19.10

Signed-off-by: 'Stanislav Maksimov' <[email protected]>

* update the bootstrap version to get docker to go 1.20.5

Signed-off-by: 'Stanislav Maksimov' <[email protected]>

* use 1.18.7 for static checks

Signed-off-by: 'Stanislav Maksimov' <[email protected]>

* lower the build version requirement to allow static checks to pass

Signed-off-by: 'Stanislav Maksimov' <[email protected]>

* experiment with using old and new Go for a single workflow

Signed-off-by: 'Stanislav Maksimov' <[email protected]>

* use old and new Go for a upgrade/downgrade workflows

Signed-off-by: 'Stanislav Maksimov' <[email protected]>

* set -buildvcs=false

Signed-off-by: 'Stanislav Maksimov' <[email protected]>

* address build errors

Signed-off-by: 'Stanislav Maksimov' <[email protected]>

* Revert "fix parameter name (#93)" (#100)

This reverts commit 76159fd.

* apply patch 12178 to v14 (#102)

Signed-off-by: Priya Bibra <[email protected]>
Signed-off-by: 'Priya Bibra' <[email protected]>

* more workflows to use self-hosted runner

* even more workflows to use self-hosted runner

* partially switch upgrade-downgrade to self-hosted runner

* Ejortegau/larger runners (#115)

* Empty commit to trigger CI

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* `slack-vitess-r14.0.5-dsdefense`: use larger runner

Signed-off-by: Tim Vaillancourt <[email protected]>

* Use runner group instead

Signed-off-by: Tim Vaillancourt <[email protected]>

* Rename group

Signed-off-by: Tim Vaillancourt <[email protected]>

* Move more jobs to runner group

Signed-off-by: Tim Vaillancourt <[email protected]>

* use vitess-ubuntu20 runner group

Signed-off-by: Tim Vaillancourt <[email protected]>

* Rever change of runner type for e2e ERS PRS new features heavy test

Signed-off-by: Eduardo J. Ortega U <[email protected]>

---------

Signed-off-by: Eduardo J. Ortega U <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Co-authored-by: Tim Vaillancourt <[email protected]>

* Revert change of runner type for shardedrecovery_stress_verticalsplit_heavy. (#116)

We do this because the test is taking a lot longer & failing in some cases with
the larger runner.

Signed-off-by: Eduardo J. Ortega U <[email protected]>

* `slack-vitess-r14.0.5`: use dedicated larger runner (#113)

* `slack-vitess-r14.0.5`: use dedicated larger runner

Signed-off-by: Tim Vaillancourt <[email protected]>

* fix fileNameFromPosition

Signed-off-by: Tim Vaillancourt <[email protected]>

* empty commit

Signed-off-by: Tim Vaillancourt <[email protected]>

* empty commit

Signed-off-by: Tim Vaillancourt <[email protected]>

---------

Signed-off-by: Tim Vaillancourt <[email protected]>

* `slack-vitess-r14.0.5`: allow conn overrides in consul topo (#111)

* `slack-vitess-r14.0.5`: allow conn overrides in consul topo

Signed-off-by: Tim Vaillancourt <[email protected]>

* fix e2e test

Signed-off-by: Tim Vaillancourt <[email protected]>

---------

Signed-off-by: Tim Vaillancourt <[email protected]>

---------

Signed-off-by: Dirkjan Bussink <[email protected]>
Signed-off-by: 'Stanislav Maksimov' <[email protected]>
Signed-off-by: Priya Bibra <[email protected]>
Signed-off-by: 'Priya Bibra' <[email protected]>
Signed-off-by: Eduardo J. Ortega U <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Co-authored-by: Dirkjan Bussink <[email protected]>
Co-authored-by: Roderick Yao <[email protected]>
Co-authored-by: pbibra <[email protected]>
Co-authored-by: Eduardo J. Ortega U <[email protected]>
Co-authored-by: Tim Vaillancourt <[email protected]>
* Switch to `pflag` for all parsing

This transparently swaps the cli parsing library used by `internal/flag`
from the standard library `flag` package to `spf13/pflag`.

It also introduces hook points for packages throughout the vitess codebase
to register their flags for either all commands using `servenv` or a
particular subset of commands. This allows these packages to continue to
define their flag variables in a package-private way, but without
polluting the global flagset.

Signed-off-by: Andrew Mason <[email protected]>

* Workaround exit code difference between stdlib `flag` and `pflag`

tl;dr stdlib `flag` has [this][1] and `pflag` does not

[1]: golang/go@dcf0929

Signed-off-by: Andrew Mason <[email protected]>

* adjust test data for difference in spacing between pflag/stdflag

Signed-off-by: Andrew Mason <[email protected]>

* update lingering legacy flag tests

Signed-off-by: Andrew Mason <[email protected]>

* Update vtgate/tabletgateway.go to use new interface to isolate flags, update test data

Signed-off-by: Andrew Mason <[email protected]>

* update flags in java test code

Signed-off-by: Andrew Mason <[email protected]>
@vitess-bot
Copy link
Contributor

vitess-bot bot commented Sep 1, 2023

Review Checklist

Hello reviewers! 👋 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.

@vitess-bot vitess-bot bot added NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work NeedsIssue A linked issue is missing for this Pull Request NeedsWebsiteDocsUpdate What it says labels Sep 1, 2023
@github-actions github-actions bot added this to the v18.0.0 milestone Sep 1, 2023
@timvaillancourt
Copy link
Contributor Author

Accidental PR 🤦

@timvaillancourt timvaillancourt deleted the bp-10619-slack-vitess-r14.0.5-dsdefense branch September 1, 2023 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work NeedsIssue A linked issue is missing for this Pull Request NeedsWebsiteDocsUpdate What it says
Projects
None yet
Development

Successfully merging this pull request may close these issues.