Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ServiceNetwork attribute to networks in netconfig #271

Merged
merged 1 commit into from
Sep 16, 2024
Merged
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Dockerfile.cross

bundle
bundle.Dockerfile
config/manager/kustomization.yaml

# Test binary, build with `go test -c`
*.test
Expand Down
5 changes: 5 additions & 0 deletions apis/bases/network.openstack.org_ipsets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,10 @@ spec:
- nexthop
type: object
type: array
serviceNetwork:
description: ServiceNetwork mapping
pattern: ^[a-z0-9][a-z0-9\-_]*[a-z0-9]$
type: string
subnet:
description: Subnet name
pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$
Expand All @@ -191,6 +195,7 @@ spec:
- address
- dnsDomain
- network
- serviceNetwork
- subnet
type: object
type: array
Expand Down
4 changes: 4 additions & 0 deletions apis/bases/network.openstack.org_netconfigs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ spec:
...
pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$
type: string
serviceNetwork:
description: Service network mapping
pattern: ^[a-z0-9][a-z0-9\-_]*[a-z0-9]$
type: string
subnets:
description: Subnets of the network
items:
Expand Down
3 changes: 3 additions & 0 deletions apis/network/v1beta1/ipset_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ type IPSetReservation struct {

// DNSDomain of the subnet
DNSDomain string `json:"dnsDomain"`

// ServiceNetwork mapping
ServiceNetwork ServiceNetNameStr `json:"serviceNetwork"`
}

