fix(service): fix error handling bugs during add blade\host (#61)

Found a couple bugs in appliance.AddBlade() and manager.AddHost(). (1) corner case where an empty bladeId is being allowed, when it shouldn't (2) Not enough information is being saved to new blade objects during error handling. This was causing a panic during the delete because the backendOps variable was a nil point being used to call a backend function.
Seagate · Dec 10, 2024 · 609f9ef · 609f9ef
1 parent 7f706a2
commit 609f9ef
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 10 deletions.
diff --git a/pkg/manager/appliance.go b/pkg/manager/appliance.go
@@ -5,6 +5,7 @@ package manager
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/google/uuid"
 	"k8s.io/klog/v2"
@@ -97,12 +98,27 @@ func (a *Appliance) AddBlade(ctx context.Context, c *openapi.Credentials) (*Blad
 		newErr := fmt.Errorf("create session failure at [%s:%d] using interface [%s]: %w", c.IpAddress, c.Port, backendName, err)
 		logger.Error(newErr, "failure: add blade")
 
+		bladeId := c.CustomId
+		if bladeId == "" { // Order CustomeId > BladeSN > UUID
+			bladeId = response.ChassisSN
+			if bladeId == "" {
+				// Generate default id using last N digits of a randomly generated uuid combined with the default prefix
+				// Example uuid: ee0328d9-258a-4e81-976e-b75aa4a2d8f5
+				uuid := uuid.New().String()
+				uuid = strings.ReplaceAll(uuid, "-", "")
+				bladeId = fmt.Sprintf("%s-%s", ID_PREFIX_BLADE_DFLT, uuid[(len(uuid)-common.NumUuidCharsForId):])
+			}
+			c.CustomId = bladeId
+		}
+
 		// Continue adding the failed blade to the datastore, but update the connection status to unavailable
 		newBlade := &Blade{
 			Id:          c.CustomId,
 			Uri:         GetCfmUriBladeId(a.Id, c.CustomId),
 			Status:      common.UNAVAILABLE,
 			ApplianceId: a.Id,
+			backendOps:  ops,
+			creds:       c,
 		}
 		a.Blades[newBlade.Id] = newBlade
 
@@ -171,6 +187,8 @@ func (a *Appliance) AddBlade(ctx context.Context, c *openapi.Credentials) (*Blad
 			Uri:         GetCfmUriBladeId(a.Id, c.CustomId),
 			Status:      common.UNAVAILABLE,
 			ApplianceId: a.Id,
+			backendOps:  ops,
+			creds:       c,
 		}
 		a.Blades[newBlade.Id] = newBlade
 
@@ -295,11 +313,11 @@ func (a *Appliance) DeleteBladeById(ctx context.Context, bladeId string) (*Blade
 
 	blade, err := a.DeleteBladeByIdBackend(ctx, bladeId)
 	if err != nil || blade == nil {
-		logger.V(2).Info("success: delete blade by id after backend session failure", "bladeId", blade.Id, "applianceId", a.Id)
+		logger.V(2).Info("success: delete blade by id after backend session failure", "bladeId", bladeId, "applianceId", a.Id)
 		return blade, err
 	}
 
-	logger.V(2).Info("success: delete blade by id", "bladeId", blade.Id, "applianceId", a.Id)
+	logger.V(2).Info("success: delete blade by id", "bladeId", bladeId, "applianceId", a.Id)
 
 	return blade, nil
 }

diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go
@@ -5,7 +5,9 @@ package manager
 import (
 	"context"
 	"fmt"
+	"strings"
 
+	"github.com/google/uuid"
 	"k8s.io/klog/v2"
 
 	"cfm/pkg/backend"
@@ -281,11 +283,26 @@ func AddHost(ctx context.Context, c *openapi.Credentials) (*Host, error) {
 		newErr := fmt.Errorf("create session failure at [%s:%d] using interface [%s]: %w", c.IpAddress, c.Port, backendName, err)
 		logger.Error(newErr, "failure: add host")
 
+		hostId := c.CustomId
+		if hostId == "" { // Order CustomeId > HostSN > UUID
+			hostId = response.ChassisSN
+			if hostId == "" {
+				// Generate default id using last N digits of the session id combined with the default prefix
+				// Example uuid: ee0328d9-258a-4e81-976e-b75aa4a2d8f5
+				uuid := uuid.New().String()
+				uuid = strings.ReplaceAll(uuid, "-", "")
+				hostId = fmt.Sprintf("%s-%s", ID_PREFIX_HOST_DFLT, uuid[(len(uuid)-common.NumUuidCharsForId):])
+			}
+			c.CustomId = hostId
+		}
+
 		// Continue adding the failed host to the datastore, but update the connection status to unavailable
 		host := &Host{
-			Id:     c.CustomId,
-			Uri:    GetCfmUriHostId(c.CustomId),
-			Status: common.UNAVAILABLE,
+			Id:         c.CustomId,
+			Uri:        GetCfmUriHostId(c.CustomId),
+			Status:     common.UNAVAILABLE,
+			backendOps: ops,
+			creds:      c,
 		}
 		deviceCache.AddHost(host, false)
 
@@ -348,9 +365,11 @@ func AddHost(ctx context.Context, c *openapi.Credentials) (*Host, error) {
 
 		// Continue adding the failed host to the datastore, but update the connection status to unavailable
 		host := &Host{
-			Id:     c.CustomId,
-			Uri:    GetCfmUriHostId(c.CustomId),
-			Status: common.UNAVAILABLE,
+			Id:         c.CustomId,
+			Uri:        GetCfmUriHostId(c.CustomId),
+			Status:     common.UNAVAILABLE,
+			backendOps: ops,
+			creds:      c,
 		}
 		deviceCache.AddHost(host, false)
 
@@ -487,11 +506,11 @@ func DeleteHostById(ctx context.Context, hostId string) (*Host, error) {
 
 	host, err := DeleteHostByIdBackend(ctx, hostId)
 	if err != nil || host == nil {
-		logger.V(2).Info("success: delete host by id after backend session failure", "hostId", host.Id)
+		logger.V(2).Info("success: delete host by id after backend session failure", "hostId", hostId)
 		return host, err
 	}
 
-	logger.V(2).Info("success: delete host by id", "hostId", host.Id)
+	logger.V(2).Info("success: delete host by id", "hostId", hostId)
 
 	return host, nil
 }