Bug Fixes & Performance Improvements (#23)

* refactor: improvements for golangci-lint

* fix: set status to 'Failure' in case of error

Co-authored-by: Artashes Balabekyan <[email protected]>

Author: pogossian

controllers/api_handler.go

@@ -128,16 +128,3 @@ func getSites(names []string) map[string]int {
 	}
 	return siteList
 }
-
-func getTenantID(name string) int {
-	tenants, err := Cred.GetTenants()
-	if err != nil {
-		fmt.Println(err)
-	}
-	for _, tenant := range tenants {
-		if tenant.Name == name {
-			return tenant.ID
-		}
-	}
-	return 0
-}

controllers/storage.go

@@ -52,23 +52,29 @@ func NewStorage() *Storage {
 func (s *Storage) Download() error {
 	s.Lock()
 	defer s.Unlock()
-	err := s.PortsStorage.Download()
-	err = s.SitesStorage.Download()
-	err = s.TenantsStorage.Download()
-	err = s.VNetStorage.Download()
-	return err
+	if err := s.PortsStorage.Download(); err != nil {
+		return err
+	}
+	if err := s.SitesStorage.Download(); err != nil {
+		return err
+	}
+	if err := s.TenantsStorage.Download(); err != nil {
+		return err
+	}
+	if err := s.VNetStorage.Download(); err != nil {
+		return err
+	}
+	return nil
 }
 
 // DownloadWithInterval .
 func (s *Storage) DownloadWithInterval() {
 	ticker := time.NewTicker(10 * time.Second)
 	for {
-		select {
-		case <-ticker.C:
-			err := s.Download()
-			if err != nil {
-				fmt.Println(err)
-			}
+		<-ticker.C
+		err := s.Download()
+		if err != nil {
+			fmt.Println(err)
 		}
 	}
 }

controllers/translations.go

@@ -33,9 +33,7 @@ func (r *VNetReconciler) VnetToVnetMeta(vnet *k8sv1alpha1.VNet) (*k8sv1alpha1.VN
 
 	for _, site := range vnet.Spec.Sites {
 		siteNames = append(siteNames, site.Name)
-		for _, port := range site.SwitchPorts {
-			ports = append(ports, port)
-		}
+		ports = append(ports, site.SwitchPorts...)
 		for _, gateway := range site.Gateways {
 			apiGateways = append(apiGateways, makeGateway(gateway))
 		}
@@ -226,11 +224,7 @@ func compareVNetMetaAPIVnetGateways(vnetMetaGateways []k8sv1alpha1.VNetMetaGatew
 
 	changelog, _ := diff.Diff(vnetGateways, apiGateways)
 
-	if len(changelog) > 0 {
-		return false
-	}
-
-	return true
+	return len(changelog) <= 0
 }
 
 func compareVNetMetaAPIVnetMembers(vnetMetaMembers []k8sv1alpha1.VNetMetaMember, apiVnetMembers []api.APIVNetInfoMember) bool {
@@ -264,22 +258,12 @@ func compareVNetMetaAPIVnetMembers(vnetMetaMembers []k8sv1alpha1.VNetMetaMember,
 	}
 
 	changelog, _ := diff.Diff(vnetMembers, apiMembers)
-
-	if len(changelog) > 0 {
-		return false
-	}
-
-	return true
+	return len(changelog) <= 0
 }
 
 func compareVNetMetaAPIVnetTenants(vnetMetaTenants []int, apiVnetTenants []int) bool {
 	changelog, _ := diff.Diff(vnetMetaTenants, apiVnetTenants)
-
-	if len(changelog) > 0 {
-		return false
-	}
-
-	return true
+	return len(changelog) <= 0
 }
 
 func compareVNetMetaAPIVnetSites(vnetMetaSites []k8sv1alpha1.VNetMetaSite, apiVnetSites []int) bool {

controllers/utils.go

@@ -17,33 +17,12 @@ limitations under the License.
 package controllers
 
 import (
-	"encoding/json"
 	"fmt"
 	"net"
-	"strconv"
-
-	api "github.com/netrisai/netrisapi"
 
 	k8sv1alpha1 "github.com/netrisai/netris-operator/api/v1alpha1"
-	"sigs.k8s.io/controller-runtime/pkg/event"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
 )
 
-func ignoreDeletionPredicate() predicate.Predicate {
-	return predicate.Funcs{
-		UpdateFunc: func(e event.UpdateEvent) bool {
-			fmt.Println("UPDATE EVENT")
-			// Ignore updates to CR status in which case metadata.Generation does not change
-			return e.MetaOld.GetGeneration() != e.MetaNew.GetGeneration()
-		},
-		DeleteFunc: func(e event.DeleteEvent) bool {
-			fmt.Println("DELETE EVENT")
-			// Evaluates to false if the object has been confirmed deleted.
-			return true
-		},
-	}
-}
-
 func makeGateway(gateway k8sv1alpha1.VNetGateway) k8sv1alpha1.VNetMetaGateway {
 	version := ""
 	ip, ipNet, err := net.ParseCIDR(gateway.String())
@@ -67,26 +46,3 @@ func makeGateway(gateway k8sv1alpha1.VNetGateway) k8sv1alpha1.VNetMetaGateway {
 	}
 	return apiGateway
 }
-
-func getVNet(id int) (vnet *api.APIVNet, err error) {
-	vnets, err := Cred.GetVNets()
-	if err != nil {
-		return vnet, err
-	}
-	for _, v := range vnets {
-		vid, err := strconv.Atoi(v.ID)
-		if err != nil {
-			return vnet, err
-		}
-		if vid == id {
-			return v, nil
-		}
-	}
-
-	return vnet, fmt.Errorf("VNet not found in Netris")
-}
-
-func toJSON(s interface{}) string {
-	js, _ := json.Marshal(s)
-	return string(js)
-}

controllers/vnet_controller.go

@@ -82,7 +82,7 @@ func (r *VNetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		_, err := r.deleteVNet(vnet, vnetMeta)
 		if err != nil {
 			logger.Error(fmt.Errorf("{deleteVNet} %s", err), "")
-			return u.patchVNetStatus(vnet, "Netris Failure", err.Error())
+			return u.patchVNetStatus(vnet, "Failure", err.Error())
 		}
 		logger.Info("Vnet deleted")
 		return ctrl.Result{}, nil
@@ -143,6 +143,9 @@ func updateVNet(vnet *api.APIVNetUpdate) (ctrl.Result, error, error) {
 		return ctrl.Result{}, fmt.Errorf("{updateVNet} %s", err), err
 	}
 	resp, err := api.ParseAPIResponse(reply.Data)
+	if err != nil {
+		return ctrl.Result{}, err, err
+	}
 	if !resp.IsSuccess {
 		return ctrl.Result{}, fmt.Errorf("{updateVNet} %s", fmt.Errorf(resp.Message)), fmt.Errorf(resp.Message)
 	}
@@ -152,6 +155,9 @@ func updateVNet(vnet *api.APIVNetUpdate) (ctrl.Result, error, error) {
 		return ctrl.Result{}, fmt.Errorf("{updateVNet} %s", err), err
 	}
 	resp, err = api.ParseAPIResponse(reply.Data)
+	if err != nil {
+		return ctrl.Result{}, err, err
+	}
 	if !resp.IsSuccess {
 		return ctrl.Result{}, fmt.Errorf("{updateVNet} %s", fmt.Errorf(resp.Message)), fmt.Errorf(resp.Message)
 	}
@@ -167,6 +173,9 @@ func (r *VNetReconciler) deleteVNet(vnet *k8sv1alpha1.VNet, vnetMeta *k8sv1alpha
 			return ctrl.Result{}, fmt.Errorf("{deleteVNet} %s", err)
 		}
 		resp, err := api.ParseAPIResponse(reply.Data)
+		if err != nil {
+			return ctrl.Result{}, err
+		}
 		if !resp.IsSuccess {
 			if resp.Message != "Invalid circuit ID" {
 				return ctrl.Result{}, fmt.Errorf("{deleteVNet} %s", fmt.Errorf(resp.Message))

controllers/vnetmeta_controller.go

@@ -83,7 +83,6 @@ func (r *VNetMetaReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 
 	if vnetMeta.Spec.ID == 0 {
 		debugLogger.Info("ID Not found in meta")
-
 		if vnetMeta.Spec.Imported {
 			logger.Info("Importing vnet")
 			debugLogger.Info("Imported yaml mode. Finding VNet by name")
@@ -94,12 +93,6 @@ func (r *VNetMetaReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 					debugLogger.Info(err.Error())
 					return ctrl.Result{RequeueAfter: requeueInterval}, nil
 				}
-				if vnet.Provisioning == 0 {
-					provisionState = "Active"
-				}
-				if vnet.State == "disabled" {
-					provisionState = "Disabled"
-				}
 				vnetMeta.Spec.ID = vnetID
 				err = r.Patch(context.Background(), vnetMeta.DeepCopyObject(), client.Merge, &client.PatchOptions{})
 				if err != nil {
@@ -117,22 +110,22 @@ func (r *VNetMetaReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		logger.Info("Creating VNet")
 		if _, err, errMsg := r.createVNet(vnetMeta); err != nil {
 			logger.Error(fmt.Errorf("{createVNet} %s", err), "")
-			return u.updateVNetStatus(vnetCR, "Netris Failure", errMsg.Error())
+			return u.updateVNetStatus(vnetCR, "Failure", errMsg.Error())
 		}
 		logger.Info("VNet Created")
 	} else {
 		vnets, err := Cred.GetVNetsByID(vnetMeta.Spec.ID)
 		if err != nil {
 			logger.Error(fmt.Errorf("{GetVNetsByID} %s", err), "")
-			return u.updateVNetStatus(vnetCR, "Netris Failure", err.Error())
+			return u.updateVNetStatus(vnetCR, "Failure", err.Error())
 		}
 		if len(vnets) == 0 {
 			debugLogger.Info("VNet not found in Netris")
 			debugLogger.Info("Going to create VNet")
 			logger.Info("Creating VNet")
 			if _, err, errMsg := r.createVNet(vnetMeta); err != nil {
 				logger.Error(fmt.Errorf("{createVNet} %s", err), "")
-				return u.updateVNetStatus(vnetCR, "Netris Failure", errMsg.Error())
+				return u.updateVNetStatus(vnetCR, "Failure", errMsg.Error())
 			}
 			logger.Info("VNet Created")
 		} else {
@@ -158,15 +151,14 @@ func (r *VNetMetaReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 				_, err, errMsg := updateVNet(updateVnet)
 				if err != nil {
 					logger.Error(fmt.Errorf("{updateVNet} %s", err), "")
-					return u.updateVNetStatus(vnetCR, "Netris Error", errMsg.Error())
+					return u.updateVNetStatus(vnetCR, "Failure", errMsg.Error())
 				}
 				logger.Info("VNet Updated")
 			}
 		}
 	}
 
 	return u.updateVNetStatus(vnetCR, provisionState, "Success")
-	// return ctrl.Result{RequeueAfter: requeueInterval}, nil
 }
 
 // SetupWithManager .
@@ -196,7 +188,10 @@ func (r *VNetMetaReconciler) createVNet(vnetMeta *k8sv1alpha1.VNetMeta) (ctrl.Re
 	}
 
 	idStruct := api.APIVNetAddReply{}
-	api.CustomDecode(resp.Data, &idStruct)
+	err = api.CustomDecode(resp.Data, &idStruct)
+	if err != nil {
+		return ctrl.Result{}, err, err
+	}
 
 	debugLogger.Info("VNet Created", "id", idStruct.CircuitID)
 

deploy/charts/netris-operator/Chart.yaml

@@ -15,12 +15,12 @@ type: application
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
 # Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 0.2.5
+version: 0.2.6
 
 # This is the version number of the application being deployed. This version number should be
 # incremented each time you make changes to the application. Versions are not expected to
 # follow Semantic Versioning. They should reflect the version the application is using.
-appVersion: v0.3.3
+appVersion: v0.3.4
 home: https://github.com/netrisai/netris-operator
 icon: https://www.netris.ai/wp-content/uploads/2020/05/logo-600.png  # [todo] Change url to permalink
 keywords: