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
…4146)

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/[email protected]/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" networkType.

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

Signed-off-by: Antonin Bas <[email protected]>
  • Loading branch information
antoninbas authored Aug 29, 2022
1 parent 53b639f commit 222c2b6
Show file tree
Hide file tree
Showing 10 changed files with 686 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
4 changes: 2 additions & 2 deletions pkg/agent/cniserver/server.go
Original file line number Diff line number Diff line change
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
2 changes: 1 addition & 1 deletion pkg/agent/secondarynetwork/cnipodcache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func getCNIPodInfoKey(obj interface{}) (string, error) {
return key, nil
}

func NewCNIPodInfoStore() CNIPodInfoStore {
func NewCNIPodInfoStore() *CNIPodInfoCache {
return &CNIPodInfoCache{
cache: cache.NewIndexer(getCNIPodInfoKey, cache.Indexers{
podIndex: podIndexFunc,
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 222c2b6

Please sign in to comment.