-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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
proxy network label should be set based on pod labels, system namespa… #54513
Conversation
…ce labels and meshNetwork config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG
@@ -286,13 +287,34 @@ func TestNewEndpointBuilderFromMetadataTopologyLabels(t *testing.T) { | |||
}, | |||
opts: Options{ClusterID: c.ctl.cluster}, | |||
} | |||
eb := cc.NewEndpointBuilderFromMetadata(c.proxy) | |||
eb := newEndpointBuilderFromMetadata(cc, c.proxy) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this test doing anything? it seems we are testing newEndpointBuilderFromMetadata but that function is now deadcode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, i just realized it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will fix
LGTM |
…ce labels and meshNetwork config
Please provide a description of this PR:
Fix #54381