Skip to content

Fix fullstatus test for backward compat#12685

Merged
mattlord merged 1 commit intovitessio:mainfrom
planetscale:fix-fullstatus-api
Mar 24, 2023
Merged

Fix fullstatus test for backward compat#12685
mattlord merged 1 commit intovitessio:mainfrom
planetscale:fix-fullstatus-api

Conversation

@rsajwani
Copy link
Copy Markdown
Contributor

@rsajwani rsajwani commented Mar 21, 2023

Description

Downgrad/Upgrade test (Reparent New Vtctl) is failing on v16.0 (https://github.com/vitessio/vitess/actions/runs/4466836285/jobs/7865121597)

In #12206 we added an additional field "super_read_only" to FullStatus message, with upgrade downgrade test we use N+1 (17) version of Vtctl along with N (v16) version of VTTablet. Since the test code does not have updated proto definition of FullStatus message (replicationdata.proto), it fails to Unmarshal the message object.

Solution:

1- If I use

opt := protojson.UnmarshalOptions{DiscardUnknown: true} 
err = opt.Unmarshal([]byte(primaryStatusString), primaryStatus)

then this will disregard the missing fields ...

Related Issue(s)

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

Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
@rsajwani rsajwani self-assigned this Mar 22, 2023
@rsajwani rsajwani changed the title fix fullstatus test for backward compat Fix fullstatus test for backward compat Mar 22, 2023
require.NoError(t, err)
primaryStatus := &replicationdatapb.FullStatus{}
err = protojson.Unmarshal([]byte(primaryStatusString), primaryStatus)
opt := protojson.UnmarshalOptions{DiscardUnknown: true}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is to make sure upgrade/downgrade test are backward compatible.

@rsajwani rsajwani marked this pull request as ready for review March 22, 2023 18:21
@rsajwani rsajwani requested a review from deepthi as a code owner March 22, 2023 18:21
@deepthi deepthi requested a review from GuptaManan100 March 22, 2023 20:55
Copy link
Copy Markdown
Contributor

@GuptaManan100 GuptaManan100 left a comment

Choose a reason for hiding this comment

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

Just a suggestion. Other than that, LGTM

@GuptaManan100
Copy link
Copy Markdown
Contributor

Just realized, I am not a code-owner for the reparent endtoend tests. I'll fix that in a separate PR.

@mattlord mattlord merged commit 15f491e into vitessio:main Mar 24, 2023
@mattlord mattlord deleted the fix-fullstatus-api branch March 24, 2023 00:04
rsajwani added a commit to planetscale/vitess that referenced this pull request Mar 24, 2023
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
GuptaManan100 pushed a commit that referenced this pull request Mar 28, 2023
Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
@hmaurer hmaurer mentioned this pull request Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: General Changes throughout the code base Type: Bug Type: Testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants