Skip to content

[VTAdmin] Validate, ValidateShard, ValidateVersionShard, GetFullStatus#11438

Merged
ajm188 merged 20 commits intovitessio:mainfrom
planetscale:frances/vtadmin-remaining
Oct 6, 2022
Merged

[VTAdmin] Validate, ValidateShard, ValidateVersionShard, GetFullStatus#11438
ajm188 merged 20 commits intovitessio:mainfrom
planetscale:frances/vtadmin-remaining

Conversation

@notfelineit
Copy link
Contributor

@notfelineit notfelineit commented Oct 4, 2022 •
edited
Loading

Description

This PR completes "VTAdmin Parity" by implementing:

  • Validate
  • ValidateShard
  • ValidateVersions
  • GetFullStatus

Validate
Validate is added to the clusters list, since validation occurs at the cluster level:
validate

ValidateShard
validateShard

GetFullStatus
GetFullStatus replaces Replication Status since it also includes that in its payload:
Screen Shot 2022-10-04 at 1 47 28 PM

ValidateVersionShard
validateVersionShard

Related Issue(s)

Checklist

  • "Backport me!" label has been added if this change should be backported Should be backported to 15
  • Tests were added or are not required Not necessary ATM
  • Documentation was added or is not required Not required

Deployment Notes

@notfelineit
Add validate
9692bc1 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
Add validate to cluster page
7f6f5b6 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
Run lint prettier fix
54d9ebe 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
Add UI for validate shard
5e69f01 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
Add validateShard
d3ef00b 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
Run make web proto
9c2eaba 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
Refactor validation results and implement validateShard
d7ee7dd 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
add http shards
b8700d5 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
Add GetFullStatus
8c946e5 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@vitess-bot
Copy link
Contributor

vitess-bot bot commented Oct 4, 2022 •
edited by notfelineit
Loading

Review Checklist

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

General

  • Ensure that the Pull Request has a descriptive title.
  • If this is a change that users need to know about, please apply the release notes (needs details) label so that merging is blocked unless the summary release notes document is included.

If a new flag is being introduced:

  • Is it really necessary to add this flag?
  • Flag names should be clear and intuitive (as far as possible)
  • Help text should be descriptive.
  • Flag names should use dashes (-) as word separators rather than underscores (_).

If a workflow is added or modified:

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

Bug fixes

  • There should be at least one unit or end-to-end test.
  • The Pull Request description should include a link to an issue that describes the bug.

Non-trivial changes

  • There should be some code comments as to why things are implemented the way they are.

New/Existing features

  • Should be documented, either by modifying the existing documentation or creating new documentation.
  • New features should have a link to a feature request issue or an RFC that documents the use cases, corner cases and test cases.

Backward compatibility

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

@notfelineit notfelineit mentioned this pull request Oct 4, 2022
Closed
7 tasks
@deepthi deepthi added Type: Enhancement Logical improvement (somewhere between a bug and feature) Component: VTAdmin VTadmin interface Backport to: release-15.0 labels Oct 4, 2022
@notfelineit notfelineit marked this pull request as ready for review October 4, 2022 22:43
@notfelineit notfelineit changed the title Frances/vtadmin remaining [VTAdmin] Validate, ValidateShard, GetFullStatus Oct 4, 2022
@deepthi
Copy link
Collaborator

deepthi commented Oct 4, 2022

LGTM visually. Let's get a code review as well.

@notfelineit
Add keys and comment
1aadf51 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
Run lint prettier fix
ff20310 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
Add ValidateVersionShard to vtctldclient
cb48ac4 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
Add ValidateVersionShard
25ea49b 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
Update test for vtctldclient
8ec151a 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
Add test for ValidateVersionShard
60e9371 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
run lint prettier fix again
3e8d429 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit notfelineit changed the title [VTAdmin] Validate, ValidateShard, GetFullStatus [VTAdmin] Validate, ValidateShard, ValidateVersionShard, GetFullStatus Oct 5, 2022
no-itsbackpack
no-itsbackpack reviewed Oct 5, 2022
View reviewed changes
no-itsbackpack
no-itsbackpack approved these changes Oct 5, 2022
View reviewed changes
Copy link

