Merge pull request #177 from sunya-ch/v1.2.2

add context timeout and connection close
foundation-model-stack · Jan 23, 2024 · 0fb93a2 · 0fb93a2
2 parents f217e2e + d1fa502
commit 0fb93a2
Show file tree

Hide file tree

Showing 15 changed files with 420 additions and 94 deletions.
diff --git a/cni/plugins/ipam/multi-nic-ipam/ipam.go b/cni/plugins/ipam/multi-nic-ipam/ipam.go
@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"time"
 
 	"bytes"
 	"errors"
@@ -58,23 +59,27 @@ func RequestIP(daemonIP string, daemonPort int, podName string, podNamespace str
 	jsonReq, err := json.Marshal(request)
 
 	if err != nil {
-		return response, errors.New(fmt.Sprintf("Marshal fail: %v", err))
+		return response, fmt.Errorf("marshal fail: %v", err)
 	} else {
-		res, err := http.Post(address, "application/json; charset=utf-8", bytes.NewBuffer(jsonReq))
+		client := http.Client{
+			Timeout: 2 * time.Minute,
+		}
+		defer client.CloseIdleConnections()
+		res, err := client.Post(address, "application/json; charset=utf-8", bytes.NewBuffer(jsonReq))
 		if err != nil {
-			return response, errors.New(fmt.Sprintf("Post fail: %v", err))
+			return response, fmt.Errorf("post fail: %v", err)
 		}
+		defer res.Body.Close()
 		if res.StatusCode != http.StatusOK {
 			return response, errors.New(res.Status)
 		}
-
 		body, err := ioutil.ReadAll(res.Body)
 		if err != nil {
-			return response, errors.New(fmt.Sprintf("Read body: %v", err))
+			return response, fmt.Errorf("read body: %v", err)
 		}
 		err = json.Unmarshal(body, &response)
 		if err == nil && len(response) == 0 {
-			return response, fmt.Errorf("Response nothing")
+			return response, fmt.Errorf("response nothing")
 		}
 		return response, err
 	}
@@ -97,23 +102,28 @@ func Deallocate(daemonPort int, podName string, podNamespace string, hostName st
 	jsonReq, err := json.Marshal(request)
 
 	if err != nil {
-		return response, errors.New(fmt.Sprintf("Marshal fail: %v", err))
+		return response, fmt.Errorf("marshal fail: %v", err)
 	} else {
-		res, err := http.Post(address, "application/json; charset=utf-8", bytes.NewBuffer(jsonReq))
+		client := http.Client{
+			Timeout: 2 * time.Minute,
+		}
+		defer client.CloseIdleConnections()
+		res, err := client.Post(address, "application/json; charset=utf-8", bytes.NewBuffer(jsonReq))
 		if err != nil {
-			return response, errors.New(fmt.Sprintf("Post fail: %v", err))
+			return response, fmt.Errorf("post fail: %v", err)
 		}
+		defer res.Body.Close()
 		if res.StatusCode != http.StatusOK {
 			return response, errors.New(res.Status)
 		}
 
 		body, err := ioutil.ReadAll(res.Body)
 		if err != nil {
-			return response, errors.New(fmt.Sprintf("Read body: %v", err))
+			return response, fmt.Errorf("read body: %v", err)
 		}
 		err = json.Unmarshal(body, &response)
 		if err == nil && len(response) == 0 {
-			return response, fmt.Errorf("Response nothing")
+			return response, fmt.Errorf("response nothing")
 		}
 		return response, err
 	}

diff --git a/cni/plugins/main/multi-nic/selector.go b/cni/plugins/main/multi-nic/selector.go
@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"time"
 
 	"bytes"
 	"errors"
@@ -56,23 +57,28 @@ func selectNICs(daemonIP string, daemonPort int, podName string, podNamespace st
 	jsonReq, err := json.Marshal(request)
 
 	if err != nil {
-		return response, errors.New(fmt.Sprintf("Marshal fail: %v", err))
+		return response, fmt.Errorf("marshal fail: %v", err)
 	} else {
-		res, err := http.Post(address, "application/json; charset=utf-8", bytes.NewBuffer(jsonReq))
+		client := http.Client{
+			Timeout: 5 * time.Minute,
+		}
+		defer client.CloseIdleConnections()
+		res, err := client.Post(address, "application/json; charset=utf-8", bytes.NewBuffer(jsonReq))
 		if err != nil {
-			return response, errors.New(fmt.Sprintf("Post fail: %v", err))
+			return response, fmt.Errorf("post fail: %v", err)
 		}
+		defer res.Body.Close()
 		if res.StatusCode != http.StatusOK {
 			return response, errors.New(res.Status)
 		}
 
 		body, err := ioutil.ReadAll(res.Body)
 		if err != nil {
-			return response, errors.New(fmt.Sprintf("Read body: %v", err))
+			return response, fmt.Errorf("read body: %v", err)
 		}
 		err = json.Unmarshal(body, &response)
 		if err == nil && len(response.Masters) == 0 {
-			return response, fmt.Errorf("Response nothing")
+			return response, fmt.Errorf("response nothing")
 		}
 		return response, err
 	}

diff --git a/controllers/cidr_handler.go b/controllers/cidr_handler.go
@@ -201,7 +201,9 @@ func (h *CIDRHandler) DeleteCIDR(cidr multinicv1.CIDR) error {
 	}
 	instance, err := h.GetCIDR(name)
 	if err == nil {
-		err = h.Client.Delete(context.Background(), instance)
+		ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
+		defer cancel()
+		err = h.Client.Delete(ctx, instance)
 	}
 	if err != nil {
 		errorMsg = errorMsg + fmt.Sprintf("%v,", err)
@@ -525,14 +527,18 @@ func (h *CIDRHandler) updateCIDR(cidrSpec multinicv1.CIDRSpec, new bool) (bool,
 		if err == nil {
 			updatedCIDR := existCIDR.DeepCopy()
 			updatedCIDR.Spec = spec
-			err = h.Client.Update(context.TODO(), updatedCIDR)
+			ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
+			defer cancel()
+			err = h.Client.Update(ctx, updatedCIDR)
 			if err == nil {
 				h.SafeCache.SetCache(def.Name, updatedCIDR.Spec)
 			}
 			h.CleanPendingIPPools(ippoolSnapshot, def.Name, updatedCIDR.Spec)
 		} else {
 			if new {
-				err = h.Client.Create(context.TODO(), mapObj)
+				ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
+				defer cancel()
+				err = h.Client.Create(ctx, mapObj)
 				if err == nil {
 					h.SafeCache.SetCache(def.Name, mapObj.Spec)
 				}

diff --git a/controllers/daemon_connector.go b/controllers/daemon_connector.go
@@ -10,7 +10,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
-	"time"
 
 	"bytes"
 	"errors"
@@ -73,11 +72,13 @@ func (dc DaemonConnector) GetInterfaces(podAddress string) ([]multinicv1.Interfa
 	var interfaces []multinicv1.InterfaceInfoType
 	address := podAddress + INTERFACE_PATH
 	// try connect and get interface from daemon pod
-	res, err := http.Get(address)
+	client := http.Client{}
+	defer client.CloseIdleConnections()
+	res, err := client.Get(address)
 	if err != nil {
 		return []multinicv1.InterfaceInfoType{}, err
 	}
-
+	defer res.Body.Close()
 	body, err := ioutil.ReadAll(res.Body)
 	if err != nil {
 		return []multinicv1.InterfaceInfoType{}, err
@@ -100,12 +101,13 @@ func (dc DaemonConnector) Join(podAddress string, hifs []multinicv1.InterfaceInf
 		return err
 	} else {
 		client := http.Client{
-			Timeout: 5 * time.Second,
+			Timeout: vars.ContextTimeout,
 		}
 		res, err := client.Post(address, "application/json; charset=utf-8", bytes.NewBuffer(jsonReq))
 		if err != nil {
 			return err
 		}
+		defer res.Body.Close()
 		if res.StatusCode != http.StatusOK {
 			return errors.New(res.Status)
 		}
@@ -145,7 +147,15 @@ func (dc DaemonConnector) putRouteRequest(podAddress string, path string, cidrNa
 	if err != nil {
 		return response, err
 	} else {
-		res, err := http.Post(address, "application/json; charset=utf-8", bytes.NewBuffer(jsonReq))
+		client := http.Client{
+			Timeout: vars.ContextTimeout,
+		}
+		defer client.CloseIdleConnections()
+		res, err := client.Post(address, "application/json; charset=utf-8", bytes.NewBuffer(jsonReq))
+		if err != nil {
+			return response, fmt.Errorf("post fail: %v", err)
+		}
+		defer res.Body.Close()
 		if err != nil {
 			response.Message = vars.ConnectionRefusedError
 			return response, err

diff --git a/controllers/hostinterface_handler.go b/controllers/hostinterface_handler.go
@@ -61,7 +61,9 @@ func (h *HostInterfaceHandler) initHostInterface(hostName string, interfaces []m
 // CreateHostInterface creates new HostInterface from an interface list get from daemon pods
 func (h *HostInterfaceHandler) CreateHostInterface(hostName string, interfaces []multinicv1.InterfaceInfoType) error {
 	newHif := h.initHostInterface(hostName, interfaces)
-	return h.Client.Create(context.TODO(), newHif)
+	ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
+	defer cancel()
+	return h.Client.Create(ctx, newHif)
 }
 
 // UpdateHostInterface updates HostInterface
@@ -73,7 +75,9 @@ func (h *HostInterfaceHandler) UpdateHostInterface(oldObj multinicv1.HostInterfa
 			Interfaces: interfaces,
 		},
 	}
-	return updateHif, h.Client.Update(context.TODO(), updateHif)
+	ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
+	defer cancel()
+	return updateHif, h.Client.Update(ctx, updateHif)
 }
 
 // GetHostInterface gets HostInterface from hostname
@@ -83,7 +87,9 @@ func (h *HostInterfaceHandler) GetHostInterface(name string) (*multinicv1.HostIn
 		Name:      name,
 		Namespace: metav1.NamespaceAll,
 	}
-	err := h.Client.Get(context.TODO(), namespacedName, instance)
+	ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
+	defer cancel()
+	err := h.Client.Get(ctx, namespacedName, instance)
 	return instance, err
 }
 
@@ -107,7 +113,9 @@ func (h *HostInterfaceHandler) ListHostInterface() (map[string]multinicv1.HostIn
 func (h *HostInterfaceHandler) DeleteHostInterface(name string) error {
 	instance, err := h.GetHostInterface(name)
 	if err == nil {
-		err = h.Client.Delete(context.TODO(), instance)
+		ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
+		defer cancel()
+		err = h.Client.Delete(ctx, instance)
 	}
 	return err
 }

diff --git a/controllers/ippool_handler.go b/controllers/ippool_handler.go
@@ -69,7 +69,9 @@ func (h *IPPoolHandler) DeleteIPPool(netAttachDef string, podCIDR string) error
 	name := h.GetIPPoolName(netAttachDef, podCIDR)
 	instance, err := h.GetIPPool(name)
 	if err == nil {
-		err = h.Client.Delete(context.TODO(), instance)
+		ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
+		defer cancel()
+		err = h.Client.Delete(ctx, instance)
 	}
 	return err
 }
@@ -132,7 +134,9 @@ func (h *IPPoolHandler) UpdateIPPool(netAttachDef string, podCIDR string, vlanCI
 		ippool.Spec = spec
 		ippool.Spec.Allocations = prevSpec.Allocations
 		ippool.ObjectMeta.Labels = labels
-		err = h.Client.Update(context.TODO(), ippool)
+		ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
+		defer cancel()
+		err = h.Client.Update(ctx, ippool)
 		if !reflect.DeepEqual(prevSpec.Excludes, excludesInterface) {
 			// report if allocated ip addresses have conflicts with the new IPPool (for example, in exclude list)
 			invalidAllocations := h.checkPoolValidity(excludesInterface, prevSpec.Allocations)
@@ -154,7 +158,9 @@ func (h *IPPoolHandler) UpdateIPPool(netAttachDef string, podCIDR string, vlanCI
 			},
 			Spec: spec,
 		}
-		err = h.Client.Create(context.Background(), newIPPool)
+		ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
+		defer cancel()
+		err = h.Client.Create(ctx, newIPPool)
 		vars.IPPoolLog.V(5).Info(fmt.Sprintf("New IPPool %s: %v, %v", ippoolName, newIPPool, err))
 	}
 	return err
@@ -192,7 +198,9 @@ func (h *IPPoolHandler) PatchIPPoolAllocations(ippoolName string, newAllocations
 	}
 	patch := client.MergeFrom(ippool.DeepCopy())
 	ippool.Spec.Allocations = newAllocations
-	return h.Client.Patch(context.Background(), ippool, patch)
+	ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
+	defer cancel()
+	return h.Client.Patch(ctx, ippool, patch)
 }
 
 func (h *IPPoolHandler) UpdateIPPools(defName string, entries []multinicv1.CIDREntry, excludes []compute.IPValue) {
@@ -234,6 +242,8 @@ func (h *IPPoolHandler) AddLabel(ippool *multinicv1.IPPool) error {
 	labels := map[string]string{vars.HostNameLabel: hostName, vars.DefNameLabel: netAttachDef}
 	patch := client.MergeFrom(ippool.DeepCopy())
 	ippool.ObjectMeta.Labels = labels
-	err := h.Client.Patch(context.Background(), ippool, patch)
+	ctx, cancel := context.WithTimeout(context.Background(), vars.ContextTimeout)
+	defer cancel()
+	err := h.Client.Patch(ctx, ippool, patch)
 	return err
 }
diff --git a/controllers/multinicnetwork_controller.go b/controllers/multinicnetwork_controller.go
@@ -184,14 +184,18 @@ func (r *MultiNicNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 				vars.NetworkLog.V(3).Info(fmt.Sprintf("CIDR %s successfully applied", multinicnetworkName))
 			}
 		}
-	} else if !instance.Spec.IsMultiNICIPAM && routeStatus == multinicv1.RouteNoApplied {
+	} else if !instance.Spec.IsMultiNICIPAM && (routeStatus == "" || routeStatus == multinicv1.RouteNoApplied) {
 		// not related to L3
+		instance.Status.Message = ""
+		instance.Status.RouteStatus = multinicv1.RouteNoApplied
+		vars.NetworkLog.V(3).Info(fmt.Sprintf("Update %s status (Non-MultiNICIPAM, RouteNoApplied)", multinicnetworkName))
 		err = r.CIDRHandler.MultiNicNetworkHandler.UpdateNetConfigStatus(instance, multinicv1.ConfigComplete, "")
 		if err != nil {
 			vars.NetworkLog.V(3).Info(fmt.Sprintf("Failed to UpdateNetConfigStatus %s for non-L3: %v", instance.Name, err))
 		}
 	} else if routeStatus != multinicv1.AllRouteApplied {
 		// some route still fails
+		vars.NetworkLog.V(3).Info(fmt.Sprintf("Update %s status (waiting for route configuration)", multinicnetworkName))
 		err = r.CIDRHandler.MultiNicNetworkHandler.UpdateNetConfigStatus(instance, multinicv1.WaitForConfig, "")
 		if err != nil {
 			vars.NetworkLog.V(3).Info(fmt.Sprintf("Failed to UpdateNetConfigStatus %s at route failure: %v", instance.Name, err))
@@ -234,12 +238,16 @@ func (r *MultiNicNetworkReconciler) GetIPAMConfig(instance *multinicv1.MultiNicN
 		ipamConfig.MasterNetAddrs = instance.Spec.MasterNetAddrs
 		return ipamConfig, nil
 	}
-	return nil, fmt.Errorf("non-MultiNicIPAM")
+	vars.NetworkLog.V(3).Info("non-MultiNicIPAM")
+	return nil, nil
 }
 
 // HandleMultiNicIPAM handles ipam if target type
 func (r *MultiNicNetworkReconciler) HandleMultiNicIPAM(instance *multinicv1.MultiNicNetwork) error {
 	ipamConfig, err := r.GetIPAMConfig(instance)
+	if ipamConfig == nil {
+		return err
+	}
 	if err == nil {
 		cidrName := instance.GetName()
 		_, err := r.CIDRHandler.GetCIDR(cidrName)