Skip to content

Commit

Permalink
Use standard value type for k8s.v1.cni.cncf.io/networks annotation
Browse files Browse the repository at this point in the history
For SecondaryNetwork support, we switch to using the standard value type
for the k8s.v1.cni.cncf.io/networks annotation:
https://pkg.go.dev/github.com/k8snetworkplumbingwg/network-attachment-definition-client@v1.3.0/pkg/apis/k8s.cni.cncf.io/v1#NetworkSelectionElement

By switching to the standard type:
* we avoid user confusion
* we ensure compatibility with other solutions such as Multus
* we support "edge" cases that were not supported before: 1) using a
  NetworkAttachmentDefinition CR defined in a different Namespace than
  the Pod, 2) providing the NetworkAttachmentDefinition CR name as
  `[namespace/]name[@interface]`
* we can use the standard ParseNetworkAnnotation function and avoid
  potential bugs or inconsistencies.

This means that it is no longer possible to provide an "interface type"
as part of the annotation value. However, this is not really a standard
concept, as this information is typically provided in the
NetworkAttachmentDefinition CR. For example, "bridge" mode for
"macvlan". And in our case we use "sriov" mode (for the "antrea" network
type).

In addition to this change, we add a good amount of unit tests, in the
context of #4142.

Signed-off-by: Antonin Bas <abas@vmware.com>
  • Loading branch information
antoninbas committed Aug 23, 2022
1 parent 4504127 commit 931a7d4
Show file tree
Hide file tree
Showing 9 changed files with 671 additions and 128 deletions.
3 changes: 2 additions & 1 deletion cmd/antrea-agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,8 @@ func run(o *Options) error {
localPodInformer,
nodeConfig.Name,
cniPodInfoStore,
cniServer)
// safe to call given that cniServer.Initialize has been called already.
cniServer.GetPodConfigurator())
go podWatchController.Run(stopCh)
}

Expand Down
2 changes: 2 additions & 0 deletions hack/update-codegen-dockerized.sh
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ function generate_mocks {
"pkg/agent/querier AgentQuerier testing"
"pkg/agent/route Interface testing"
"pkg/agent/ipassigner IPAssigner testing"
"pkg/agent/secondarynetwork/podwatch InterfaceConfigurator testing"
"pkg/agent/secondarynetwork/ipam IPAMDelegator testing"
"pkg/antctl AntctlClient ."
"pkg/controller/networkpolicy EndpointQuerier testing"
"pkg/controller/querier ControllerQuerier testing"
Expand Down
8 changes: 4 additions & 4 deletions pkg/agent/cniserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,10 @@ type CNIConfig struct {
}

// updateResultIfaceConfig processes the result from the IPAM plugin and does the following:
// * updates the IP configuration for each assigned IP address: this includes computing the
// - updates the IP configuration for each assigned IP address: this includes computing the
// gateway (if missing) based on the subnet and setting the interface pointer to the container
// interface
// * if there is no default route, add one using the provided default gateway
// - if there is no default route, add one using the provided default gateway
func updateResultIfaceConfig(result *current.Result, defaultIPv4Gateway net.IP, defaultIPv6Gateway net.IP) {
for _, ipc := range result.IPs {
// result.Interfaces[0] is host interface, and result.Interfaces[1] is container interface
Expand Down Expand Up @@ -204,7 +204,7 @@ func (s *CNIServer) isCNIVersionSupported(reqVersion string) bool {
return exist
}

func (s *CNIServer) valiateCNIAndIPAMType(cniConfig *CNIConfig) *cnipb.CniCmdResponse {
func (s *CNIServer) validateCNIAndIPAMType(cniConfig *CNIConfig) *cnipb.CniCmdResponse {
var ipamType string
if cniConfig.IPAM != nil {
ipamType = cniConfig.IPAM.Type
Expand Down Expand Up @@ -251,7 +251,7 @@ func (s *CNIServer) validateRequestMessage(request *cnipb.CniCmdRequest) (*CNICo
return nil, s.incompatibleCniVersionResponse(cniVersion)
}

if resp := s.valiateCNIAndIPAMType(cniConfig); resp != nil {
if resp := s.validateCNIAndIPAMType(cniConfig); resp != nil {
return nil, resp
}
if !s.isChaining && !cniConfig.secondaryNetworkIPAM {
Expand Down
15 changes: 13 additions & 2 deletions pkg/agent/secondarynetwork/ipam/ipam_delegator.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,18 @@ const (
defaultCNIPath = "/opt/cni/bin"
)

func GetIPAMSubnetAddress(netConfig []byte, cmdArgs *invoke.Args) (*current.Result, error) {
type IPAMDelegator interface {
GetIPAMSubnetAddress(netConfig []byte, cmdArgs *invoke.Args) (*current.Result, error)
DelIPAMSubnetAddress(netConfig []byte, cmdArgs *invoke.Args) error
}

type delegator struct{}

func NewIPAMDelegator() *delegator {
return &delegator{}
}

func (d *delegator) GetIPAMSubnetAddress(netConfig []byte, cmdArgs *invoke.Args) (*current.Result, error) {
var success = false
defer func() {
if !success {
Expand All @@ -56,7 +67,7 @@ func GetIPAMSubnetAddress(netConfig []byte, cmdArgs *invoke.Args) (*current.Resu
return ipamResult, nil
}

func DelIPAMSubnetAddress(netConfig []byte, cmdArgs *invoke.Args) error {
func (d *delegator) DelIPAMSubnetAddress(netConfig []byte, cmdArgs *invoke.Args) error {
cmdArgs.Command = "DEL"
if err := delegateNoResult(ipamWhereabouts, netConfig, cmdArgs); err != nil {
return err
Expand Down
79 changes: 79 additions & 0 deletions pkg/agent/secondarynetwork/ipam/testing/mock_ipam.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 931a7d4

Please sign in to comment.