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

Handle breaking change for ServiceVirtualIP restore #14149

Merged
merged 3 commits into from
Aug 11, 2022

Conversation

kisunji
Copy link
Contributor

@kisunji kisunji commented Aug 11, 2022

fixes #14107

Description

Consul 1.13.0 changed ServiceVirtualIP to use PeeredServiceName instead of ServiceName which was a breaking change for those using service mesh and wanted to restore their snapshot after upgrading to 1.13.0.

This commit handles existing data with older ServiceName and converts it during restore so that there are no incompatibility issues when restoring from older snapshots.

Testing & Reproduction steps

  • Manually tested in OSS and ENT by making a snapshot using Consul 1.12.3, then attempting to restore with the changes in this branch.
  • Added unit tests restoring both versions of the struct

Consul 1.13.0 changed ServiceVirtualIP to use PeeredServiceName instead of ServiceName which was a breaking change for those using service mesh and wanted to restore their snapshot after upgrading to 1.13.0.

This commit handles existing data with older ServiceName and converts it during restore so that there are no issues when restoring from older snapshots.
Name string
acl.EnterpriseMeta
Name string
acl.EnterpriseMeta `mapstructure:",squash"`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noticed in testing that mapstructure does not handle embedded structs without this tag

Copy link
Member

@mkeeler mkeeler left a comment

Choose a reason for hiding this comment

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

The code looks good. Would it be possible to add a test?

@kisunji
Copy link
Contributor Author

kisunji commented Aug 11, 2022

The code looks good. Would it be possible to add a test?

Yep I'll try to write a test for this

@kisunji kisunji added backport/1.13 theme/cluster-peering Related to Consul's cluster peering feature labels Aug 11, 2022
@kisunji kisunji changed the title Handle breaking change for ServiceVirtualIP restore. Handle breaking change for ServiceVirtualIP restore Aug 11, 2022
@kisunji kisunji requested a review from mkeeler August 11, 2022 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/cluster-peering Related to Consul's cluster peering feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to start up the cluster when upgrading to 1.13.0
2 participants