Add check for host NQN before un mapping it from Subsystem

Author: VinayKumarHavanur

storage_drivers/ontap/api/abstraction_rest.go

@@ -3451,23 +3451,36 @@ func (d OntapAPIREST) NVMeEnsureNamespaceUnmapped(ctx context.Context, hostNQN,
 		return false, fmt.Errorf("error getting hosts mapped to subsystem with UUID %s; %v", subsystemUUID, err)
 	}
 
-	if subsystemHosts == nil {
-		return false, fmt.Errorf("error getting hosts attached to subsystem %v", subsystemUUID)
+	hostFound := false
+	for _, host := range subsystemHosts {
+		if host != nil && *host.Nqn == hostNQN {
+			hostFound = true
+			break
+		}
 	}
 
 	// In case of multiple hosts attached to a subsystem (e.g. in RWX case), do not delete the namespace,
 	// subsystem or the published info
 	if len(subsystemHosts) > 1 {
-		Logc(ctx).Infof("Multiple hosts are attached to this subsystem %v. Do not delete namespace or subsystem",
-			subsystemUUID)
-		// Remove HostNQN from the subsystem using api call
-		if err := d.api.NVMeRemoveHostFromSubsystem(ctx, hostNQN, subsystemUUID); err != nil {
-			Logc(ctx).Errorf("Remove host from subsystem failed; %v", err)
-			return false, err
+		if hostFound {
+			Logc(ctx).Infof("Multiple hosts are attached to this subsystem %v. Do not delete namespace or subsystem",
+				subsystemUUID)
+			if err = d.api.NVMeRemoveHostFromSubsystem(ctx, hostNQN, subsystemUUID); err != nil {
+				Logc(ctx).Errorf("Remove host from subsystem failed; %v", err)
+				return false, err
+			}
 		}
 		return false, nil
 	}
 
+	// If there is only one host attached to the subsystem, check if it is the host which is being asked to be unmapped
+	if len(subsystemHosts) == 1 && !hostFound {
+		Logc(ctx).Infof("Hosts are attached to this subsystem %v. Do not delete namespace or subsystem",
+			subsystemUUID)
+		return false, nil
+	}
+
+	// Proceed further as there are no hosts or only the specified host is present that needs to be unmapped.
 	// Unmap the namespace from the subsystem
 	err = d.api.NVMeSubsystemRemoveNamespace(ctx, subsystemUUID, namespaceUUID)
 	if err != nil {

storage_drivers/ontap/api/abstraction_rest_test.go

@@ -865,6 +865,8 @@ func TestNVMeNamespaceUnmapped(t *testing.T) {
 	nsUUID := "fakeNsUUID"
 	hostNQN := "fakeHostNQN"
 	host2NQN := "fakeHost2NQN"
+	nonExistentHostNQN := "fakeHostNQNNonExisting"
+
 	host1 := &models.NvmeSubsystemHost{Nqn: &hostNQN}
 	host2 := &models.NvmeSubsystemHost{Nqn: &host2NQN}
 	var removePublishInfo bool
@@ -901,11 +903,14 @@ func TestNVMeNamespaceUnmapped(t *testing.T) {
 	mock.EXPECT().ClientConfig().Return(clientConfig).AnyTimes()
 	mock.EXPECT().NVMeIsNamespaceMapped(ctx, subsystemUUID, nsUUID).Return(true, nil).Times(1)
 	mock.EXPECT().NVMeGetHostsOfSubsystem(ctx, subsystemUUID).Return(nil, nil).Times(1)
+	mock.EXPECT().NVMeSubsystemRemoveNamespace(ctx, subsystemUUID, nsUUID).AnyTimes().Return(nil).Times(1)
+	mock.EXPECT().NVMeNamespaceCount(ctx, subsystemUUID).Return(int64(0), nil).Times(1)
+	mock.EXPECT().NVMeSubsystemDelete(ctx, subsystemUUID).Return(nil).Times(1)
 
 	removePublishInfo, err = oapi.NVMeEnsureNamespaceUnmapped(ctx, hostNQN, subsystemUUID, nsUUID)
 
-	assert.Equal(t, false, removePublishInfo, "subsystem removed")
-	assert.Error(t, err)
+	assert.Equal(t, true, removePublishInfo, "subsystem removed")
+	assert.NoError(t, err)
 
 	// case 5: multiple hosts of the subsystem returned  but error while removing host from subsystem
 	mock.EXPECT().ClientConfig().Return(clientConfig).AnyTimes()
@@ -977,6 +982,28 @@ func TestNVMeNamespaceUnmapped(t *testing.T) {
 
 	assert.Equal(t, true, removePublishInfo, "subsystem is not removed")
 	assert.NoError(t, err)
+
+	// case 11: Success when the nqn is already removed from subsystem
+	mock.EXPECT().ClientConfig().Return(clientConfig).AnyTimes()
+	mock.EXPECT().NVMeIsNamespaceMapped(ctx, subsystemUUID, nsUUID).Return(true, nil).Times(1)
+	mock.EXPECT().NVMeGetHostsOfSubsystem(ctx, subsystemUUID).Return([]*models.NvmeSubsystemHost{host1},
+		nil).Times(1)
+
+	removePublishInfo, err = oapi.NVMeEnsureNamespaceUnmapped(ctx, nonExistentHostNQN, subsystemUUID, nsUUID)
+
+	assert.Equal(t, false, removePublishInfo, "nqn is unmapped")
+	assert.NoError(t, err)
+
+	// case 12: Success when the nqn is already removed from subsystem with multiple hosts
+	mock.EXPECT().ClientConfig().Return(clientConfig).AnyTimes()
+	mock.EXPECT().NVMeIsNamespaceMapped(ctx, subsystemUUID, nsUUID).Return(true, nil).Times(1)
+	mock.EXPECT().NVMeGetHostsOfSubsystem(ctx, subsystemUUID).Return([]*models.NvmeSubsystemHost{host1, host2},
+		nil).Times(1)
+
+	removePublishInfo, err = oapi.NVMeEnsureNamespaceUnmapped(ctx, nonExistentHostNQN, subsystemUUID, nsUUID)
+
+	assert.Equal(t, false, removePublishInfo, "nqn is unmapped")
+	assert.NoError(t, err)
 }
 
 func TestNVMeIsNamespaceMapped(t *testing.T) {