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.

Sign up for GitHub

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

Jump to bottom

Validate serviceCIDR configuration only if AntreaProxy is disabled #2936

Merged
merged 1 commit into from
Nov 9, 2021
Merged

Validate serviceCIDR configuration only if AntreaProxy is disabled #2936

merged 1 commit into from
Nov 9, 2021

Conversation

wenyingd
Copy link
Contributor

Move the validation for serviceCIDR under the condition that AntreaProxy
is disabled. So that the user could not ignore the item when using
AntreaProxy instead of kube-proxy.

Fixes #2935

Signed-off-by: wenyingd wenyingd@vmware.com

@codecov-commenter
Copy link

codecov-commenter commented Oct 26, 2021 •
edited
Loading

Codecov Report

Merging #2936 (7f1fc37) into main (5db792d) will increase coverage by 1.22%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2936      +/-   ##
==========================================
+ Coverage   59.84%   61.06%   +1.22%     
==========================================
  Files         289      289              
  Lines       24551    24550       -1     
==========================================
+ Hits        14693    14992     +299     
+ Misses       8250     7932     -318     
- Partials     1608     1626      +18
Flag Coverage Δ
kind-e2e-tests 48.20% <ø> (+1.39%) ⬆️
unit-tests 40.18% <ø> (-0.02%) ⬇️

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

Impacted Files Coverage Δ
pkg/agent/route/route_linux.go 46.67% <ø> (+19.68%) ⬆️
...g/controller/networkpolicy/store/appliedtogroup.go 86.36% <0.00%> (-3.04%) ⬇️
...gent/controller/noderoute/node_route_controller.go 54.91% <0.00%> (-1.10%) ⬇️
pkg/controller/egress/controller.go 83.82% <0.00%> (-1.00%) ⬇️
pkg/ovs/ovsconfig/ovs_client.go 49.15% <0.00%> (+0.21%) ⬆️
pkg/agent/openflow/client.go 57.76% <0.00%> (+0.65%) ⬆️
...ntroller/networkpolicy/networkpolicy_controller.go 71.34% <0.00%> (+0.91%) ⬆️
...kg/agent/flowexporter/connections/conntrack_ovs.go 77.57% <0.00%> (+1.21%) ⬆️
pkg/monitor/controller.go 29.10% <0.00%> (+1.49%) ⬆️
pkg/ovs/openflow/ofctrl_action.go 69.58% <0.00%> (+1.66%) ⬆️
... and 10 more

@wenyingd
Copy link
Contributor Author

/test-all
/test-ipv6-all
/test-ipv6-only-all
/test-integration

@wenyingd
Copy link
Contributor Author

/test-windows-all

antoninbas
antoninbas reviewed Oct 26, 2021
View reviewed changes
cmd/antrea-agent/options.go Outdated Show resolved Hide resolved
cmd/antrea-agent/options.go Outdated Show resolved Hide resolved
antoninbas
antoninbas previously approved these changes Oct 27, 2021
View reviewed changes
@wenyingd
Copy link
Contributor Author

wenyingd commented Oct 27, 2021 •
edited
Loading

Since @lzhecheng has another PR(#2939) to optimize the windows routeClient, and I want to rebase this PR on his then I could remove serviceCIDR from routeClient in this one.

jianjuns
jianjuns previously approved these changes Oct 27, 2021
View reviewed changes
cmd/antrea-agent/options.go
@@ -240,8 +230,19 @@ func (o *Options) setDefaults() {
}

func (o *Options) validateAntreaProxyConfig() error {
if !features.DefaultFeatureGate.Enabled(features.AntreaProxy) && len(o.config.AntreaProxy.SkipServices) > 0 {
klog.InfoS("skipServices will be ignored because AntreaProxy is disabled", "skipServices", o.config.AntreaProxy.SkipServices)
if !features.DefaultFeatureGate.Enabled(features.AntreaProxy) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It is a little strange to validate non-AntreaProxy config in validateAntreaProxyConfig(). Probably not a big deal though.

@wenyingd wenyingd dismissed stale reviews from jianjuns and antoninbas via d42bd33 November 1, 2021 03:57
@wenyingd wenyingd force-pushed the issue_2935 branch 2 times, most recently from d42bd33 to a6d066e Compare November 1, 2021 04:09
@wenyingd
Copy link
Contributor Author

wenyingd commented Nov 1, 2021

/test-all
/test-ipv6-all
/test-ipv6-only-all
/test-integration
/test-windows-all
/test-windows-proxyall-e2e

@wenyingd
Copy link
Contributor Author

wenyingd commented Nov 1, 2021

/test-integration
/test-windows-conformance
/test-windows-networkpolicy
/test-windows-proxyall-e2e

jianjuns
jianjuns previously approved these changes Nov 1, 2021
View reviewed changes
@wenyingd
Copy link
Contributor Author

wenyingd commented Nov 2, 2021

/test-windows-conformance

2 similar comments
@wenyingd
Copy link
Contributor Author

wenyingd commented Nov 2, 2021

/test-windows-conformance

@wenyingd
Copy link
Contributor Author

wenyingd commented Nov 2, 2021

/test-windows-conformance

@wenyingd
Validate serviceCIDR configuration if AntreaProxy is disabled
7f1fc37 
Move the validation for serviceCIDR under the condition that AntreaProxy
is disabled. So that the user could not ignore the item when using
AntreaProxy instead of kube-proxy.

Signed-off-by: wenyingd <wenyingd@vmware.com>
@wenyingd
Copy link
Contributor Author

wenyingd commented Nov 5, 2021

/test-all
/test-ipv6-all
/test-ipv6-only-all
/test-windows-all
/test-windows-proxyall-e2e

Rerun tests after rebasing the code

@wenyingd
Copy link
Contributor Author

wenyingd commented Nov 8, 2021

/test-all
/test-ipv6-all
/test-ipv6-only-all
/test-windows-all
/test-windows-proxyall-e2e

@wenyingd
Copy link
Contributor Author

wenyingd commented Nov 8, 2021

/test-integration

@tnqn
Copy link
Member

tnqn commented Nov 9, 2021

/test-integration

@wenyingd
Copy link
Contributor Author

wenyingd commented Nov 9, 2021

/test-integration
/test-flexible-ipam-e2e
/test-hw-offload

@wenyingd
Copy link
Contributor Author

wenyingd commented Nov 9, 2021

/test-integration

@tnqn tnqn merged commit 6097f37 into antrea-io:main Nov 9, 2021
qiyueyao added a commit to Dyanngg/antrea that referenced this pull request Nov 9, 2021
@qiyueyao
Revert "Validate serviceCIDR configuration if AntreaProxy is disabled (…
f9dea0c 
…antrea-io#2936)"

This reverts commit 6097f37.
@wenyingd wenyingd deleted the issue_2935 branch August 15, 2022 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jianjuns jianjuns jianjuns approved these changes

@tnqn tnqn tnqn approved these changes

@antoninbas antoninbas antoninbas left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove the validation of serviceCIDR in Agent configuration if AntreaProxy is enabled
5 participants
@wenyingd @codecov-commenter @tnqn @antoninbas @jianjuns