Skip to content

Commit

Permalink
Do not use log.Fatal() (#116)
Browse files Browse the repository at this point in the history
* Do not use log.Fatal()

Return error when the k8s client cannot be initalized. Setup procedure
failure will exit the DHCP service anyway, do not use `log.Fatal()`

* Remove `log.Fatal()` leftover

* Bring back error context when initializing k8s client

* Fix review findings
  • Loading branch information
damyan authored Apr 8, 2024
1 parent fbeebad commit 13c771a
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 24 deletions.
39 changes: 18 additions & 21 deletions plugins/ipam/k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"encoding/hex"
"encoding/json"
"fmt"
"net"
"os"
"reflect"
Expand Down Expand Up @@ -39,45 +40,47 @@ type K8sClient struct {
EventRecorder record.EventRecorder
}

func NewK8sClient(namespace string, subnetNames []string) K8sClient {
func NewK8sClient(namespace string, subnetNames []string) (*K8sClient, error) {

if err := ipamv1alpha1.AddToScheme(scheme.Scheme); err != nil {
log.Fatal("Unable to add registered types ipam to client scheme: ", err)
return nil, fmt.Errorf("unable to add registered types ipam to client scheme: %w", err)
}

cfg := config.GetConfigOrDie()
cl, err := client.New(cfg, client.Options{})
if err != nil {
log.Fatal("Failed to create controller runtime client: ", err)
return nil, fmt.Errorf("failed to create controller runtime client: %w", err)
}

clientset, err := ipam.NewForConfig(cfg)
if err != nil {
log.Fatal("Failed to create IPAM clientset: ", err)
return nil, fmt.Errorf("failed to create IPAM clientset: %w", err)
}

corev1Client, err := corev1client.NewForConfig(cfg)
if err != nil {
log.Fatal("Failed to create core client: ", err)
return nil, fmt.Errorf("failed to create core client: %w", err)
}

broadcaster := record.NewBroadcaster()

// Leader id, needs to be unique
id, err := os.Hostname()
if err != nil {
log.Fatal("Failed to get hostname: ", err)
return nil, fmt.Errorf("failed to get hostname: %w", err)
}
recorder := broadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: id})
broadcaster.StartRecordingToSink(&corev1client.EventSinkImpl{Interface: corev1Client.Events("")})

return K8sClient{
k8sClient := K8sClient{
Client: cl,
Clientset: *clientset,
Namespace: namespace,
SubnetNames: subnetNames,
Ctx: context.Background(),
EventRecorder: recorder,
}
return &k8sClient, nil
}

func (k K8sClient) createIpamIP(ipaddr net.IP, mac net.HardwareAddr) error {
Expand Down Expand Up @@ -126,8 +129,7 @@ func (k K8sClient) getMatchingSubnet(subnetName string, ipaddr net.IP) (*ipamv1a
existingSubnet := subnet.DeepCopy()
err := k.Client.Get(k.Ctx, client.ObjectKeyFromObject(subnet), existingSubnet)
if err != nil && !apierrors.IsNotFound(err) {
err = errors.Wrapf(err, "Failed to get subnet %s/%s", k.Namespace, subnetName)
return nil, err
return nil, fmt.Errorf("failed to get subnet %s/%s: %w", k.Namespace, subnetName, err)
}
if apierrors.IsNotFound(err) {
log.Debugf("Cannot select subnet %s/%s, does not exist", k.Namespace, subnetName)
Expand All @@ -144,8 +146,7 @@ func (k K8sClient) getMatchingSubnet(subnetName string, ipaddr net.IP) (*ipamv1a
func (k K8sClient) prepareCreateIpamIP(subnetName string, ipaddr net.IP, mac net.HardwareAddr) (*ipamv1alpha1.IP, error) {
ip, err := ipamv1alpha1.IPAddrFromString(ipaddr.String())
if err != nil {
err = errors.Wrapf(err, "Failed to parse IP %s", ipaddr)
return nil, err
return nil, fmt.Errorf("failed to parse IP %s: %w", ipaddr, err)
}

// a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and
Expand Down Expand Up @@ -175,8 +176,7 @@ func (k K8sClient) prepareCreateIpamIP(subnetName string, ipaddr net.IP, mac net
existingIpamIP := ipamIP.DeepCopy()
err = k.Client.Get(k.Ctx, client.ObjectKeyFromObject(ipamIP), existingIpamIP)
if err != nil && !apierrors.IsNotFound(err) {
err = errors.Wrapf(err, "Failed to get IP %s/%s", existingIpamIP.Namespace, existingIpamIP.Name)
return nil, err
return nil, fmt.Errorf("failed to get IP %s/%s: %w", existingIpamIP.Namespace, existingIpamIP.Name, err)
}

// create IPAM IP if not exists or delete existing if ip differs
Expand All @@ -189,14 +189,12 @@ func (k K8sClient) prepareCreateIpamIP(subnetName string, ipaddr net.IP, mac net
// delete old IP object
err = k.Client.Delete(k.Ctx, existingIpamIP)
if err != nil {
err = errors.Wrapf(err, "Failed to delete IP %s/%s", existingIpamIP.Namespace, existingIpamIP.Name)
return nil, err
return nil, fmt.Errorf("failed to delete IP %s/%s: %w", existingIpamIP.Namespace, existingIpamIP.Name, err)
}

err = k.waitForDeletion(existingIpamIP)
if err != nil {
err = errors.Wrapf(err, "Failed to delete IP %s/%s", existingIpamIP.Namespace, existingIpamIP.Name)
return nil, err
return nil, fmt.Errorf("failed to delete IP %s/%s: %w", existingIpamIP.Namespace, existingIpamIP.Name, err)
}

k.EventRecorder.Eventf(existingIpamIP, corev1.EventTypeNormal, "Deleted", "Deleted old IPAM IP")
Expand All @@ -223,7 +221,7 @@ func (k K8sClient) waitForDeletion(ipamIP *ipamv1alpha1.IP) error {
TimeoutSeconds: &timeout,
})
if err != nil {
log.Fatalf("Error watching for IP: %v", err)
log.Errorf("Error watching for IP: %v", err)
}

log.Debugf("Watching for changes to IP %s/%s...", namespace, resourceName)
Expand All @@ -236,14 +234,13 @@ func (k K8sClient) waitForDeletion(ipamIP *ipamv1alpha1.IP) error {
return nil
}
}
return errors.New("Timeout reached, IP not deleted")
return errors.New("timeout reached, IP not deleted")
}

func (k K8sClient) doCreateIpamIP(ipamIP *ipamv1alpha1.IP, subnetName string) error {
err := k.Client.Create(k.Ctx, ipamIP)
if err != nil && !apierrors.IsAlreadyExists(err) {
err = errors.Wrapf(err, "Failed to create IP %s/%s", ipamIP.Namespace, ipamIP.Name)
return err
return fmt.Errorf("failed to create IP %s/%s: %w", ipamIP.Namespace, ipamIP.Name, err)
}
if apierrors.IsAlreadyExists(err) {
// do not create IP, because the deletion is not yet ready
Expand Down
11 changes: 8 additions & 3 deletions plugins/ipam/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ var Plugin = plugins.Plugin{
}

var (
k8sClient K8sClient
k8sClient *K8sClient
)

func parseArgs(args ...string) (string, []string, error) {
Expand All @@ -42,8 +42,13 @@ func setup6(args ...string) (handler.Handler6, error) {
if err != nil {
return nil, err
}
k8sClient = NewK8sClient(namespace, subnetNames)
log.Printf("loaded ipam plugin for DHCPv6.")

k8sClient, err = NewK8sClient(namespace, subnetNames)
if err != nil {
return nil, fmt.Errorf("failed to create k8s client: %w", err)
}

log.Printf("Loaded ipam plugin for DHCPv6.")
return handler6, nil
}

Expand Down

0 comments on commit 13c771a

Please sign in to comment.