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

Export Service's name to support multi-ports #3561

Merged
merged 1 commit into from
Apr 7, 2022

Conversation

luolanzone
Copy link
Contributor

@luolanzone luolanzone commented Mar 31, 2022

In v1.5, we removed original Service name during export/import, but the CNI will fail to realize the Service/Endpoint due to imported Endpoint still has original name, we export Service's name in this PR so we can fix this kind of issue and also support Service with multiple ports.

Fixes #3588

Signed-off-by: Lan Luo [email protected]

@luolanzone
Copy link
Contributor Author

/test-multicluster-e2e

@codecov-commenter
Copy link

codecov-commenter commented Mar 31, 2022

Codecov Report

Merging #3561 (93ac6fb) into main (e13869a) will increase coverage by 26.03%.
The diff coverage is 64.70%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #3561       +/-   ##
===========================================
+ Coverage   38.12%   64.16%   +26.03%     
===========================================
  Files         114      278      +164     
  Lines       15271    27821    +12550     
===========================================
+ Hits         5822    17850    +12028     
+ Misses       8967     8061      -906     
- Partials      482     1910     +1428     
Flag Coverage Δ
integration-tests ?
kind-e2e-tests 51.03% <62.50%> (?)
unit-tests 43.51% <100.00%> (?)

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

Impacted Files Coverage Δ
pkg/agent/route/route_linux.go 47.61% <62.50%> (ø)
...trollers/multicluster/resourceexport_controller.go 69.50% <100.00%> (ø)
...ntrollers/multicluster/serviceexport_controller.go 66.97% <100.00%> (ø)
antrea/pkg/ovs/ovsconfig/errors.go
antrea/pkg/agent/openflow/groups.go
antrea/pkg/agent/openflow/pipeline.go
antrea/pkg/util/k8s/client.go
...ea/pkg/agent/ipassigner/responder/arp_responder.go
antrea/pkg/ovs/ovsctl/ovsctl_others.go
antrea/pkg/agent/util/ethtool/ethtool_linux.go
... and 385 more

@luolanzone
Copy link
Contributor Author

/test-multicluster-e2e

@luolanzone
Copy link
Contributor Author

/test-multicluster-e2e

@luolanzone luolanzone added the area/multi-cluster Issues or PRs related to multi cluster. label Mar 31, 2022
@luolanzone
Copy link
Contributor Author

/test-multicluster-e2e

@luolanzone
Copy link
Contributor Author

/test-multicluster-e2e

@luolanzone
Copy link
Contributor Author

/test-multicluster-e2e

2 similar comments
@hjiajing
Copy link
Contributor

/test-multicluster-e2e

@hjiajing
Copy link
Contributor

/test-multicluster-e2e

@luolanzone luolanzone marked this pull request as draft March 31, 2022 09:15
@luolanzone luolanzone marked this pull request as ready for review April 1, 2022 01:22
@luolanzone luolanzone requested review from jianjuns and tnqn April 1, 2022 01:22
@luolanzone
Copy link
Contributor Author

/test-multicluster-e2e

1 similar comment
@luolanzone
Copy link
Contributor Author

/test-multicluster-e2e

tnqn
tnqn previously approved these changes Apr 1, 2022
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -412,8 +412,8 @@ converging the update until users correct it to match the Service definition in
ResourceImport.
2. When a member cluster has already exported a Service, e.g.: `default/nginx` with TCP
Port `80`, then other member clusters can only export the same Service with the same Ports
definition. Otherwise, Antrea Multi-cluster Controller will skip converging the mismatched
ResourceExport into the corresponding ResourceImport until users correct it.
definition including port's name. Otherwise, Antrea Multi-cluster Controller will skip converging
Copy link
Contributor

Choose a reason for hiding this comment

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

port names or ports' names

converging -> converting?

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

@tnqn
Copy link
Member

tnqn commented Apr 6, 2022

/skip-all
/test-multicluster-e2e

@luolanzone
Copy link
Contributor Author

/test-multicluster-e2e

1 similar comment
@XinShuYang
Copy link
Contributor

/test-multicluster-e2e

@tnqn tnqn added action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. kind/bug Categorizes issue or PR as related to a bug. labels Apr 7, 2022
@tnqn tnqn merged commit 03b3f2b into antrea-io:main Apr 7, 2022
@luolanzone luolanzone deleted the fix-multiports branch April 7, 2022 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. area/multi-cluster Issues or PRs related to multi cluster. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fail to realize MC service when it has port name
6 participants