fix: push datastore updates down into manager layer

This simplifies manager and datastore syncronization of "status" state.
Seagate · Sep 20, 2024 · df366ed · df366ed
1 parent 6e01adb
commit df366ed
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 94 deletions.
diff --git a/pkg/api/api_default_service.go b/pkg/api/api_default_service.go
@@ -751,36 +751,31 @@ func (cfm *CfmApiService) HostsComposeMemory(ctx context.Context, hostId string,
 
 // HostsDeleteById - Guarenteed host deletion from service.
 func (cfm *CfmApiService) HostsDeleteById(ctx context.Context, hostId string) (openapi.ImplResponse, error) {
+	var h openapi.Host
+
 	host, err := manager.DeleteHostById(ctx, hostId)
 	if err != nil {
-		// Force delete on failure since, currently, manager always removes host from deviceCache, even on failure
-		datastore.DStore().GetDataStore().DeleteHost(hostId)
-		datastore.DStore().Store()
-
-		h := openapi.Host{Id: host.Id}
+		h = openapi.Host{Id: hostId}
 
-		return openapi.Response(http.StatusOK, h), nil
-	}
-
-	datastore.DStore().GetDataStore().DeleteHost(host.Id)
-	datastore.DStore().Store()
+	} else {
 
-	h := openapi.Host{
-		Id:        host.Id,
-		IpAddress: host.GetNetIp(),
-		Port:      int32(host.GetNetPort()),
-		Status:    string(host.Status),
-		Ports: openapi.MemberItem{
-			Uri: manager.GetCfmUriHostPorts(host.Id),
-		},
-		Memory: openapi.MemberItem{
-			Uri: manager.GetCfmUriHostMemory(host.Id),
-		},
-		MemoryDevices: openapi.MemberItem{
-			Uri: manager.GetCfmUriHostMemoryDevices(host.Id),
-		},
-		LocalMemoryMiB:  -1, // Not implemented
-		RemoteMemoryMiB: -1, // Not implemented
+		h = openapi.Host{
+			Id:        host.Id,
+			IpAddress: host.GetNetIp(),
+			Port:      int32(host.GetNetPort()),
+			Status:    string(host.Status),
+			Ports: openapi.MemberItem{
+				Uri: manager.GetCfmUriHostPorts(host.Id),
+			},
+			Memory: openapi.MemberItem{
+				Uri: manager.GetCfmUriHostMemory(host.Id),
+			},
+			MemoryDevices: openapi.MemberItem{
+				Uri: manager.GetCfmUriHostMemoryDevices(host.Id),
+			},
+			LocalMemoryMiB:  -1, // Not implemented
+			RemoteMemoryMiB: -1, // Not implemented
+		}
 	}
 
 	return openapi.Response(http.StatusOK, h), nil
@@ -822,41 +817,15 @@ func (cfm *CfmApiService) HostsGet(ctx context.Context) (openapi.ImplResponse, e
 }
 
 // HostsGetById - Get information for a single CXL Host.
-// Note: Do NOT add\delete to\from the datastore here.
 func (cfm *CfmApiService) HostsGetById(ctx context.Context, hostId string) (openapi.ImplResponse, error) {
 	host, err := manager.GetHostById(ctx, hostId)
 	if err != nil || host == nil {
-
-		// If host get buy id fails AND host in datastore, set it offline
-		req := datastore.UpdateHostRequest{
-			HostId: hostId,
-			Status: common.OFFLINE,
-		}
-		datastore.DStore().GetDataStore().UpdateHost(ctx, &req)
-		datastore.DStore().Store()
-
 		return formatErrorResp(ctx, err.(*common.RequestError))
 	}
 
-	// Update datastore status
-	req := datastore.UpdateHostRequest{
-		HostId: host.Id,
-		Status: common.ConnectionStatus(host.Status),
-	}
-	datastore.DStore().GetDataStore().UpdateHost(ctx, &req)
-	datastore.DStore().Store()
-
-	var totals *manager.ResponseHostMemoryTotals
-	if host.Status == common.ONLINE {
-		totals, err = host.GetMemoryTotals(ctx)
-		if err != nil {
-			return formatErrorResp(ctx, err.(*common.RequestError))
-		}
-	} else {
-		totals = &manager.ResponseHostMemoryTotals{
-			LocalMemoryMib:  0,
-			RemoteMemoryMib: 0,
-		}
+	totals, err := host.GetMemoryTotals(ctx)
+	if err != nil {
+		return formatErrorResp(ctx, err.(*common.RequestError))
 	}
 
 	h := openapi.Host{
@@ -1005,25 +974,9 @@ func (cfm *CfmApiService) HostsPost(ctx context.Context, credentials openapi.Cre
 
 	host, err := manager.AddHost(ctx, &credentials)
 	if err != nil {
-
-		if datastore.DStore().GetDataStore().ContainsHost(credentials.CustomId) {
-
-			// If host add fails BUT host already in datastore, set pre-existing host offline
-			req := datastore.UpdateHostRequest{
-				HostId: credentials.CustomId,
-				Status: common.OFFLINE,
-			}
-			datastore.DStore().GetDataStore().UpdateHost(ctx, &req)
-			datastore.DStore().Store()
-		}
-
 		return formatErrorResp(ctx, err.(*common.RequestError))
 	}
 
-	datastore.DStore().GetDataStore().DeleteHost(host.Id)
-	datastore.DStore().GetDataStore().AddHost(&credentials)
-	datastore.DStore().Store()
-
 	h := openapi.Host{
 		Id:        host.Id,
 		IpAddress: host.GetNetIp(),
@@ -1052,13 +1005,6 @@ func (cfm *CfmApiService) HostsResyncById(ctx context.Context, hostId string) (o
 		return formatErrorResp(ctx, err.(*common.RequestError))
 	}
 
-	req := datastore.UpdateHostRequest{
-		HostId: host.Id,
-		Status: common.ConnectionStatus(host.Status),
-	}
-	datastore.DStore().GetDataStore().UpdateHost(ctx, &req)
-	datastore.DStore().Store()
-
 	h := openapi.Host{
 		Id:        host.Id,
 		IpAddress: host.GetNetIp(),

diff --git a/pkg/common/datastore/datastore.go b/pkg/common/datastore/datastore.go
@@ -149,25 +149,23 @@ func (c *DataStore) UpdateBlade(ctx context.Context, r *BladeUpdateRequest) erro
 	return appliance.UpdateBlade(ctx, r)
 }
 
-type UpdateHostRequest struct {
+type UpdateHostStatusRequest struct {
 	HostId string
 	Status common.ConnectionStatus
 }
 
-// UpdateHost: Update a host's data
-func (c *DataStore) UpdateHost(ctx context.Context, r *UpdateHostRequest) error {
+// UpdateHost: Update a host's status
+func (c *DataStore) UpdateHostStatus(ctx context.Context, r *UpdateHostStatusRequest) error {
 	logger := klog.FromContext(ctx)
 
 	host, exists := c.SavedHosts[r.HostId]
 	if !exists {
-		err := fmt.Errorf("host [%s] not found in data store during host update", r.HostId)
-		logger.Error(err, "failure: update host")
+		err := fmt.Errorf("host [%s] not found in data store during host status update", r.HostId)
+		logger.Error(err, "failure: update host status")
 		return err
 	}
 
-	if r.Status != "" {
-		host.ConnectionStatus = r.Status
-	}
+	host.ConnectionStatus = r.Status
 
 	return nil
 }

diff --git a/pkg/manager/host.go b/pkg/manager/host.go
@@ -9,6 +9,7 @@ import (
 
 	"cfm/pkg/backend"
 	"cfm/pkg/common"
+	"cfm/pkg/common/datastore"
 	"cfm/pkg/openapi"
 
 	"k8s.io/klog/v2"
@@ -329,6 +330,15 @@ func (h *Host) GetMemoryTotals(ctx context.Context) (*ResponseHostMemoryTotals,
 
 	var local, remote int32
 
+	response := ResponseHostMemoryTotals{
+		LocalMemoryMib:  0,
+		RemoteMemoryMib: 0,
+	}
+
+	if h.Status == common.OFFLINE {
+		return &response, nil
+	}
+
 	for _, memory := range h.Memory {
 		totals, err := memory.GetTotals(ctx)
 		if err != nil || totals == nil {
@@ -341,10 +351,8 @@ func (h *Host) GetMemoryTotals(ctx context.Context) (*ResponseHostMemoryTotals,
 		remote += totals.RemoteMemoryMib
 	}
 
-	response := ResponseHostMemoryTotals{
-		LocalMemoryMib:  local,
-		RemoteMemoryMib: remote,
-	}
+	response.LocalMemoryMib = local
+	response.RemoteMemoryMib = remote
 
 	logger.V(2).Info("success: get memory totals", "hostId", h.Id)
 
@@ -431,6 +439,14 @@ func (h *Host) UpdateConnectionStatusBackend(ctx context.Context) {
 		h.Status = common.ONLINE
 	}
 
+	// Update datastore status
+	r := datastore.UpdateHostStatusRequest{
+		HostId: h.Id,
+		Status: common.ConnectionStatus(h.Status),
+	}
+	datastore.DStore().GetDataStore().UpdateHostStatus(ctx, &r)
+	datastore.DStore().Store()
+
 	logger.V(2).Info("update host status(backend)", "status", h.Status, "hostId", h.Id)
 }
 

diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go
@@ -8,6 +8,7 @@ import (
 
 	"cfm/pkg/backend"
 	"cfm/pkg/common"
+	"cfm/pkg/common/datastore"
 	"cfm/pkg/openapi"
 
 	"k8s.io/klog/v2"
@@ -261,9 +262,13 @@ func AddHost(ctx context.Context, c *openapi.Credentials) (*Host, error) {
 		return nil, &common.RequestError{StatusCode: common.StatusManagerInitializationFailure, Err: newErr}
 	}
 
-	// Cache it
+	// Add host to device cache
 	deviceCache.AddHost(host)
 
+	// Add host to datastore
+	datastore.DStore().GetDataStore().AddHost(c)
+	datastore.DStore().Store()
+
 	logger.V(2).Info("success: add host", "hostId", host.Id)
 
 	return host, nil
@@ -293,16 +298,22 @@ func DeleteHostById(ctx context.Context, hostId string) (*Host, error) {
 		logger.Error(newErr, "failure: delete host by id")
 
 		// Currently, backend ALWAYS deletes the host session from the backend map.
-		// Delete host from manager map as well.
-		logger.V(2).Info("force host deletion after backend session failure", "hostId", host.Id)
+		// Delete host from manager cache and datastore as well
+		logger.V(2).Info("force host deletion after backend delete session failure", "hostId", host.Id)
 		deviceCache.DeleteHostById(host.Id)
+		datastore.DStore().GetDataStore().DeleteHost(host.Id)
+		datastore.DStore().Store()
 
 		return nil, &common.RequestError{StatusCode: common.StatusHostDeleteSessionFailure, Err: newErr}
 	}
 
-	// delete host from cache
+	// delete host from manager cache
 	h := deviceCache.DeleteHostById(host.Id)
 
+	// delete host from datastore
+	datastore.DStore().GetDataStore().DeleteHost(host.Id)
+	datastore.DStore().Store()
+
 	logger.V(2).Info("success: delete host by id", "hostId", h.Id)
 
 	return h, nil