Skip to content

Commit

Permalink
fix: hostdevicenetwork IPAM
Browse files Browse the repository at this point in the history 
An empty IPAM string in the HostDeviceNetwork
Spec was rendered without brackets, making the
NAD config a not valid json.

Signed-off-by: Fred Rolland <frolland@nvidia.com>
  • Loading branch information
@rollandf
rollandf committed Feb 21, 2024
1 parent f981a63 commit 9df5609
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 51 deletions.
2 changes: 1 addition & 1 deletion manifests/state-hostdevice-network/0010-hostdevice-net-cr.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ spec:
"cniVersion":"0.3.1",
"name":"{{.HostDeviceNetworkName}}",
"type":"host-device",
"ipam": {{.CrSpec.IPAM}}
"ipam":{{or .CrSpec.IPAM "{}"}}
}'
27 changes: 27 additions & 0 deletions pkg/state/common_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package state_test

import (
"encoding/json"

clustertype_mocks "github.com/Mellanox/network-operator/pkg/clustertype/mocks"
"github.com/Mellanox/network-operator/pkg/state"
"github.com/Mellanox/network-operator/pkg/staticconfig"
Expand All @@ -33,3 +35,28 @@ func getTestCatalog() state.InfoCatalog {
catalog.Add(state.InfoTypeClusterType, &clusterTypeProvider)
return catalog
}

type ipam struct {
Type string `json:"type"`
Range string `json:"range"`
Exclude []string `json:"exclude"`
}

type nadConfig struct {
CNIVersion string `json:"cniVersion"`
Name string `json:"name"`
Type string `json:"type"`
Master string `json:"master"`
Mode string `json:"mode"`
MTU int `json:"mtu"`
IPAM ipam `json:"ipam"`
}

func getNADConfig(jsonData string) (*nadConfig, error) {
config := &nadConfig{}
err := json.Unmarshal([]byte(jsonData), &config)
if err != nil {
return nil, err
}
return config, nil
}
167 changes: 117 additions & 50 deletions pkg/state/state_hostdevice_network_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,78 +14,145 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package state
package state_test

import (
"strings"
"context"
"fmt"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"

netattdefv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"

mellanoxv1alpha1 "github.com/Mellanox/network-operator/api/v1alpha1"
"github.com/Mellanox/network-operator/pkg/render"
"github.com/Mellanox/network-operator/pkg/testing/mocks"
"github.com/Mellanox/network-operator/pkg/utils"
)
"github.com/Mellanox/network-operator/pkg/state"

func checkResourceNameAnnotation(obj *unstructured.Unstructured) {
annotations := obj.Object["metadata"].(map[string]interface{})["annotations"].(map[string]interface{})
resourceName := annotations["k8s.v1.cni.cncf.io/resourceName"].(string)
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

Expect(HavePrefix(resourceName, resourceNamePrefix))
Expect(strings.Count(resourceName, resourceNamePrefix)).To(Equal(1))
}
const resourceNamePrefix = "nvidia.com/"

var _ = Describe("HostDevice Network State rendering tests", func() {

var hostDeviceNetState state.State
var catalog state.InfoCatalog
var client client.Client

BeforeEach(func() {
scheme := runtime.NewScheme()
Expect(mellanoxv1alpha1.AddToScheme(scheme)).NotTo(HaveOccurred())
Expect(netattdefv1.AddToScheme(scheme)).NotTo(HaveOccurred())
client = fake.NewClientBuilder().WithScheme(scheme).Build()
manifestDir := "../../manifests/state-hostdevice-network"
s, err := state.NewStateHostDeviceNetwork(client, manifestDir)
Expect(err).NotTo(HaveOccurred())
hostDeviceNetState = s
catalog = getTestCatalog()
})

Context("HostDevice Network State", func() {
It("Should Render NetworkAttachmentDefinition", func() {
client := mocks.ControllerRuntimeClient{}
manifestBaseDir := "../../manifests/state-hostdevice-network"

files, err := utils.GetFilesWithSuffix(manifestBaseDir, render.ManifestFileSuffix...)
testName := "host-device"
testResourceName := "test"
cr := getHostDeviceNetwork(testName, testResourceName)
err := client.Create(context.Background(), cr)
Expect(err).NotTo(HaveOccurred())
renderer := render.NewRenderer(files)

stateName := "state-host-device-network"
sriovDpState := stateHostDeviceNetwork{
stateSkel: stateSkel{
name: stateName,
description: "Host Device net-attach-def CR deployed in cluster",
client: &client,
renderer: renderer,
},
}

status, err := hostDeviceNetState.Sync(context.Background(), cr, catalog)
Expect(err).NotTo(HaveOccurred())
Expect(sriovDpState.Name()).To(Equal(stateName))

namespace := "namespace"
name := "test_resource_without_prefix"
ipam := "fake IPAM"
spec := &mellanoxv1alpha1.HostDeviceNetworkSpec{}
spec.NetworkNamespace = namespace
spec.ResourceName = name
spec.IPAM = ipam

cr := &mellanoxv1alpha1.HostDeviceNetwork{}
cr.Name = name
cr.Spec = *spec
objs, err := sriovDpState.getManifestObjects(cr, testLogger)
Expect(status).To(BeEquivalentTo(state.SyncStateReady))

By("Verify NetworkAttachmentDefinition")
nad := &netattdefv1.NetworkAttachmentDefinition{}
err = client.Get(context.Background(), types.NamespacedName{Namespace: testNamespace, Name: testName}, nad)
Expect(err).NotTo(HaveOccurred())
Expect(len(objs)).To(Equal(1))

checkRenderedNetAttachDef(objs[0], namespace, name, ipam)
checkResourceNameAnnotation(objs[0])
nadConfig, err := getNADConfig(nad.Spec.Config)
Expect(err).NotTo(HaveOccurred())
Expect(nadConfig.Type).To(Equal("host-device"))
expectedNad := getExpectedHostDeviceNetNAD(testName, "{}")
Expect(nad.Spec).To(BeEquivalentTo(expectedNad.Spec))
Expect(nad.Name).To(Equal(testName))
Expect(nad.Namespace).To(Equal(testNamespace))
Expect(err).NotTo(HaveOccurred())
resourceName, ok := nad.Annotations["k8s.v1.cni.cncf.io/resourceName"]
Expect(ok).To(BeTrue())
Expect(resourceName).To(Equal(resourceNamePrefix + testResourceName))
})
It("Should Render NetworkAttachmentDefinition with resource with prefix", func() {
testName := "host-device"
testResourceName := resourceNamePrefix + "test"
cr := getHostDeviceNetwork(testName, testResourceName)
err := client.Create(context.Background(), cr)
Expect(err).NotTo(HaveOccurred())
status, err := hostDeviceNetState.Sync(context.Background(), cr, catalog)
Expect(err).NotTo(HaveOccurred())
Expect(status).To(BeEquivalentTo(state.SyncStateReady))

spec.ResourceName = resourceNamePrefix + "test_resource_with_prefix"
objs, err = sriovDpState.getManifestObjects(cr, testLogger)
By("Verify NetworkAttachmentDefinition")
nad := &netattdefv1.NetworkAttachmentDefinition{}
err = client.Get(context.Background(), types.NamespacedName{Namespace: testNamespace, Name: testName}, nad)
Expect(err).NotTo(HaveOccurred())
nadConfig, err := getNADConfig(nad.Spec.Config)
Expect(err).NotTo(HaveOccurred())
Expect(nadConfig.Type).To(Equal("host-device"))
expectedNad := getExpectedHostDeviceNetNAD(testName, "{}")
Expect(nad.Spec).To(BeEquivalentTo(expectedNad.Spec))
Expect(nad.Name).To(Equal(testName))
Expect(nad.Namespace).To(Equal(testNamespace))
Expect(err).NotTo(HaveOccurred())
resourceName, ok := nad.Annotations["k8s.v1.cni.cncf.io/resourceName"]
Expect(ok).To(BeTrue())
Expect(resourceName).To(Equal(testResourceName))
})
It("Should Render NetworkAttachmentDefinition with IPAM", func() {
ipam := "{\"type\":\"whereabouts\",\"range\":\"192.168.2.225/28\"," +
"\"exclude\":[\"192.168.2.229/30\",\"192.168.2.236/32\"]}"
testName := "host-device"
testResourceName := "test"
cr := getHostDeviceNetwork(testName, testResourceName)
cr.Spec.IPAM = ipam
err := client.Create(context.Background(), cr)
Expect(err).NotTo(HaveOccurred())
status, err := hostDeviceNetState.Sync(context.Background(), cr, catalog)
Expect(err).NotTo(HaveOccurred())
Expect(status).To(BeEquivalentTo(state.SyncStateReady))

By("Verify NetworkAttachmentDefinition")
nad := &netattdefv1.NetworkAttachmentDefinition{}
err = client.Get(context.Background(), types.NamespacedName{Namespace: testNamespace, Name: testName}, nad)
Expect(err).NotTo(HaveOccurred())
nadConfig, err := getNADConfig(nad.Spec.Config)
Expect(err).NotTo(HaveOccurred())
checkResourceNameAnnotation(objs[0])
Expect(nadConfig.Type).To(Equal("host-device"))
Expect(nad.Name).To(Equal(testName))
Expect(nad.Namespace).To(Equal(testNamespace))

expectedNad := getExpectedHostDeviceNetNAD(testName, ipam)
Expect(nad.Spec).To(BeEquivalentTo(expectedNad.Spec))
})
})
})

func getExpectedHostDeviceNetNAD(testName, ipam string) *netattdefv1.NetworkAttachmentDefinition {
nad := &netattdefv1.NetworkAttachmentDefinition{}
cfg := fmt.Sprintf("{ \"cniVersion\":\"0.3.1\", \"name\":%q, \"type\":\"host-device\", "+
"\"ipam\":%s }",
testName, ipam)
nad.Spec.Config = cfg
return nad
}

func getHostDeviceNetwork(testName, resourceName string) *mellanoxv1alpha1.HostDeviceNetwork {
cr := &mellanoxv1alpha1.HostDeviceNetwork{
Spec: mellanoxv1alpha1.HostDeviceNetworkSpec{
NetworkNamespace: testNamespace,
ResourceName: resourceName,
},
}
cr.Name = testName
return cr
}

0 comments on commit 9df5609

Please sign in to comment.