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

tests: Pin api submodule to compatibile version for upgrade tests #15830

Closed
wants to merge 1 commit into from

Conversation

eculver
Copy link
Contributor

@eculver eculver commented Dec 17, 2022

Description

This is an alternative to #15827. The details are in #15827 but the gist is that because the upgrade tests use the latest version of the api submodule, the test code will fail when running against a Consul 1.14 container instance because it is sending a config field that is not known. I was able to pull in a compatible api submodule using a replace directive. It seems to work, so I'm wondering if we should use this instead of removing the test code that triggers the compatibility issue.

Testing & Reproduction steps

cd test/integration/consul-container
go test -run TestPeering_UpgradeToTarget_fromLatest ./... --target-version local --latest-version latest

@eculver eculver added the pr/no-changelog PR does not need a corresponding .changelog entry label Dec 17, 2022
require (
github.com/docker/docker v20.10.11+incompatible
github.com/docker/go-connections v0.4.0
github.com/hashicorp/consul v1.14.1
github.com/hashicorp/consul/api v1.18.0
github.com/hashicorp/consul/api/v118 v118.0.0-00010101000000-000000000000
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know why the line above does not result in api/v1.18.0 from being used, but it doesn't, the replace directive appears to be required.

@huikang
Copy link
Contributor

huikang commented Dec 17, 2022

@eculver , there is another alternative: I think in Go the json tag will ignore ",omitempty" if it is a struct. So we may need to change all of them to pointer like *TransparentProxyConfig

consul/api/config_entry.go

Lines 294 to 298 in ab199a1

TransparentProxy *TransparentProxyConfig `json:",omitempty" alias:"transparent_proxy"`
Config map[string]interface{} `json:",omitempty"`
MeshGateway MeshGatewayConfig `json:",omitempty" alias:"mesh_gateway"`
Expose ExposeConfig `json:",omitempty"`
AccessLogs AccessLogsConfig `json:",omitempty"`

Please see #15831

@eculver
Copy link
Contributor Author

eculver commented Dec 19, 2022

Closing this in favor of #15831.

@eculver eculver closed this Dec 19, 2022
@eculver eculver deleted the eculver/upgrade-tests-api-module branch December 19, 2022 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/no-changelog PR does not need a corresponding .changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants