Skip to content

Commit

Permalink
Improve logging in route controller (#467)
Browse files Browse the repository at this point in the history 
Improve logging in route controller. Addionally ensure that `envtest` is
using the correct Kubernetes binaries.
  • Loading branch information
@afritzler
afritzler authored Nov 22, 2024
1 parent e60cdfe commit acaee4d
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 60 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ BIN_NAME = "cloud-provider-ironcore"
IMG ?= controller:latest

# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
ENVTEST_K8S_VERSION = 1.30.3
ENVTEST_K8S_VERSION = 1.30.0

ifeq (,$(shell go env GOBIN))
GOBIN=$(shell go env GOPATH)/bin
Expand Down
79 changes: 44 additions & 35 deletions pkg/cloudprovider/ironcore/routes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,61 +36,64 @@ func newIroncoreRoutes(targetClient client.Client, ironcoreClient client.Client,
}

func (o ironcoreRoutes) ListRoutes(ctx context.Context, clusterName string) ([]*cloudprovider.Route, error) {
klog.V(2).InfoS("List Routes", "Cluster", clusterName)
klog.V(2).InfoS("Listing routes for cluster", "Cluster", clusterName)

// Retrieve network interfaces matching the given namespace, network, and cluster label
networkInterfaces := &networkingv1alpha1.NetworkInterfaceList{}
if err := o.ironcoreClient.List(ctx, networkInterfaces, client.InNamespace(o.ironcoreNamespace), client.MatchingFields{
networkInterfaceSpecNetworkRefNameField: o.cloudConfig.NetworkName,
}, client.MatchingLabels{
LabelKeyClusterName: clusterName,
}); err != nil {
return nil, err
return nil, fmt.Errorf("failed to list network interfaces for cluster %s: %w", clusterName, err)
}

var routes []*cloudprovider.Route
// iterate over all network interfaces and collect all the prefixes as Routes
// Iterate over each network interface and compile route information based on its prefixes
for _, nic := range networkInterfaces.Items {
// Verify that the network interface is associated with a machine reference
if nic.Spec.MachineRef != nil && nic.Spec.MachineRef.Name != "" {
for _, prefix := range nic.Status.Prefixes {
destinationCIDR := prefix.String()
// Retrieve node addresses based on the machine reference name
targetNodeAddresses, err := o.getTargetNodeAddresses(ctx, nic.Spec.MachineRef.Name)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get target node addresses for machine %s: %w", nic.Spec.MachineRef.Name, err)
}
route := &cloudprovider.Route{
Name: clusterName + "-" + destinationCIDR,
DestinationCIDR: destinationCIDR,
TargetNode: types.NodeName(nic.Spec.MachineRef.Name),
TargetNodeAddresses: targetNodeAddresses,
}

routes = append(routes, route)
}
}
}

klog.V(2).InfoS("Current Routes", "Cluster", clusterName, "Network", o.cloudConfig.NetworkName, "Routes", routes)
klog.V(2).InfoS("Route listing completed", "Cluster", clusterName, "Network", o.cloudConfig.NetworkName, "RouteCount", len(routes))
return routes, nil
}

func (o ironcoreRoutes) CreateRoute(ctx context.Context, clusterName string, nameHint string, route *cloudprovider.Route) error {
klog.V(2).InfoS("Creating Route", "Cluster", clusterName, "Route", route, "NameHint", nameHint)
klog.V(2).InfoS("Initiating CreateRoute operation", "Cluster", clusterName, "Route", route, "NameHint", nameHint)

// get the machine object based on the node name
// Retrieve the machine object corresponding to the node name
nodeName := string(route.TargetNode)
machine := &computev1alpha1.Machine{}
if err := o.ironcoreClient.Get(ctx, client.ObjectKey{Namespace: o.ironcoreNamespace, Name: nodeName}, machine); err != nil {
if apierrors.IsNotFound(err) {
klog.V(2).InfoS("Machine object for node not found, returning InstanceNotFound", "Node", nodeName)
return cloudprovider.InstanceNotFound
}
return fmt.Errorf("failed to get machine object for node %s: %w", nodeName, err)
}
klog.V(3).InfoS("Machine object retrieval completed", "Node", nodeName)

// loop over all node addresses and find based on internal IP the matching network interface
// Iterate over all target node addresses and identify the matching network interface using internal IPs
for _, address := range route.TargetNodeAddresses {
// only use internal IP addresses
if address.Type == corev1.NodeInternalIP {
// loop over all network interfaces of the machine
klog.V(3).InfoS("Evaluating internal IP address", "Node", nodeName, "Address", address.Address)
for _, networkInterface := range machine.Status.NetworkInterfaces {
interfaceFound := false
for _, p := range networkInterface.IPs {
Expand All @@ -100,16 +103,18 @@ func (o ironcoreRoutes) CreateRoute(ctx context.Context, clusterName string, nam
}
}

// if the interface is found, add the prefix to the network interface
// If a matching network interface is found, proceed with prefix operations
if interfaceFound {
// get the network interface object
klog.V(3).InfoS("Matching network interface identified", "Node", nodeName, "Address", address.Address)
networkInterfaceName := getNetworkInterfaceName(machine, networkInterface)
nic := &networkingv1alpha1.NetworkInterface{}
if err := o.ironcoreClient.Get(ctx, client.ObjectKey{Namespace: o.ironcoreNamespace, Name: networkInterfaceName}, nic); err != nil {
return err
}

// check if the prefix is already added
klog.V(3).InfoS("NetworkInterface retrieval completed", "NetworkInterface", networkInterfaceName)

// Check if the prefix already exists in the network interface
prefixExists := false
for _, prefix := range nic.Status.Prefixes {
if prefix.Prefix.String() == route.DestinationCIDR {
Expand All @@ -118,50 +123,53 @@ func (o ironcoreRoutes) CreateRoute(ctx context.Context, clusterName string, nam
}
}

// if the prefix is not added, add it
// If the prefix is not found, add it to the network interface specification
if !prefixExists {
klog.V(3).InfoS("Prefix not detected, proceeding with addition", "NetworkInterface", networkInterfaceName, "Prefix", route.DestinationCIDR)
nicBase := nic.DeepCopy()
ipPrefix := commonv1alpha1.MustParseIPPrefix(route.DestinationCIDR)
prefixSource := networkingv1alpha1.PrefixSource{
Value: &ipPrefix,
}
nic.Spec.Prefixes = append(nic.Spec.Prefixes, prefixSource)

klog.V(2).InfoS("Updating NetworkInterface by adding prefix", "NetworkInterface", client.ObjectKeyFromObject(nic), "Node", nodeName, "Prefix", route.DestinationCIDR)
klog.V(2).InfoS("Applying patch to NetworkInterface to incorporate new prefix", "NetworkInterface", networkInterfaceName, "Node", nodeName, "Prefix", route.DestinationCIDR)
if err := o.ironcoreClient.Patch(ctx, nic, client.MergeFrom(nicBase)); err != nil {
return fmt.Errorf("failed to patch NetworkInterface %s for Node %s: %w", client.ObjectKeyFromObject(nic), nodeName, err)
return fmt.Errorf("failed to patch NetworkInterface %s for Node %s: %w", networkInterfaceName, nodeName, err)
}
klog.V(2).InfoS("Patch applied to NetworkInterface", "NetworkInterface", networkInterfaceName, "Prefix", route.DestinationCIDR)
} else {
klog.V(2).InfoS("NetworkInterface prefix already exists", "NetworkInterface", client.ObjectKeyFromObject(nic), "Node", nodeName, "Prefix", route.DestinationCIDR)
klog.V(3).InfoS("Prefix already present on NetworkInterface", "NetworkInterface", networkInterfaceName, "Prefix", route.DestinationCIDR)
}
} else {
klog.V(4).InfoS("No matching network interface identified for IP address", "Node", nodeName, "Address", address.Address)
}
}
} else {
klog.V(4).InfoS("Ignoring non-internal IP address", "Node", nodeName, "AddressType", address.Type, "Address", address.Address)
}
}

klog.V(2).InfoS("Created Route", "Cluster", clusterName, "Route", route, "NameHint", nameHint)

klog.V(2).InfoS("CreateRoute operation completed", "Cluster", clusterName, "Route", route, "NameHint", nameHint)
return nil
}

func (o ironcoreRoutes) DeleteRoute(ctx context.Context, clusterName string, route *cloudprovider.Route) error {
klog.V(2).InfoS("Deleting Route", "Cluster", clusterName, "Route", route)
klog.V(2).InfoS("Initiating route deletion", "Cluster", clusterName, "Route", route)

// get the machine object based on the node name
// Retrieve the machine object based on the node name
nodeName := string(route.TargetNode)
machine := &computev1alpha1.Machine{}
if err := o.ironcoreClient.Get(ctx, client.ObjectKey{Namespace: o.ironcoreNamespace, Name: nodeName}, machine); err != nil {
if apierrors.IsNotFound(err) {
return cloudprovider.InstanceNotFound
}
return fmt.Errorf("failed to get machine object for node %s: %w", nodeName, err)
return fmt.Errorf("failed to retrieve machine object for node %s: %w", nodeName, err)
}

// loop over all node addresses and find based on internal IP the matching network interface
// Iterate over all target node addresses and find matching network interfaces by internal IP
for _, address := range route.TargetNodeAddresses {
// only use internal IP addresses
if address.Type == corev1.NodeInternalIP {
// loop over all network interfaces of the machine
klog.V(3).InfoS("Evaluating internal IP address for network interface match", "Node", nodeName, "Address", address.Address)
for _, networkInterface := range machine.Status.NetworkInterfaces {
interfaceFound := false
for _, p := range networkInterface.IPs {
Expand All @@ -171,35 +179,36 @@ func (o ironcoreRoutes) DeleteRoute(ctx context.Context, clusterName string, rou
}
}

// if the interface is found, add the prefix to the network interface
// If a matching network interface is found, attempt to remove the prefix
if interfaceFound {
// get the network interface object
networkInterfaceName := getNetworkInterfaceName(machine, networkInterface)
nic := &networkingv1alpha1.NetworkInterface{}
if err := o.ironcoreClient.Get(ctx, client.ObjectKey{Namespace: o.ironcoreNamespace, Name: networkInterfaceName}, nic); err != nil {
return err
}

// check if the prefix exists
for i, prefix := range nic.Status.Prefixes {
if prefix.Prefix.String() == route.DestinationCIDR {
// Check for the prefix in the network interface's spec and remove it if present
for i, prefix := range nic.Spec.Prefixes {
if prefix.Value.String() == route.DestinationCIDR {
nicBase := nic.DeepCopy()
nic.Spec.Prefixes = append(nic.Spec.Prefixes[:i], nic.Spec.Prefixes[i+1:]...)
klog.V(2).InfoS("Prefix found and removed", "Prefix", prefix.Prefix.String(), "Prefixes after", nic.Spec.Prefixes)
klog.V(2).InfoS("Prefix identified and marked for removal", "Prefix", prefix.Value.String(), "UpdatedPrefixes", nic.Spec.Prefixes)

if err := o.ironcoreClient.Patch(ctx, nic, client.MergeFrom(nicBase)); err != nil {
return fmt.Errorf("failed to patch NetworkInterface %s for Node %s: %w", client.ObjectKeyFromObject(nic), nodeName, err)
}

klog.V(2).InfoS("Prefix removal patch applied", "NetworkInterface", networkInterfaceName, "Node", nodeName)
break
}
}
}
}
} else {
klog.V(4).InfoS("Ignoring non-internal IP address", "Node", nodeName, "AddressType", address.Type, "Address", address.Address)
}
}
klog.V(2).InfoS("Deleted Route", "Cluster", clusterName, "Route", route)

klog.V(2).InfoS("Route deletion completed", "Cluster", clusterName, "Route", route)
return nil
}

Expand Down
29 changes: 6 additions & 23 deletions pkg/cloudprovider/ironcore/routes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,7 @@ var _ = Describe("Routes", func() {
Expect(k8sClient.Patch(ctx, machine, client.MergeFrom(machineBase))).To(Succeed())

By("getting list of routes")
var routes []*cloudprovider.Route
Expect(routesProvider.ListRoutes(ctx, clusterName)).Should(Equal(routes))
Expect(routesProvider.ListRoutes(ctx, clusterName)).Should(BeEmpty())
})

It("should ensure that a prefix has been created for a route", func(ctx SpecContext) {
Expand Down Expand Up @@ -376,14 +375,6 @@ var _ = Describe("Routes", func() {
},
},
},
{
Name: "ephemeral",
NetworkInterfaceSource: computev1alpha1.NetworkInterfaceSource{
NetworkInterfaceRef: &corev1.LocalObjectReference{
Name: fmt.Sprintf("%s-%s", machine.Name, "ephemeral"),
},
},
},
}
machine.Status.State = computev1alpha1.MachineStateRunning
machine.Status.NetworkInterfaces = []computev1alpha1.NetworkInterfaceStatus{
Expand All @@ -410,6 +401,7 @@ var _ = Describe("Routes", func() {
},
},
})).To(Succeed())

Eventually(Object(staticNetworkInterface)).Should(SatisfyAll(
HaveField("Spec.Prefixes", ContainElement(networkingv1alpha1.PrefixSource{
Value: commonv1alpha1.MustParseNewIPPrefix("100.0.0.1/24"),
Expand All @@ -428,22 +420,13 @@ var _ = Describe("Routes", func() {
},
},
})).To(Succeed())

Eventually(Object(ephemeralNetworkInterface)).Should(SatisfyAll(
HaveField("Spec.Prefixes", ContainElement(networkingv1alpha1.PrefixSource{
Value: commonv1alpha1.MustParseNewIPPrefix("192.168.0.1/32"),
})),
))

By("patching the static network interface status to have the correct prefix in status")
staticNetworkInterfaceBase = staticNetworkInterface.DeepCopy()
staticNetworkInterface.Status.Prefixes = []commonv1alpha1.IPPrefix{commonv1alpha1.MustParseIPPrefix("100.0.0.1/24")}
Expect(k8sClient.Status().Patch(ctx, staticNetworkInterface, client.MergeFrom(staticNetworkInterfaceBase))).To(Succeed())

By("patching the ephemeral network interface status to have the correct prefix in status")
ephemeralNetworkInterfaceBase = staticNetworkInterface.DeepCopy()
ephemeralNetworkInterface.Status.Prefixes = []commonv1alpha1.IPPrefix{commonv1alpha1.MustParseIPPrefix("192.168.0.1/32")}
Expect(k8sClient.Status().Patch(ctx, ephemeralNetworkInterface, client.MergeFrom(ephemeralNetworkInterfaceBase))).To(Succeed())

By("deleting prefix for static route")
Expect(routesProvider.DeleteRoute(ctx, clusterName, &cloudprovider.Route{
Name: "foo",
Expand All @@ -457,9 +440,8 @@ var _ = Describe("Routes", func() {
},
})).To(Succeed())

var prefixSources []networkingv1alpha1.PrefixSource
Eventually(Object(staticNetworkInterface)).Should(SatisfyAll(
HaveField("Spec.Prefixes", Equal(prefixSources)),
HaveField("Spec.Prefixes", BeEmpty()),
))

By("deleting prefix for ephemeral route")
Expand All @@ -474,8 +456,9 @@ var _ = Describe("Routes", func() {
},
},
})).To(Succeed())

Eventually(Object(ephemeralNetworkInterface)).Should(SatisfyAll(
HaveField("Spec.Prefixes", Equal(prefixSources)),
HaveField("Spec.Prefixes", BeEmpty()),
))
})
})
13 changes: 12 additions & 1 deletion pkg/cloudprovider/ironcore/suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ package ironcore

import (
"context"
"fmt"
"os"
"path/filepath"
"runtime"
"testing"
"time"

Expand Down Expand Up @@ -70,7 +73,15 @@ var _ = BeforeSuite(func() {
var err error

By("bootstrapping test environment")
testEnv = &envtest.Environment{}
testEnv = &envtest.Environment{
// The BinaryAssetsDirectory is only required if you want to run the tests directly
// without call the makefile target test. If not informed it will look for the
// default path defined in controller-runtime which is /usr/local/kubebuilder/.
// Note that you must have the required binaries setup under the bin directory to perform
// the tests directly. When we run make test it will be setup and used automatically.
BinaryAssetsDirectory: filepath.Join("..", "..", "..", "bin", "k8s",
fmt.Sprintf("1.30.0-%s-%s", runtime.GOOS, runtime.GOARCH)),
}
testEnvExt = &envtestext.EnvironmentExtensions{
APIServiceDirectoryPaths: []string{
modutils.Dir("github.com/ironcore-dev/ironcore", "config", "apiserver", "apiservice", "bases"),
Expand Down

0 comments on commit acaee4d

Please sign in to comment.