From 5456391827e551d2a756bc3f87a573dac8b7e9b7 Mon Sep 17 00:00:00 2001 From: rabi Date: Wed, 11 Sep 2024 10:05:17 +0530 Subject: [PATCH] WIP Add IsCtlPlane attribute to network We don't have to hardcode the ctlplane network in dataplane operator. Signed-off-by: rabi --- apis/bases/network.openstack.org_ipsets.yaml | 4 +++ .../network.openstack.org_netconfigs.yaml | 4 +++ apis/network/v1beta1/common_webhook.go | 1 + apis/network/v1beta1/ipset_types.go | 3 +++ apis/network/v1beta1/netconfig_types.go | 4 +++ apis/network/v1beta1/netconfig_webhook.go | 10 ++++++-- .../bases/network.openstack.org_ipsets.yaml | 4 +++ .../network.openstack.org_netconfigs.yaml | 4 +++ config/samples/network_v1beta1_netconfig.yaml | 1 + controllers/network/ipset_controller.go | 25 ++++++++++++------- tests/functional/base_test.go | 13 +++++----- tests/functional/ipset_controller_test.go | 25 +++++++++++++------ 12 files changed, 74 insertions(+), 24 deletions(-) diff --git a/apis/bases/network.openstack.org_ipsets.yaml b/apis/bases/network.openstack.org_ipsets.yaml index 503c946f..fb3de61d 100644 --- a/apis/bases/network.openstack.org_ipsets.yaml +++ b/apis/bases/network.openstack.org_ipsets.yaml @@ -156,6 +156,9 @@ spec: gateway: description: Gateway optional gateway for the network type: string + isCtlPlane: + description: Whether it is ctlplane network + type: boolean mtu: description: MTU of the network type: integer @@ -190,6 +193,7 @@ spec: required: - address - dnsDomain + - isCtlPlane - network - subnet type: object diff --git a/apis/bases/network.openstack.org_netconfigs.yaml b/apis/bases/network.openstack.org_netconfigs.yaml index 4d43d8c5..346ebe09 100644 --- a/apis/bases/network.openstack.org_netconfigs.yaml +++ b/apis/bases/network.openstack.org_netconfigs.yaml @@ -46,6 +46,9 @@ spec: dnsDomain: description: DNSDomain name of the Network type: string + isCtlPlane: + description: Whether this network is control plane network + type: boolean mtu: default: 1500 description: MTU of the network @@ -128,6 +131,7 @@ spec: type: array required: - dnsDomain + - isCtlPlane - name - subnets type: object diff --git a/apis/network/v1beta1/common_webhook.go b/apis/network/v1beta1/common_webhook.go index 1d2730cd..382c4209 100644 --- a/apis/network/v1beta1/common_webhook.go +++ b/apis/network/v1beta1/common_webhook.go @@ -35,6 +35,7 @@ const ( errDefaultRouteChanged = "defaultRoute must not change" errMultiDefaultRoute = "%s defaultRoute can only be requested on a singe network" errNoDefaultRoute = "defaultRoute requested, but not configured for subnet %s" + errMultiControlPlane = "ctlPlaneNetwork can only be on a singe network" ) func getNetConfig( diff --git a/apis/network/v1beta1/ipset_types.go b/apis/network/v1beta1/ipset_types.go index 15ade7fa..b6883a31 100644 --- a/apis/network/v1beta1/ipset_types.go +++ b/apis/network/v1beta1/ipset_types.go @@ -82,6 +82,9 @@ type IPSetReservation struct { // DNSDomain of the subnet DNSDomain string `json:"dnsDomain"` + + // Whether it is ctlplane network + IsCtlPlane bool `json:"isCtlPlane"` } // IPSetStatus defines the observed state of IPSet diff --git a/apis/network/v1beta1/netconfig_types.go b/apis/network/v1beta1/netconfig_types.go index 41697ad0..f118daab 100644 --- a/apis/network/v1beta1/netconfig_types.go +++ b/apis/network/v1beta1/netconfig_types.go @@ -46,6 +46,10 @@ type Network struct { // +kubebuilder:validation:Required // Subnets of the network Subnets []Subnet `json:"subnets"` + + // +kubebuilder:validation:optional + // Whether this network is control plane network + IsCtlPlane bool `json:"isCtlPlane"` } // Subnet definition diff --git a/apis/network/v1beta1/netconfig_webhook.go b/apis/network/v1beta1/netconfig_webhook.go index 7371fffa..731678f6 100644 --- a/apis/network/v1beta1/netconfig_webhook.go +++ b/apis/network/v1beta1/netconfig_webhook.go @@ -146,7 +146,7 @@ func valiateNetworks( allErrs := field.ErrorList{} netNames := map[string]field.Path{} netCIDR := map[string]field.Path{} - + ctlPlaneCount := 0 for netIdx, _net := range networks { path := path.Child("networks").Index(netIdx) @@ -171,8 +171,14 @@ func valiateNetworks( allErrs = append(allErrs, err...) } } - } + if _net.IsCtlPlane { + ctlPlaneCount++ + } + if ctlPlaneCount > 1 { + allErrs = append(allErrs, field.Invalid(path.Child("IsCtlPlane"), _net.IsCtlPlane, errMultiControlPlane)) + } + } return allErrs } diff --git a/config/crd/bases/network.openstack.org_ipsets.yaml b/config/crd/bases/network.openstack.org_ipsets.yaml index 503c946f..fb3de61d 100644 --- a/config/crd/bases/network.openstack.org_ipsets.yaml +++ b/config/crd/bases/network.openstack.org_ipsets.yaml @@ -156,6 +156,9 @@ spec: gateway: description: Gateway optional gateway for the network type: string + isCtlPlane: + description: Whether it is ctlplane network + type: boolean mtu: description: MTU of the network type: integer @@ -190,6 +193,7 @@ spec: required: - address - dnsDomain + - isCtlPlane - network - subnet type: object diff --git a/config/crd/bases/network.openstack.org_netconfigs.yaml b/config/crd/bases/network.openstack.org_netconfigs.yaml index 4d43d8c5..346ebe09 100644 --- a/config/crd/bases/network.openstack.org_netconfigs.yaml +++ b/config/crd/bases/network.openstack.org_netconfigs.yaml @@ -46,6 +46,9 @@ spec: dnsDomain: description: DNSDomain name of the Network type: string + isCtlPlane: + description: Whether this network is control plane network + type: boolean mtu: default: 1500 description: MTU of the network @@ -128,6 +131,7 @@ spec: type: array required: - dnsDomain + - isCtlPlane - name - subnets type: object diff --git a/config/samples/network_v1beta1_netconfig.yaml b/config/samples/network_v1beta1_netconfig.yaml index d23f9342..3e173169 100644 --- a/config/samples/network_v1beta1_netconfig.yaml +++ b/config/samples/network_v1beta1_netconfig.yaml @@ -15,6 +15,7 @@ spec: start: 192.168.122.150 cidr: 192.168.122.0/24 gateway: 192.168.122.1 + isCtlPlane: true - name: internalapi dnsDomain: internalapi.example.com subnets: diff --git a/controllers/network/ipset_controller.go b/controllers/network/ipset_controller.go index d621a568..1c594085 100644 --- a/controllers/network/ipset_controller.go +++ b/controllers/network/ipset_controller.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "net" + "sort" corev1 "k8s.io/api/core/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" @@ -274,6 +275,11 @@ func (r *IPSetReconciler) reconcileNormal(ctx context.Context, instance *network return ctrl.Result{}, err } + // sort instance.Status.Reservations by Network + sort.Slice(instance.Status.Reservation, func(i, j int) bool { + return instance.Status.Reservation[i].IsCtlPlane && !instance.Status.Reservation[j].IsCtlPlane + }) + instance.Status.Conditions.MarkTrue(networkv1.ReservationReadyCondition, networkv1.ReservationReadyMessage) Log.Info("IPSet is ready:", "instance", instance.Name, "ipSetRes", ipSetRes.Spec.Reservation) @@ -426,15 +432,16 @@ func (r *IPSetReconciler) ensureReservation( // add IP to the reservation and IPSet status reservations reservationSpec.Reservation[string(netDef.Name)] = *ip ipsetRes := networkv1.IPSetReservation{ - Network: netDef.Name, - Subnet: subnetDef.Name, - Address: ip.Address, - MTU: netDef.MTU, - Cidr: subnetDef.Cidr, - Vlan: subnetDef.Vlan, - Gateway: subnetDef.Gateway, - Routes: subnetDef.Routes, - DNSDomain: netDef.DNSDomain, + Network: netDef.Name, + Subnet: subnetDef.Name, + Address: ip.Address, + MTU: netDef.MTU, + Cidr: subnetDef.Cidr, + Vlan: subnetDef.Vlan, + Gateway: subnetDef.Gateway, + Routes: subnetDef.Routes, + DNSDomain: netDef.DNSDomain, + IsCtlPlane: netDef.IsCtlPlane, } if ipsetNet.DefaultRoute != nil && *ipsetNet.DefaultRoute && subnetDef.Gateway != nil { ipsetRes.Gateway = subnetDef.Gateway diff --git a/tests/functional/base_test.go b/tests/functional/base_test.go index b6d6b811..4ed7702c 100644 --- a/tests/functional/base_test.go +++ b/tests/functional/base_test.go @@ -469,12 +469,13 @@ func GetNetConfigSpec(nets ...networkv1.Network) map[string]interface{} { return spec } -func GetNetSpec(name string, subnets ...networkv1.Subnet) networkv1.Network { +func GetNetSpec(name string, isCtlPlane bool, subnets ...networkv1.Subnet) networkv1.Network { net := networkv1.Network{ - Name: networkv1.NetNameStr(name), - DNSDomain: fmt.Sprintf("%s.example.com", strings.ToLower(name)), - MTU: 1400, - Subnets: []networkv1.Subnet{}, + Name: networkv1.NetNameStr(name), + DNSDomain: fmt.Sprintf("%s.example.com", strings.ToLower(name)), + MTU: 1400, + Subnets: []networkv1.Subnet{}, + IsCtlPlane: isCtlPlane, } net.Subnets = append(net.Subnets, subnets...) @@ -483,7 +484,7 @@ func GetNetSpec(name string, subnets ...networkv1.Subnet) networkv1.Network { } func GetDefaultNetConfigSpec() map[string]interface{} { - net := GetNetSpec(net1, GetSubnet1(subnet1)) + net := GetNetSpec(net1, false, GetSubnet1(subnet1)) return GetNetConfigSpec(net) } diff --git a/tests/functional/ipset_controller_test.go b/tests/functional/ipset_controller_test.go index ead7f066..451e56fb 100644 --- a/tests/functional/ipset_controller_test.go +++ b/tests/functional/ipset_controller_test.go @@ -280,7 +280,7 @@ var _ = Describe("IPSet controller", func() { When("a GetDefaultIPSetSpec IPSet gets created using a custom NetConfig", func() { BeforeEach(func() { - netSpec := GetNetSpec(net1, GetSubnet1(subnet1)) + netSpec := GetNetSpec(net1, false, GetSubnet1(subnet1)) netSpec.Subnets[0].DNSDomain = ptr.To("subnet1.net-1.example.com") netCfg := CreateNetConfig(namespace, GetNetConfigSpec(netSpec)) ipset := CreateIPSet(namespace, GetDefaultIPSetSpec()) @@ -319,9 +319,9 @@ var _ = Describe("IPSet controller", func() { var ipSetNetworks []networkv1.IPSetNetwork BeforeEach(func() { netSpecs = []networkv1.Network{} - netSpecs = append(netSpecs, GetNetSpec(net1, GetSubnet1(subnet1))) - netSpecs = append(netSpecs, GetNetSpec(net2, GetSubnet2(subnet1))) - netSpecs = append(netSpecs, GetNetSpec(net3, GetSubnet3(subnet1))) + netSpecs = append(netSpecs, GetNetSpec(net1, false, GetSubnet1(subnet1))) + netSpecs = append(netSpecs, GetNetSpec(net2, true, GetSubnet2(subnet1))) + netSpecs = append(netSpecs, GetNetSpec(net3, false, GetSubnet3(subnet1))) ipSetNetworks = []networkv1.IPSetNetwork{} ipSetNetworks = append(ipSetNetworks, GetIPSetNet1Lower()) @@ -369,11 +369,22 @@ var _ = Describe("IPSet controller", func() { Eventually(func(g Gomega) { g.Expect(k8sClient.Get(ctx, ipSetName, instance)).Should(Succeed()) + ctlplaneNetwork := instance.Status.Reservation[0].Network + ctlplaneFound := false for i := 0; i < len(ipSetNetworks); i++ { // first assert that the instance networks are in the same order as we specified g.Expect(instance.Spec.Networks[i].Name).To(Equal(ipSetNetworks[i].Name)) // then assert that the reservation networks are in the same order - g.Expect(instance.Spec.Networks[i].Name).To(Equal(instance.Status.Reservation[i].Network)) + // other than ctlplane network being the first one. + if ipSetNetworks[i].Name == ctlplaneNetwork { + ctlplaneFound = true + continue + } + if ctlplaneFound { + g.Expect(instance.Spec.Networks[i].Name).To(Equal(instance.Status.Reservation[i].Network)) + } else { + g.Expect(instance.Spec.Networks[i].Name).To(Equal(instance.Status.Reservation[i+1].Network)) + } } g.Expect(k8sClient.Update(ctx, instance)).Should(Succeed()) }, timeout, interval).Should(Succeed()) @@ -382,8 +393,8 @@ var _ = Describe("IPSet controller", func() { When("an IPSet with Immutable flag gets created", func() { BeforeEach(func() { - net1Spec := GetNetSpec(net1, GetSubnet1(subnet1)) - net2Spec := GetNetSpec(net2, GetSubnet2(subnet1)) + net1Spec := GetNetSpec(net1, false, GetSubnet1(subnet1)) + net2Spec := GetNetSpec(net2, false, GetSubnet2(subnet1)) netCfg := CreateNetConfig(namespace, GetNetConfigSpec(net1Spec, net2Spec)) netCfgName.Name = netCfg.GetName() netCfgName.Namespace = netCfg.GetNamespace()