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

xds: enable XDS federation by default #6151

Merged
merged 3 commits into from
Mar 27, 2023
Merged

xds: enable XDS federation by default #6151

merged 3 commits into from
Mar 27, 2023

Conversation

apolcyn
Copy link
Contributor

@apolcyn apolcyn commented Mar 24, 2023

Integration tests have been green, so let's enable this (verification of recent test results in https://b.corp.google.com/issues/262593165#comment31).

cc @easwars

RELEASE NOTES:

  • federation: enable support variable by default

@easwars easwars added the Type: Behavior Change Behavior changes not categorized as bugs label Mar 24, 2023
@easwars easwars added this to the 1.55 Release milestone Mar 24, 2023
defer func() func() {
oldEnv := envconfig.XDSFederation
envconfig.XDSFederation = true
envconfig.XDSFederation = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we fix the failing test instead of disabling federation for the duration of this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I might be reading this wrong - this should only be disabling federation for the duration of the one test case that explicitly disables it, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh sorry, I missed the if !tt.env.

Can we simplify this by getting rid of the if !tt.env?

	oldEnv := envconfig.XDSFederation
	envconfig.XDSFederation = tt.env
	defer func() { envconfig.XDSFederation = oldEnv }()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@apolcyn
Copy link
Contributor Author

apolcyn commented Mar 27, 2023

Thanks for the review @easwars. Please merge if this looks good to you (I don't have write access)

@easwars easwars merged commit 44cebb8 into grpc:master Mar 27, 2023
1 check passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Behavior Change Behavior changes not categorized as bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants