Merge pull request #1174 from sjd78/pool-load-break

Fix Pool handling and refactor VM loading
oVirt · Mar 30, 2020 · 68ba99c · 68ba99c
2 parents 2f9b7dd + 796b681
commit 68ba99c
Show file tree

Hide file tree

Showing 10 changed files with 138 additions and 303 deletions.
diff --git a/src/actions/vm.js b/src/actions/vm.js
@@ -24,11 +24,8 @@ import {
   SET_FILTERS,
   SET_OVIRT_API_VERSION,
   SET_VM_ACTION_RESULT,
-  SET_VM_CDROM,
-  SET_VM_CONSOLES,
   SET_VM_DISKS,
   SET_VM_NICS,
-  SET_VM_SESSIONS,
   SET_VM_SNAPSHOTS,
   SET_VM_SORT,
   SHUTDOWN_VM,
@@ -328,26 +325,6 @@ export function vmActionInProgress ({ vmId, name, started }) {
   }
 }
 
-export function setVmConsoles ({ vmId, consoles }) {
-  return {
-    type: SET_VM_CONSOLES,
-    payload: {
-      vmId,
-      consoles,
-    },
-  }
-}
-
-export function setVmSessions ({ vmId, sessions }) {
-  return {
-    type: SET_VM_SESSIONS,
-    payload: {
-      vmId,
-      sessions,
-    },
-  }
-}
-
 export function setVmSnapshots ({ vmId, snapshots }) {
   return {
     type: SET_VM_SNAPSHOTS,
@@ -379,16 +356,6 @@ export function setChanged ({ value }) {
   }
 }
 
-export function setVmCdRom ({ cdrom, vmId }) {
-  return {
-    type: SET_VM_CDROM,
-    payload: {
-      cdrom,
-      vmId,
-    },
-  }
-}
-
 export function getVmCdRom ({ vmId, current = true }) {
   return {
     type: GET_VM_CDROM,

diff --git a/src/components/VmsList/Vms.js b/src/components/VmsList/Vms.js
@@ -22,9 +22,9 @@ class Vms extends React.Component {
     this.loadMore = this.loadMore.bind(this)
   }
 
-  loadMore () {
+  loadMore (scrollPage) {
     if (this.props.vms.get('notAllPagesLoaded')) {
-      this.props.onUpdate(this.props.vms.get('page') + 1)
+      this.props.fetchPage(this.props.vms.get('page') + 1)
     }
   }
 
@@ -35,25 +35,35 @@ class Vms extends React.Component {
 
     const filters = vms.get('filters').toJS()
 
-    const sortedVms = vms.get('vms').filter(vm => filterVms(vm, filters)).toList().map(vm => vm.set('isVm', true))
-    const sortedPools = vms.get('pools')
+    // Filter the VMs (1. apply the filter bar criteria, 2. only show Pool VMs if the Pool exists)
+    const filteredVms = vms.get('vms')
+      .filter(vm => filterVms(vm, filters))
+      .filter(vm => vm.getIn(['pool', 'id'], false) ? !!vms.getIn(['pools', vm.getIn(['pool', 'id'])], false) : true)
+      .toList()
+      .map(vm => vm.set('isVm', true))
+
+    // Filter the Pools (only show a Pool card if the user can currently 'Take' a VM from it)
+    const filteredPools = vms.get('pools')
       .filter(pool => alwaysShowPoolCard || (pool.get('vmsCount') < pool.get('maxUserVms') && pool.get('size') > 0 && filterVms(pool, filters)))
       .toList()
 
-    const vmsPoolsMerge = [ ...sortedVms, ...sortedPools ].sort(sortFunction(sort))
+    // Display the VMs and Pools together, sorted nicely
+    const vmsAndPools = [ ...filteredVms, ...filteredPools ].sort(sortFunction(sort))
+
+    const hasMore = vms.get('notAllPagesLoaded')
 
     return (
       <InfiniteScroll
         loadMore={this.loadMore}
         isReverse={!sort.isAsc}
-        hasMore={vms.get('notAllPagesLoaded')}
+        hasMore={hasMore}
         loader={<Loader key='infinite-scroll-loader' size={SIZES.LARGE} />}
         useWindow={false}
       >
         <ScrollPositionHistory uniquePrefix='vms-list' scrollContainerSelector='#page-router-render-component'>
           <div className='container-fluid container-cards-pf'>
             <div className={`row row-cards-pf ${style['cards-container']}`}>
-              {vmsPoolsMerge.map(instance =>
+              {vmsAndPools.map(instance =>
                 instance.get('isVm')
                   ? <Vm vm={instance} key={instance.get('id')} />
                   : <Pool pool={instance} key={instance.get('id')} />
@@ -69,7 +79,7 @@ class Vms extends React.Component {
 Vms.propTypes = {
   vms: PropTypes.object.isRequired,
   alwaysShowPoolCard: PropTypes.bool,
-  onUpdate: PropTypes.func.isRequired,
+  fetchPage: PropTypes.func.isRequired,
 }
 
 export default connect(
@@ -78,6 +88,6 @@ export default connect(
     alwaysShowPoolCard: !state.config.get('filter'),
   }),
   (dispatch) => ({
-    onUpdate: (page) => dispatch(getByPage({ page })),
+    fetchPage: (page) => dispatch(getByPage({ page })),
   })
 )(Vms)
diff --git a/src/constants/index.js b/src/constants/index.js
@@ -102,11 +102,8 @@ export const SET_USER_SESSION_TIMEOUT_INTERVAL = 'SET_USER_SESSION_TIMEOUT_INTER
 export const SET_USER_MESSAGES = 'SET_USER_MESSAGES'
 export const SET_USERMSG_NOTIFIED = 'SET_USERMSG_NOTIFIED'
 export const SET_VM_ACTION_RESULT = 'SET_VM_ACTION_RESULT'
-export const SET_VM_CDROM = 'SET_VM_CDROM'
-export const SET_VM_CONSOLES = 'SET_VM_CONSOLES'
 export const SET_VM_DISKS = 'SET_VM_DISKS'
 export const SET_VM_NICS = 'SET_VM_NICS'
-export const SET_VM_SESSIONS = 'SET_VM_SESSIONS'
 export const SET_VM_SORT = 'SET_VM_SORT'
 export const SET_VNIC_PROFILES = 'SET_VNIC_PROFILES'
 export const SET_VM_SNAPSHOTS = 'SET_VM_SNAPSHOTS'

diff --git a/src/ovirtapi/index.js b/src/ovirtapi/index.js
@@ -4,7 +4,7 @@ import type {
   DiskType,
   NicType,
   SnapshotType,
-  VmType, ApiVmType,
+  VmType,
 } from './types'
 
 import Selectors from '../selectors'
@@ -34,10 +34,6 @@ const OvirtApi = {
   // ---- Data transform functions (API -> internal, internal -> API)
   //
   //
-  vmToInternal ({ vm, getSubResources = false }: { vm: ApiVmType, getSubResources: boolean }): VmType {
-    return Transforms.VM.toInternal({ vm, includeSubResources: getSubResources })
-  },
-
   poolToInternal: Transforms.Pool.toInternal,
 
   diskToInternal: Transforms.DiskAttachment.toInternal,
@@ -570,6 +566,14 @@ function getOptionWithoutDefault (optionName: string, version: string): Promise<
     })
 }
 
+// export default new Proxy(OvirtApi, {
+//   get (target: Object, prop: string, receiver: Object): any {
+//     if (typeof target[prop] === 'function') {
+//       console.info(`getting OvirtApi.${prop}`)
+//     }
+//     return Reflect.get(...arguments)
+//   },
+// })
 export default OvirtApi
 export {
   Transforms,

diff --git a/src/ovirtapi/transform.js b/src/ovirtapi/transform.js
@@ -27,14 +27,6 @@ import type {
   ApiRoleType, RoleType,
 } from './types'
 
-import {
-  canUserChangeCd,
-  canUserEditVm,
-  canUserEditVmStorage,
-  getUserPermits,
-  canUserManipulateSnapshots,
-} from '../utils'
-
 import { isWindows } from '_/helpers'
 
 function vCpusCount ({ cpu }: { cpu: Object }): number {
@@ -95,7 +87,11 @@ function getPoolColor (id: string): string {
 //
 //
 const VM = {
-  toInternal ({ vm, includeSubResources = false }: { vm: ApiVmType, includeSubResources?: boolean }): VmType {
+  toInternal ({ vm }: { vm: ApiVmType }): VmType {
+    const permissions = vm.permissions && vm.permissions.permission
+      ? Permissions.toInternal({ permissions: vm.permissions.permission })
+      : []
+
     const parsedVm: Object = {
       name: vm['name'],
       description: vm['description'],
@@ -166,10 +162,6 @@ const VM = {
       sessions: [],
       nics: [],
       statistics: [],
-      permits: new Set(),
-      canUserChangeCd: true,
-      canUserEditVm: false,
-      canUserManipulateSnapshots: false,
 
       ssoGuestAgent: vm.sso.methods && vm.sso.methods.method && vm.sso.methods.method.length > 0 && vm.sso.methods.method.findIndex(method => method.id === 'guest_agent') > -1,
       display: {
@@ -181,52 +173,48 @@ const VM = {
         name: vm.time_zone.name,
         offset: vm.time_zone.utc_offset,
       },
-    }
 
-    if (includeSubResources) {
-      if (vm.cdroms && vm.cdroms.cdrom) {
-        parsedVm.cdrom = CdRom.toInternal({ cdrom: vm.cdroms.cdrom[0] }) // in oVirt there is always exactly 1 cdrom
-      }
+      // roles are required to calculate permits and 'canUse*', therefore its done in sagas
+      permissions,
+      userPermits: new Set(),
+      canUserChangeCd: true,
+      canUserEditVm: false,
+      canUserManipulateSnapshots: false,
+      canUserEditVmStorage: false,
+    }
 
-      if (vm.graphics_consoles && vm.graphics_consoles.graphics_console) {
-        parsedVm.consoles = VmConsoles.toInternal({ consoles: vm.graphics_consoles })
-      }
+    if (vm.cdroms && vm.cdroms.cdrom) {
+      parsedVm.cdrom = CdRom.toInternal({ cdrom: vm.cdroms.cdrom[0] }) // in oVirt there is always exactly 1 cdrom
+    }
 
-      if (vm.disk_attachments && vm.disk_attachments.disk_attachment) {
-        parsedVm.disks = vm.disk_attachments.disk_attachment.map(
-          attachment => DiskAttachment.toInternal({ attachment, disk: attachment.disk })
-        )
-      }
+    if (vm.graphics_consoles && vm.graphics_consoles.graphics_console) {
+      parsedVm.consoles = VmConsoles.toInternal({ consoles: vm.graphics_consoles })
+    }
 
-      if (vm.nics && vm.nics.nic) {
-        parsedVm.nics = vm.nics.nic.map(
-          nic => Nic.toInternal({ nic })
-        )
-      }
+    if (vm.disk_attachments && vm.disk_attachments.disk_attachment) {
+      parsedVm.disks = vm.disk_attachments.disk_attachment.map(
+        attachment => DiskAttachment.toInternal({ attachment, disk: attachment.disk })
+      )
+    }
 
-      if (vm.sessions && vm.sessions.session) {
-        parsedVm.sessions = VmSessions.toInternal({ sessions: vm.sessions })
-      }
+    if (vm.nics && vm.nics.nic) {
+      parsedVm.nics = vm.nics.nic.map(
+        nic => Nic.toInternal({ nic })
+      )
+    }
 
-      if (vm.snapshots && vm.snapshots.snapshot) {
-        parsedVm.snapshots = vm.snapshots.snapshot.map(
-          snapshot => Snapshot.toInternal({ snapshot })
-        )
-      }
+    if (vm.sessions && vm.sessions.session) {
+      parsedVm.sessions = VmSessions.toInternal({ sessions: vm.sessions })
+    }
 
-      if (vm.statistics && vm.statistics.statistic) {
-        parsedVm.statistics = VmStatistics.toInternal({ statistics: vm.statistics.statistic })
-      }
+    if (vm.snapshots && vm.snapshots.snapshot) {
+      parsedVm.snapshots = vm.snapshots.snapshot.map(
+        snapshot => Snapshot.toInternal({ snapshot })
+      )
+    }
 
-      if (vm.permissions && vm.permissions.permission) {
-        parsedVm.permits = getUserPermits(Permissions.toInternal({
-          permissions: vm.permissions.permission,
-        }))
-        parsedVm.canUserChangeCd = canUserChangeCd(parsedVm.permits)
-        parsedVm.canUserEditVm = canUserEditVm(parsedVm.permits)
-        parsedVm.canUserManipulateSnapshots = canUserManipulateSnapshots(parsedVm.permits)
-        parsedVm.canUserEditVmStorage = canUserEditVmStorage(parsedVm.permits)
-      }
+    if (vm.statistics && vm.statistics.statistic) {
+      parsedVm.statistics = VmStatistics.toInternal({ statistics: vm.statistics.statistic })
     }
 
     return parsedVm

diff --git a/src/ovirtapi/transport.js b/src/ovirtapi/transport.js
@@ -70,14 +70,14 @@ function httpGet ({ url, custHeaders = {} }: GetRequestType): Promise<Object> {
     ...custHeaders,
   }
 
-  console.log(`http GET[${myCounter}] -> url: "${url}", headers: ${logHeaders(headers)}`)
+  console.log(`http GET[${myCounter}] 🡒 url: "${url}", headers: ${logHeaders(headers)}`)
   return $.ajax(url, {
     type: 'GET',
     headers,
   })
     .then((data: Object): Object => {
       notifyStop(requestId)
-      console.log(`http GET[${myCounter}] <- data:`, data)
+      console.log(`http GET[${myCounter}] 🡐 data:`, data)
       return data
     })
     .catch((data: Object): Promise<Object> => {

diff --git a/src/reducers/vms.js b/src/reducers/vms.js
@@ -59,21 +59,29 @@ const vms = actionReducer(initialState, {
       if (!state.getIn(['vms', vm.id])) {
         state = state.set('notAllPagesLoaded', true)
       }
+
       updates[vm.id] = vm
       updates[vm.id].actionResults = state.getIn(['vms', vm.id, 'actionResults'], EMPTY_MAP).toJS()
 
       if (copySubResources) {
-        updates[vm.id].cdrom = state.getIn(['vms', vm.id, 'cdrom'], Immutable.fromJS({ file: { id: '' } })).toJS()
+        // Only copy consoles if the VM does not already have any
         updates[vm.id].consoles = vm.consoles.length === 0
           ? state.getIn(['vms', vm.id, 'consoles'], EMPTY_ARRAY).toJS()
           : vm.consoles
+
+        updates[vm.id].cdrom = state.getIn(['vms', vm.id, 'cdrom'], Immutable.fromJS({ file: { id: '' } })).toJS()
         updates[vm.id].disks = state.getIn(['vms', vm.id, 'disks'], EMPTY_ARRAY).toJS()
         updates[vm.id].nics = state.getIn(['vms', vm.id, 'nics'], EMPTY_ARRAY).toJS()
         updates[vm.id].sessions = state.getIn(['vms', vm.id, 'sessions'], EMPTY_ARRAY).toJS()
         updates[vm.id].snapshots = state.getIn(['vms', vm.id, 'snapshots'], EMPTY_ARRAY).toJS()
         updates[vm.id].statistics = state.getIn(['vms', vm.id, 'statistics'], EMPTY_MAP).toJS()
+
         updates[vm.id].permissions = state.getIn(['vms', vm.id, 'permissions'], EMPTY_ARRAY).toJS()
+        updates[vm.id].userPermits = state.getIn(['vms', vm.id, 'userPermits'], EMPTY_ARRAY).toJS()
+        updates[vm.id].canUserChangeCd = state.getIn(['vms', vm.id, 'canUserChangeCd'], true)
         updates[vm.id].canUserEditVm = state.getIn(['vms', vm.id, 'canUserEditVm'], false)
+        updates[vm.id].canUserManipulateSnapshots = state.getIn(['vms', vm.id, 'canUserManipulateSnapshots'], false)
+        updates[vm.id].canUserEditVmStorage = state.getIn(['vms', vm.id, 'canUserEditVmStorage'], false)
       }
     })
 
@@ -211,22 +219,18 @@ const vms = actionReducer(initialState, {
   },
 
   [UPDATE_VMPOOLS_COUNT] (state) {
-    state.get('pools').toList().map(pool => {
-      state = state.setIn(['pools', pool.get('id'), 'vmsCount'], 0)
-    })
+    state = state.update('pools', pools => pools.map(pool => pool.set('vmsCount', 0)))
 
-    state.get('vms').toList().map(vm => {
-      // Check if vm is in actual pool and its down, checking for down vms is for not count that vms in admin mode
-      if (
-        vm.getIn(['pool', 'id']) &&
-        (
-          vm.get('status') !== 'down' ||
-          state.getIn(['pools', vm.getIn(['pool', 'id']), 'type']) === 'manual'
-        )
-      ) {
-        state = state.updateIn(['pools', vm.getIn(['pool', 'id']), 'vmsCount'], count => count + 1)
+    state.get('vms').map(vm => {
+      const poolId = vm.getIn(['pool', 'id'])
+      if (poolId && state.getIn(['pools', poolId])) {
+        // VM is in a known pool ... down VMs don't count against the user total unless it is a manual pool
+        if (vm.get('status') !== 'down' || state.getIn(['pools', poolId, 'type']) === 'manual') {
+          state = state.updateIn(['pools', poolId, 'vmsCount'], count => count + 1)
+        }
       }
     })
+
     return state
   },
 

diff --git a/src/sagas/console/index.js b/src/sagas/console/index.js
@@ -86,15 +86,6 @@ export function* getRDPVm (action) {
 
 // -----
 
-export function* fetchConsoleVmMeta ({ vmId }) {
-  const consoles = yield callExternalAction('consoles', Api.consoles, { type: 'INTERNAL_CONSOLES', payload: { vmId } })
-
-  if (consoles && consoles['graphics_console']) { // && consoles['graphics_console'].length > 0) {
-    return Api.consolesToInternal({ consoles })
-  }
-  return []
-}
-
 /**
  * Check the sessions on a VM and if someone other then the given user has an open
  * console, flag the console as "in use" requiring manual confirmation to open the