// IPSetStatus defines the observed state of IPSet
Expand Down
11 changes: 11 additions & 0 deletions apis/network/v1beta1/netconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ import (
// NetNameStr is used for validation of a net name.
type NetNameStr string

// +kubebuilder:validation:Pattern="^[a-z0-9][a-z0-9\\-_]*[a-z0-9]$"
type ServiceNetNameStr string

func ToDefaultServiceNetwork(n NetNameStr) ServiceNetNameStr {
return ServiceNetNameStr(strings.ToLower(string(n)))
}

// Network definition
type Network struct {
// +kubebuilder:validation:Required
Expand All @@ -46,6 +53,10 @@ type Network struct {
// +kubebuilder:validation:Required
// Subnets of the network
Subnets []Subnet `json:"subnets"`

// +kubebuilder:validation:optional
// Service network mapping
ServiceNetwork ServiceNetNameStr `json:"serviceNetwork,omitempty"`
}

// Subnet definition
Expand Down
9 changes: 5 additions & 4 deletions apis/network/v1beta1/netconfig_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,12 @@ var _ webhook.Defaulter = &NetConfig{}

// Default implements webhook.Defaulter so a webhook will be registered for the type
func (r *NetConfig) Default() {
netconfiglog.Info("default", "name", r.Name)
for _, net := range r.Spec.Networks {
if net.ServiceNetwork == "" {
net.ServiceNetwork = ToDefaultServiceNetwork(net.Name)
}
}

// TODO(user): fill in your defaulting logic.
}

//+kubebuilder:webhook:path=/validate-network-openstack-org-v1beta1-netconfig,mutating=false,failurePolicy=fail,sideEffects=None,groups=network.openstack.org,resources=netconfigs,verbs=create;update;delete,versions=v1beta1,name=vnetconfig.kb.io,admissionReviewVersions=v1
Expand Down Expand Up @@ -146,7 +149,6 @@ func valiateNetworks(
allErrs := field.ErrorList{}
netNames := map[string]field.Path{}
netCIDR := map[string]field.Path{}

for netIdx, _net := range networks {
path := path.Child("networks").Index(netIdx)

Expand All @@ -172,7 +174,6 @@ func valiateNetworks(
}
}
}

return allErrs
}

Expand Down
5 changes: 5 additions & 0 deletions config/crd/bases/network.openstack.org_ipsets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,10 @@ spec:
- nexthop
type: object
type: array
serviceNetwork:
description: ServiceNetwork mapping
pattern: ^[a-z0-9][a-z0-9\-_]*[a-z0-9]$
type: string
subnet:
description: Subnet name
pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$
Expand All @@ -191,6 +195,7 @@ spec:
- address
- dnsDomain
- network
- serviceNetwork
- subnet
type: object
type: array
Expand Down
4 changes: 4 additions & 0 deletions config/crd/bases/network.openstack.org_netconfigs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ spec:
...
pattern: ^[a-zA-Z0-9][a-zA-Z0-9\-_]*[a-zA-Z0-9]$
type: string
serviceNetwork:
description: Service network mapping
pattern: ^[a-z0-9][a-z0-9\-_]*[a-z0-9]$
type: string
subnets:
description: Subnets of the network
items:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ spec:
kind: DNSMasq
name: dnsmasqs.network.openstack.org
version: v1beta1
- description: InstanceHA is the Schema for the instancehas API
displayName: Instance HA
kind: InstanceHA
name: instancehas.instanceha.openstack.org
version: v1beta1
- description: IPSet is the Schema for the ipsets API
displayName: IPSet
kind: IPSet
Expand All @@ -51,6 +56,10 @@ spec:
displayName: Redis
kind: Redis
name: redises.redis.openstack.org
specDescriptors:
- description: TLS settings for Redis service and internal Redis replication
displayName: TLS
path: tls
version: v1beta1
- description: Reservation is the Schema for the reservations API
displayName: Reservation
Expand Down
34 changes: 25 additions & 9 deletions controllers/network/ipset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"context"
"fmt"
"net"
"sort"
"strings"

corev1 "k8s.io/api/core/v1"
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -43,6 +45,8 @@ import (
util "github.com/openstack-k8s-operators/lib-common/modules/common/util"
)

const CtlPlaneNetwork = "ctlplane"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I don't think this needs to be Heap allocated right? Looks like we're only using this const in the reconcileNormal function, so the const can be stack allocated in that function.

Unless you were planning to use it externally somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

const is a compile time construct in go, so there is no question heap/stack memory allocation for them at run time. They're logically replaced in code at compile time.


// IPSetReconciler reconciles a IPSet object
type IPSetReconciler struct {
client.Client
Expand Down Expand Up @@ -274,6 +278,13 @@ func (r *IPSetReconciler) reconcileNormal(ctx context.Context, instance *network
return ctrl.Result{}, err
}

// sort instance.Status.Reservations
sort.Slice(instance.Status.Reservation, func(i, j int) bool {
return (strings.EqualFold(string(instance.Status.Reservation[i].ServiceNetwork),
CtlPlaneNetwork) && !strings.EqualFold(string(instance.Status.Reservation[j].ServiceNetwork),
CtlPlaneNetwork))
})

instance.Status.Conditions.MarkTrue(networkv1.ReservationReadyCondition, networkv1.ReservationReadyMessage)

Log.Info("IPSet is ready:", "instance", instance.Name, "ipSetRes", ipSetRes.Spec.Reservation)
Expand Down Expand Up @@ -398,6 +409,10 @@ func (r *IPSetReconciler) ensureReservation(
return nil, err
}

if netDef.ServiceNetwork == "" {
netDef.ServiceNetwork = networkv1.ToDefaultServiceNetwork(netDef.Name)
}

// set net: subnet label
reservationLabels = util.MergeStringMaps(reservationLabels,
map[string]string{
Expand Down Expand Up @@ -426,15 +441,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,
ServiceNetwork: netDef.ServiceNetwork,
}
if ipsetNet.DefaultRoute != nil && *ipsetNet.DefaultRoute && subnetDef.Gateway != nil {
ipsetRes.Gateway = subnetDef.Gateway
Expand Down
14 changes: 14 additions & 0 deletions tests/functional/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,20 @@ func GetNetSpec(name string, subnets ...networkv1.Subnet) networkv1.Network {
return net
}

func GetNetCtlplaneSpec(name string, 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{},
ServiceNetwork: "ctlplane",
}

net.Subnets = append(net.Subnets, subnets...)

return net
}

func GetDefaultNetConfigSpec() map[string]interface{} {
net := GetNetSpec(net1, GetSubnet1(subnet1))
return GetNetConfigSpec(net)
Expand Down
16 changes: 14 additions & 2 deletions tests/functional/ipset_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ var _ = Describe("IPSet controller", func() {
BeforeEach(func() {
netSpecs = []networkv1.Network{}
netSpecs = append(netSpecs, GetNetSpec(net1, GetSubnet1(subnet1)))
netSpecs = append(netSpecs, GetNetSpec(net2, GetSubnet2(subnet1)))
netSpecs = append(netSpecs, GetNetCtlplaneSpec(net2, GetSubnet2(subnet1)))
netSpecs = append(netSpecs, GetNetSpec(net3, GetSubnet3(subnet1)))

ipSetNetworks = []networkv1.IPSetNetwork{}
Expand Down Expand Up @@ -369,11 +369,23 @@ var _ = Describe("IPSet controller", func() {

Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(ctx, ipSetName, instance)).Should(Succeed())
ctlplaneNetwork := instance.Status.Reservation[0].Network
g.Expect(instance.Status.Reservation[0].ServiceNetwork).To(Equal(networkv1.ServiceNetNameStr("ctlplane")))
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))
stuggi marked this conversation as resolved.
Show resolved Hide resolved
} 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())
Expand Down
Loading