@no-itsbackpack no-itsbackpack left a comment •
edited by deepthi
Loading

Choose a reason for hiding this comment

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

✅ on UI changes 🚀 🚀 🚀

@notfelineit
Modify during 1:1 with gomez
5f4111c 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
Run lint prettier fix
0f2efd6 
Signed-off-by: notfelineit <notfelineit@gmail.com>
ajm188
ajm188 reviewed Oct 6, 2022
View reviewed changes
Copy link
Contributor

@ajm188 ajm188 left a comment

Choose a reason for hiding this comment

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

couple nits but also some required minor changes on the golang side, but looking great!

go/vt/vtadmin/api.go Outdated
return nil, err
}

if !api.authz.IsAuthorized(ctx, c.ID, rbac.TabletResource, rbac.GetAction) {
Copy link
Contributor

@ajm188 ajm188 Oct 6, 2022

Choose a reason for hiding this comment

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

we might want to revisit this and create a new resource; it may make sense to allow some users to "get" a tablet (i.e. see what's in the topo) without being able to view the full status

Copy link
Contributor Author

@notfelineit notfelineit Oct 6, 2022

Choose a reason for hiding this comment

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

Added a TabletFullStatusResource. It can be changed if another resource seems better.

@notfelineit
Address PR review comments
b234fa9 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@notfelineit
Update indentation
5d70186 
Signed-off-by: notfelineit <notfelineit@gmail.com>
@ajm188 ajm188 merged commit 85b3b66 into vitessio:main Oct 6, 2022
@ajm188 ajm188 deleted the frances/vtadmin-remaining branch October 6, 2022 20:07
@vitess-bot vitess-bot bot mentioned this pull request Oct 6, 2022
Merged
@GuptaManan100 GuptaManan100 mentioned this pull request Oct 12, 2022
Merged
3 tasks
frouioui pushed a commit that referenced this pull request Oct 12, 2022
@vitess-bot @notfelineit
[release-15.0] [VTAdmin] Validate, ValidateShard, `ValidateVersio…
bd8d5c8 
…nShard`, `GetFullStatus` (#11438) (#11452)

* Add validate

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Add validate to cluster page

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Run lint prettier fix

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Add UI for validate shard

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Add validateShard

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Run make web proto

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Refactor validation results and implement validateShard

Signed-off-by: notfelineit <notfelineit@gmail.com>

* add http shards

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Add GetFullStatus

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Add keys and comment

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Run lint prettier fix

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Add ValidateVersionShard to vtctldclient

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Add ValidateVersionShard

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Update test for vtctldclient

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Add test for ValidateVersionShard

Signed-off-by: notfelineit <notfelineit@gmail.com>

* run lint prettier fix again

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Modify during 1:1 with gomez

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Run lint prettier fix

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Address PR review comments

Signed-off-by: notfelineit <notfelineit@gmail.com>

* Update indentation

Signed-off-by: notfelineit <notfelineit@gmail.com>

Signed-off-by: notfelineit <notfelineit@gmail.com>
Co-authored-by: notfelineit <notfelineit@gmail.com>
@austenLacy austenLacy mentioned this pull request Apr 26, 2023
Merged
@austenLacy austenLacy mentioned this pull request Aug 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@doeg doeg Awaiting requested review from doeg

@rohit-nayak-ps rohit-nayak-ps Awaiting requested review from rohit-nayak-ps rohit-nayak-ps is a code owner

@deepthi deepthi Awaiting requested review from deepthi

@mattlord mattlord Awaiting requested review from mattlord mattlord is a code owner

2 more reviewers

@no-itsbackpack no-itsbackpack no-itsbackpack approved these changes

@ajm188 ajm188 ajm188 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Component: VTAdmin VTadmin interface Type: Enhancement Logical improvement (somewhere between a bug and feature)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@notfelineit @deepthi @no-itsbackpack @ajm188