Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions pkg/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,17 @@ func CmdAdd(args *skel.CmdArgs) error {
return err
}

// Check for OvnPort and MAC in args.cni (CNI convention)
// args.cni takes precedence over CNI_ARGS per CNI spec
if netconf.Args != nil && netconf.Args.CNI != nil {
if netconf.Args.CNI.OvnPort != "" {
ovnPort = netconf.Args.CNI.OvnPort
}
if netconf.Args.CNI.MAC != "" {
mac = netconf.Args.CNI.MAC
}
}
Comment on lines +282 to +289

Choose a reason for hiding this comment

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

high

While this logic correctly handles args.cni for the ADD command, the CmdCheck function has not been updated with similar logic. CmdCheck also uses ovnPort to determine the bridge name, but it currently only reads it from CNI_ARGS. This will cause CHECK to fail when OvnPort is provided via args.cni, as it won't be able to determine the correct bridge name for validation. Please update CmdCheck to also read ovnPort from args.cni with the same precedence to ensure consistent behavior.


var vlanTagNum uint = 0
trunks := make([]uint, 0)
portType := "access"
Expand Down Expand Up @@ -558,6 +569,12 @@ func CmdDel(args *skel.CmdArgs) error {
if envArgs != nil {
ovnPort = string(envArgs.OvnPort)
}
// Check for OvnPort in args.cni (CNI convention) - takes precedence
if cache.Netconf.Args != nil && cache.Netconf.Args.CNI != nil {
if cache.Netconf.Args.CNI.OvnPort != "" {
ovnPort = cache.Netconf.Args.CNI.OvnPort
}
}
ovsDriver, err := ovsdb.NewOvsDriver(cache.Netconf.SocketFile)
if err != nil {
return err
Expand Down Expand Up @@ -679,6 +696,12 @@ func CmdCheck(args *skel.CmdArgs) error {
if envArgs != nil {
ovnPort = string(envArgs.OvnPort)
}
// Check for OvnPort in args.cni (CNI convention) - takes precedence
if netconf.Args != nil && netconf.Args.CNI != nil {
if netconf.Args.CNI.OvnPort != "" {
ovnPort = netconf.Args.CNI.OvnPort
}
}
ovsDriver, err := ovsdb.NewOvsDriver(netconf.SocketFile)
if err != nil {
return err
Expand Down
64 changes: 64 additions & 0 deletions pkg/plugin/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,70 @@ var testFunc = func(version string) {
Expect(string(output[:len(output)-1])).To(Equal(ovsOutput))
})
})
Context("specified OvnPort via args.cni", func() {
It("should successfully complete ADD, CHECK and DEL commands with args.cni OvnPort", func() {
const ovsOutput = "external_ids : {iface-id=test-port-from-args}"

// OvnPort specified in args.cni (CNI convention) instead of CNI_ARGS
conf := fmt.Sprintf(`{
"cniVersion": "%s",
"name": "mynet",
"type": "ovs",
"bridge": "%s",
"args": {
"cni": {
"OvnPort": "test-port-from-args"
}
}}`, version, bridgeName)

targetNs := newNS()
defer func() {
closeNS(targetNs)
}()

By("Calling ADD command with args.cni OvnPort")
hostIfName, result := testAdd(conf, false, false, "", targetNs)
hostIface := result.(*current.Result).Interfaces[0]

By("Verifying iface-id was set from args.cni")
output, err := exec.Command("ovs-vsctl", "--column=external_ids", "find", "Interface", fmt.Sprintf("name=%s", hostIface.Name)).CombinedOutput()
Expect(err).NotTo(HaveOccurred())
Expect(string(output[:len(output)-1])).To(Equal(ovsOutput))

By("Calling CHECK command")
testCheck(conf, result, targetNs)

By("Calling DEL command")
testDel(conf, hostIfName, targetNs, true)
})
It("should prefer args.cni OvnPort over CNI_ARGS", func() {
// args.cni should take precedence per CNI conventions
const ovsOutput = "external_ids : {iface-id=args-cni-port}"

conf := fmt.Sprintf(`{
"cniVersion": "%s",
"name": "mynet",
"type": "ovs",
"bridge": "%s",
"args": {
"cni": {
"OvnPort": "args-cni-port"
}
}}`, version, bridgeName)

targetNs := newNS()
defer func() {
closeNS(targetNs)
}()

// Pass a different ovnPort via CNI_ARGS - args.cni should win
result := attach(targetNs, conf, IFNAME, "", "cni-args-port")
hostIface := result.Interfaces[0]
output, err := exec.Command("ovs-vsctl", "--column=external_ids", "find", "Interface", fmt.Sprintf("name=%s", hostIface.Name)).CombinedOutput()
Expect(err).NotTo(HaveOccurred())
Expect(string(output[:len(output)-1])).To(Equal(ovsOutput))
})
})
Context("specified OfportRequest", func() {
It("should configure an ovs interface with a specific ofport", func() {
// Pick a random ofport 5000-6000
Expand Down
11 changes: 11 additions & 0 deletions pkg/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ type NetConfs interface {
NetConf | MirrorNetConf
}

// CNIArgs contains arguments passed via args.cni in the CNI config
// This follows CNI conventions: https://www.cni.dev/docs/conventions/
type CNIArgs struct {
OvnPort string `json:"OvnPort,omitempty"`
MAC string `json:"MAC,omitempty"`
}

// NetConf extends types.NetConf for ovs-cni
type NetConf struct {
types.NetConf
Expand All @@ -40,6 +47,10 @@ type NetConf struct {
SocketFile string `json:"socket_file"`
LinkStateCheckRetries int `json:"link_state_check_retries"`
LinkStateCheckInterval int `json:"link_state_check_interval"`
// Args contains CNI args passed via args.cni in the config
Args *struct {
CNI *CNIArgs `json:"cni,omitempty"`
} `json:"args,omitempty"`
}

// MirrorNetConf extends types.NetConf for ovs-mirrors
Expand Down
110 changes: 110 additions & 0 deletions pkg/types/types_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// Copyright (c) 2026 VEXXHOST, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package types

import (
"encoding/json"
"testing"
)

func TestNetConfArgsCNIParsing(t *testing.T) {
tests := []struct {
name string
config string
expectedOvnPort string
expectedMAC string
}{
{
name: "args.cni with OvnPort",
config: `{
"cniVersion": "1.0.0",
"name": "test-net",
"type": "ovs",
"bridge": "br-int",
"args": {
"cni": {
"OvnPort": "test-port-id"
}
}
}`,
expectedOvnPort: "test-port-id",
expectedMAC: "",
},
{
name: "args.cni with OvnPort and MAC",
config: `{
"cniVersion": "1.0.0",
"name": "test-net",
"type": "ovs",
"bridge": "br-int",
"args": {
"cni": {
"OvnPort": "my-ovn-port",
"MAC": "fa:16:3e:aa:bb:cc"
}
}
}`,
expectedOvnPort: "my-ovn-port",
expectedMAC: "fa:16:3e:aa:bb:cc",
},
{
name: "no args section",
config: `{
"cniVersion": "1.0.0",
"name": "test-net",
"type": "ovs",
"bridge": "br-int"
}`,
expectedOvnPort: "",
expectedMAC: "",
},
{
name: "empty args.cni",
config: `{
"cniVersion": "1.0.0",
"name": "test-net",
"type": "ovs",
"bridge": "br-int",
"args": {
"cni": {}
}
}`,
expectedOvnPort: "",
expectedMAC: "",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var netconf NetConf
if err := json.Unmarshal([]byte(tt.config), &netconf); err != nil {
t.Fatalf("Failed to unmarshal config: %v", err)
}

var gotOvnPort, gotMAC string
if netconf.Args != nil && netconf.Args.CNI != nil {
gotOvnPort = netconf.Args.CNI.OvnPort
gotMAC = netconf.Args.CNI.MAC
}

if gotOvnPort != tt.expectedOvnPort {
t.Errorf("OvnPort = %q, want %q", gotOvnPort, tt.expectedOvnPort)
}
if gotMAC != tt.expectedMAC {
t.Errorf("MAC = %q, want %q", gotMAC, tt.expectedMAC)
}
})
}
}