Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Add a failing e2e test for multiple ports issue for a service in the mesh #4026

Conversation

trstringer
Copy link
Contributor

Description:

Currently if you create a service in the mesh with multiple ports, it will cause unexpected and undesirable behavior due to how the proxy is configured. More information can be found on #3777.

This PR introduces a failing test to uncover this issue.

This PR is marked as a draft until a PR is opened to resolve the issue.

Testing done:

Added e2e test.

Affected area:

Functional Area
Tests [x]

Please answer the following questions with yes/no.

  1. Does this change contain code from or inspired by another project?

No.

  1. Is this a breaking change?

No.

@trstringer trstringer changed the title Add failing e2e test for multiple ports issue for a service in the mesh Add a failing e2e test for multiple ports issue for a service in the mesh Aug 23, 2021
Signed-off-by: Thomas Stringer <thstring@microsoft.com>
@trstringer trstringer force-pushed the thstring/multi-port-test-failure branch from a152fa5 to 62a3851 Compare August 23, 2021 20:19
…s before requiring only success

Signed-off-by: Thomas Stringer <thstring@microsoft.com>
@codecov-commenter
Copy link

Codecov Report

Merging #4026 (f296f7c) into main (4dcc321) will increase coverage by 0.76%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4026      +/-   ##
==========================================
+ Coverage   67.54%   68.31%   +0.76%     
==========================================
  Files         204      205       +1     
  Lines       11639    11631       -8     
==========================================
+ Hits         7862     7946      +84     
+ Misses       3726     3635      -91     
+ Partials       51       50       -1     
Flag Coverage Δ
unittests 68.31% <ø> (+0.76%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/injector/envoy_config.go 83.13% <0.00%> (-3.50%) ⬇️
pkg/certificate/rotor/rotor.go 84.37% <0.00%> (-3.13%) ⬇️
pkg/cli/environment.go 75.51% <0.00%> (-2.76%) ⬇️
cmd/cli/install.go 81.34% <0.00%> (-2.66%) ⬇️
pkg/trafficpolicy/trafficpolicy.go 96.02% <0.00%> (-2.62%) ⬇️
cmd/cli/osm.go 81.39% <0.00%> (-1.11%) ⬇️
cmd/cli/policy_check_pods.go 73.95% <0.00%> (-0.05%) ⬇️
pkg/envoy/bootstrap/config.go 93.91% <0.00%> (-0.05%) ⬇️
pkg/envoy/rds/route/route_config.go 100.00% <0.00%> (ø)
pkg/cli/policy_check.go 0.00% <0.00%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4dcc321...f296f7c. Read the comment docs.

@shashankram
Copy link
Member

I have mirrored this test to the PR that resolves this issue: https://github.com/openservicemesh/osm/pull/4074/commits

@shashankram shashankram added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 3, 2021
@shashankram
Copy link
Member

This test was cherry-picked to https://github.com/openservicemesh/osm/pull/4074/commits which was merged. Closing this since it's not required anymore.

@shashankram shashankram closed this Sep 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants