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

Support OVS bridge creation for secondary network #5279

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

jianjuns
Copy link
Contributor

Add OVS bridge configuration to the secondary network configuration in antrea-agent.conf, which specifies the OVS bridges for Pod secondary networks and also physical interfaces of the bridges. At the moment, only a single bridge is supported and at most one physical interface can be configured on the bridge. antrea-agent will automatically create the OVS bridge and connects the physical interface (if specified) to the bridge, when the bridge is specified in the secondary network configuration and does not exist on the host.

Issue: #5278

@jianjuns jianjuns added area/component/cni Issues or PRs related to the cni component area/interface Issues or PRs related to network interfaces labels Jul 20, 2023
@jianjuns jianjuns force-pushed the sec-bridge branch 4 times, most recently from df05751 to 4fd4c8b Compare July 20, 2023 23:31
@jianjuns jianjuns added area/ovs/ovsdb Issues or PRs related to OVS database. and removed area/component/cni Issues or PRs related to the cni component labels Jul 21, 2023
@jianjuns jianjuns force-pushed the sec-bridge branch 2 times, most recently from 48a365d to 0876de0 Compare July 22, 2023 22:15
@jianjuns jianjuns marked this pull request as ready for review July 22, 2023 22:16
pkg/agent/secondarynetwork/init.go Outdated Show resolved Hide resolved
pkg/config/agent/config.go Outdated Show resolved Hide resolved
@jianjuns jianjuns force-pushed the sec-bridge branch 3 times, most recently from 3b202b7 to 3033f4d Compare July 29, 2023 16:59
build/charts/antrea/values.yaml Outdated Show resolved Hide resolved
@@ -597,7 +597,7 @@ func (o *Options) validateK8sNodeOptions() error {
o.dnsServerOverride = hostPort
}

return nil
return o.validateSecondaryNetworkConfig()
Copy link
Contributor

Choose a reason for hiding this comment

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

this should follow the same convention as above, and wrap the error:

	if err := o.validateAntreaProxyConfig(encapMode); err != nil {
		return fmt.Errorf("proxy config is invalid: %w", err)
	}
	if err := o.validateFlowExporterConfig(); err != nil {
		return fmt.Errorf("failed to validate flow exporter config: %v", err)
	}
	if err := o.validateMulticastConfig(); err != nil {
		return fmt.Errorf("failed to validate multicast config: %v", err)
	}
	if err := o.validateEgressConfig(encapMode); err != nil {
		return fmt.Errorf("failed to validate egress config: %v", err)
	}

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

Comment on lines 129 to 130
defer mockNewOVSBridge(mockOVSBridgeClient)()
defer mockInterfaceByName()()
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use the new t.Cleanup functionality here, to simplify calling the functions and ensure that cleanup always happens correctly. For example:

func mockInterfaceByName(t *testing.T) func() {
	prevFunc := interfaceByNameFn
	interfaceByNameFn = func(name string) (*net.Interface, error) {
		if name == nonExistingInterface {
			return nil, errors.New("interface not found")
		}
		return nil, nil
	}
	t.Cleanup(func() { interfaceByNameFn = prevFunc })
}

// in caller
mockInterfaceByName(t)

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

Add OVS bridge configuration to the secondary network configuration in
antrea-agent.conf, which specifies the OVS bridges for Pod secondary
networks and also physical interfaces of the bridges. At the moment,
only a single bridge is supported and at most one physical interface
can be configured on the bridge. antrea-agent will automatically create
the OVS bridge and connects the physical interface (if specified) to
the bridge, when the bridge is specified in the secondary network
configuration and does not exist on the host.

Signed-off-by: Jianjun Shen <shenj@vmware.com>
@jianjuns
Copy link
Contributor Author

jianjuns commented Aug 6, 2023

Rebased on top of #5341.

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

Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM

@jianjuns
Copy link
Contributor Author

jianjuns commented Aug 7, 2023

/test-all

@jianjuns
Copy link
Contributor Author

jianjuns commented Aug 7, 2023

/test-conformance

@jianjuns jianjuns merged commit 8ac8a91 into antrea-io:main Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/interface Issues or PRs related to network interfaces area/ovs/ovsdb Issues or PRs related to OVS database.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants