From fcf34c009a7b21169e5471bfffee484e60167858 Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Mon, 3 Jul 2023 15:08:59 +0300 Subject: [PATCH] Create the webhook validations for externallyCreated variable Signed-off-by: Sebastian Sch --- pkg/webhook/validate.go | 38 +++++ pkg/webhook/validate_test.go | 312 +++++++++++++++++++++++++++++++++++ 2 files changed, 350 insertions(+) diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go index 01f753608..151080fa6 100644 --- a/pkg/webhook/validate.go +++ b/pkg/webhook/validate.go @@ -165,6 +165,13 @@ func staticValidateSriovNetworkNodePolicy(cr *sriovnetworkv1.SriovNetworkNodePol if cr.Spec.VdpaType == constants.VdpaTypeVirtio && cr.Spec.EswitchMode != sriovnetworkv1.ESwithModeSwitchDev { return false, fmt.Errorf("virtio/vdpa requires the device to be configured in switchdev mode") } + + // Externally created: we don't support ExternallyCreated + EswitchMode + //TODO: if needed we will need to add this in the future as today EswitchMode is for HWOFFLOAD + if cr.Spec.ExternallyCreated && cr.Spec.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev { + return false, fmt.Errorf("ExternallyCreated doesn't support the device to be configured in switchdev mode") + } + return true, nil } @@ -240,6 +247,21 @@ func validatePolicyForNodeState(policy *sriovnetworkv1.SriovNetworkNodePolicy, s if policy.Spec.NumVfs > MlxMaxVFs && iface.Vendor == MellanoxID { return fmt.Errorf("numVfs(%d) in CR %s exceed the maximum allowed value(%d)", policy.Spec.NumVfs, policy.GetName(), MlxMaxVFs) } + + // Externally create validations + if policy.Spec.ExternallyCreated { + if policy.Spec.NumVfs != iface.NumVfs { + return fmt.Errorf("numVfs(%d) in CR %s is not equal to the number of virtual functions allocated for the PF externally value(%d)", policy.Spec.NumVfs, policy.GetName(), iface.NumVfs) + } + + if policy.Spec.Mtu != 0 && policy.Spec.Mtu != iface.Mtu { + return fmt.Errorf("MTU(%d) in CR %s is not equal to the MTU for the PF externally value(%d)", policy.Spec.Mtu, policy.GetName(), iface.Mtu) + } + + if policy.Spec.LinkType != "" && strings.ToLower(policy.Spec.LinkType) != strings.ToLower(iface.LinkType) { + return fmt.Errorf("LinkType(%d) in CR %s is not equal to the LinkType for the PF externally value(%d)", policy.Spec.Mtu, policy.GetName(), iface.Mtu) + } + } // vdpa: only mellanox cards are supported if policy.Spec.VdpaType == constants.VdpaTypeVirtio && iface.Vendor != MellanoxID { return fmt.Errorf("vendor(%s) in CR %s not supported for virtio-vdpa", iface.Vendor, policy.GetName()) @@ -281,6 +303,22 @@ func validatePfNames(current *sriovnetworkv1.SriovNetworkNodePolicy, previous *s // since it should already be evaluated in previous run. preName, preRngSt, preRngEnd, _ := sriovnetworkv1.ParsePFName(prePf) if curName == preName { + // reject policy with externallyManage if there is a policy on the same PF without it + if current.Spec.ExternallyCreated != previous.Spec.ExternallyCreated { + return fmt.Errorf("externallyManage is inconsistent with existing policy %s", previous.GetName()) + } + + // reject policy with externallyManage if there is a policy on the same PF with switch dev + if current.Spec.ExternallyCreated && previous.Spec.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev { + return fmt.Errorf("externallyManage overlap with switchdev mode in existing policy %s", previous.GetName()) + } + + // reject policy with externallyManage if there is a policy on the same PF with switch dev + if previous.Spec.ExternallyCreated && current.Spec.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev { + return fmt.Errorf("switchdev overlap with externallyManage mode in existing policy %s", previous.GetName()) + } + + // Check for overlapping ranges if curRngEnd < preRngSt || curRngSt > preRngEnd { return nil } else { diff --git a/pkg/webhook/validate_test.go b/pkg/webhook/validate_test.go index 028d62a5e..904f6377c 100644 --- a/pkg/webhook/validate_test.go +++ b/pkg/webhook/validate_test.go @@ -71,6 +71,7 @@ func newNodeState() *SriovNetworkNodeState { Vendor: "8086", NumVfs: 4, TotalVfs: 64, + LinkType: "ETH", }, { VFs: []VirtualFunction{ @@ -84,6 +85,7 @@ func newNodeState() *SriovNetworkNodeState { Vendor: "8086", NumVfs: 4, TotalVfs: 64, + LinkType: "ETH", }, { VFs: []VirtualFunction{ @@ -97,6 +99,7 @@ func newNodeState() *SriovNetworkNodeState { Vendor: "8086", NumVfs: 4, TotalVfs: 64, + LinkType: "ETH", }, }, }, @@ -243,6 +246,291 @@ func TestValidatePolicyForNodeStateWithInvalidNumVfsPolicy(t *testing.T) { g.Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("numVfs(%d) in CR %s exceed the maximum allowed value(%d)", policy.Spec.NumVfs, policy.GetName(), state.Status.Interfaces[0].TotalVfs)))) } +func TestValidatePolicyForNodeStateWithInvalidNumVfsExternallyCreated(t *testing.T) { + state := newNodeState() + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f0"}, + RootDevices: []string{"0000:86:00.0"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 5, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("numVfs(%d) in CR %s is not equal to the number of virtual functions allocated for the PF externally value(%d)", policy.Spec.NumVfs, policy.GetName(), state.Status.Interfaces[0].NumVfs)))) +} + +func TestValidatePolicyForNodeStateWithValidNumVfsExternallyCreated(t *testing.T) { + state := newNodeState() + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f0"}, + RootDevices: []string{"0000:86:00.0"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 4, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).ToNot(HaveOccurred()) +} + +func TestValidatePolicyForNodePolicyWithOutExternallyManageConflict(t *testing.T) { + appliedPolicy := newNodePolicy() + appliedPolicy.Spec.ExternallyCreated = true + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p0", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f1#3-4"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 63, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodePolicy(policy, appliedPolicy) + g.Expect(err).ToNot(HaveOccurred()) +} + +func TestValidatePolicyForNodePolicyWithExternallyManageConflict(t *testing.T) { + appliedPolicy := newNodePolicy() + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p0", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f1#3-4"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 63, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodePolicy(policy, appliedPolicy) + g.Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("externallyManage is inconsistent with existing policy %s", appliedPolicy.ObjectMeta.Name)))) +} + +func TestValidatePolicyForNodePolicyWithExternallyManageConflictWithSwitchDev(t *testing.T) { + appliedPolicy := newNodePolicy() + appliedPolicy.Spec.EswitchMode = ESwithModeSwitchDev + + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p0", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f1#3-4"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 63, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodePolicy(policy, appliedPolicy) + g.Expect(err).To(HaveOccurred()) +} + +func TestValidatePolicyForNodePolicyWithSwitchDevConflictWithExternallyManage(t *testing.T) { + appliedPolicy := newNodePolicy() + appliedPolicy.Spec.ExternallyCreated = true + + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p0", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f1#3-4"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 63, + Priority: 99, + ResourceName: "p0", + EswitchMode: ESwithModeSwitchDev, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodePolicy(policy, appliedPolicy) + g.Expect(err).To(HaveOccurred()) +} + +func TestValidatePolicyForNodeStateWithExternallyManageAndMTU(t *testing.T) { + state := newNodeState() + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f0"}, + RootDevices: []string{"0000:86:00.0"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 4, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + Mtu: 1500, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).ToNot(HaveOccurred()) +} + +func TestValidatePolicyForNodeStateWithExternallyManageAndDifferentMTU(t *testing.T) { + state := newNodeState() + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f0"}, + RootDevices: []string{"0000:86:00.0"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 4, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + Mtu: 9000, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).To(HaveOccurred()) +} + +func TestValidatePolicyForNodeStateWithExternallyManageAndLinkType(t *testing.T) { + state := newNodeState() + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f0"}, + RootDevices: []string{"0000:86:00.0"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 4, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + LinkType: "ETH", + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).ToNot(HaveOccurred()) + + policy.Spec.LinkType = "eth" + err = validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).ToNot(HaveOccurred()) + + policy.Spec.LinkType = "ETH" + state.Status.Interfaces[0].LinkType = "eth" + err = validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).ToNot(HaveOccurred()) +} + +func TestValidatePolicyForNodeStateWithExternallyManageAndDifferentLinkType(t *testing.T) { + state := newNodeState() + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f0"}, + RootDevices: []string{"0000:86:00.0"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 4, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + Mtu: 9000, + LinkType: "IB", + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).To(HaveOccurred()) +} + func TestValidatePolicyForNodePolicyWithOverlappedVfRange(t *testing.T) { appliedPolicy := newNodePolicy() policy := &SriovNetworkNodePolicy{ @@ -545,6 +833,30 @@ func TestStaticValidateSriovNetworkNodePolicyVdpaMustSpecifySwitchDev(t *testing g.Expect(ok).To(Equal(false)) } +func TestStaticValidateSriovNetworkNodePolicyWithExternallyCreatedAndSwitchDev(t *testing.T) { + policy := &SriovNetworkNodePolicy{ + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + Vendor: "8086", + DeviceID: "158b", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 63, + Priority: 99, + ResourceName: "p0", + EswitchMode: "switchdev", + ExternallyCreated: true, + }, + } + g := NewGomegaWithT(t) + ok, err := staticValidateSriovNetworkNodePolicy(policy) + g.Expect(err).To(HaveOccurred()) + g.Expect(ok).To(BeFalse()) +} + func TestValidatePolicyForNodeStateVdpaWithNotSupportedVendor(t *testing.T) { state := newNodeState() policy := &SriovNetworkNodePolicy{