From 97f3b2a51eed877c7fb6947443ad6c0f46205643 Mon Sep 17 00:00:00 2001 From: Felipe Lalanne Date: Wed, 1 Sep 2021 22:13:27 +0000 Subject: [PATCH] Update types and create methods for reporting v3 state --- src/api-binder.ts | 17 +- src/compose/application-manager.ts | 143 ++++++++++- src/compose/composition-steps.ts | 4 +- src/compose/images.ts | 6 +- src/compose/service-manager.ts | 11 +- src/device-state.ts | 40 +++- src/device-state/current-state.ts | 18 +- src/types/state.ts | 73 +++++- test/src/compose/application-manager.spec.ts | 238 +++++++++++++++++++ 9 files changed, 507 insertions(+), 43 deletions(-) diff --git a/src/api-binder.ts b/src/api-binder.ts index 5501fb7c..5198769a 100644 --- a/src/api-binder.ts +++ b/src/api-binder.ts @@ -215,7 +215,7 @@ export async function patchDevice( } if (!conf.provisioned) { - throw new Error('DEvice must be provisioned to update a device'); + throw new Error('Device must be provisioned to update a device'); } if (balenaApi == null) { @@ -308,11 +308,7 @@ export async function fetchDeviceTags(): Promise { )}`, ); } - return { - id: id.right, - name: name.right, - value: value.right, - }; + return { id: id.right, name: name.right, value: value.right }; }); } @@ -408,17 +404,14 @@ async function reportInitialEnv( const defaultConfig = deviceConfig.getDefaults(); - const currentState = await deviceState.getCurrentState(); - const targetConfig = await deviceConfig.formatConfigKeys( - targetConfigUnformatted, - ); + const currentConfig = await deviceConfig.getCurrent(); + const targetConfig = deviceConfig.formatConfigKeys(targetConfigUnformatted); - if (!currentState.local.config) { + if (!currentConfig) { throw new InternalInconsistencyError( 'No config defined in reportInitialEnv', ); } - const currentConfig: Dictionary = currentState.local.config; for (const [key, value] of _.toPairs(currentConfig)) { let varValue = value; // We want to disable local mode when joining a cloud diff --git a/src/compose/application-manager.ts b/src/compose/application-manager.ts index 0b69a5d9..3ba06dcc 100644 --- a/src/compose/application-manager.ts +++ b/src/compose/application-manager.ts @@ -36,7 +36,9 @@ import { CompositionStep, generateStep } from './composition-steps'; import { InstancedAppState, TargetApps, - DeviceReportFields, + DeviceLegacyReport, + AppState, + ServiceState, } from '../types/state'; import { checkTruthy } from '../lib/validation'; import { Proxyvisor } from '../proxyvisor'; @@ -44,7 +46,7 @@ import { EventEmitter } from 'events'; type ApplicationManagerEventEmitter = StrictEventEmitter< EventEmitter, - { change: DeviceReportFields } + { change: DeviceLegacyReport } >; const events: ApplicationManagerEventEmitter = new EventEmitter(); export const on: typeof events['on'] = events.on.bind(events); @@ -997,3 +999,140 @@ export async function getLegacyState() { return { local: apps, dependent }; } + +// TODO: this function is probably more inefficient than it needs to be, since +// it tried to optimize for readability, look for a way to make it simpler +export async function getState() { + const [services, images] = await Promise.all([ + serviceManager.getState(), + imageManager.getState(), + ]); + + type ServiceInfo = { + appId: number; + appUuid: string; + commit: string; + serviceName: string; + createdAt?: Date; + } & ServiceState; + + // Get service data from images + const stateFromImages: ServiceInfo[] = images.map( + ({ + appId, + appUuid, + name, + commit, + serviceName, + status, + downloadProgress, + }) => ({ + appId, + appUuid, + image: name, + commit, + serviceName, + status: status as string, + ...(downloadProgress && { download_progress: downloadProgress }), + }), + ); + + // Get all services and augment service data from the image if any + const stateFromServices = services + .map(({ appId, appUuid, commit, serviceName, status, createdAt }) => [ + // Only include appUuid if is available, if not available we'll get it from the image + { + appId, + ...(appUuid && { appUuid }), + commit, + serviceName, + status, + createdAt, + }, + // Get the corresponding image to augment the service data + stateFromImages.find( + (img) => img.serviceName === serviceName && img.commit === commit, + ), + ]) + // We cannot report services that do not have an image as the API + // requires passing the image name + .filter(([, img]) => !!img) + .map(([svc, img]) => ({ ...img, ...svc } as ServiceInfo)) + .map((svc, __, serviceList) => { + // If the service is not running it cannot be a handover + if (svc.status !== 'Running') { + return svc; + } + + // If there one or more running services with the same name and appUuid, but different + // release, then we are still handing over so we need to report the appropriate + // status + const siblings = serviceList.filter( + (s) => + s.appUuid === svc.appUuid && + s.serviceName === svc.serviceName && + s.status === 'Running' && + s.commit !== svc.commit, + ); + + // There should really be only one element on the `siblings` array, but + // we chose the oldest service to have its status reported as 'Handing over' + if ( + siblings.length > 0 && + siblings.every((s) => svc.createdAt!.getTime() < s.createdAt!.getTime()) + ) { + return { ...svc, status: 'Handing over' }; + } else if (siblings.length > 0) { + return { ...svc, status: 'Awaiting handover' }; + } + return svc; + }); + + const servicesToReport = + // The full list of services is the union of images that have no container created yet + stateFromImages + .filter( + (img) => + !stateFromServices.some( + (svc) => + img.serviceName === svc.serviceName && img.commit === svc.commit, + ), + ) + // With the services that have a container + .concat(stateFromServices); + + // Get the list of commits for all appIds from the database + const commitsForApp = ( + await Promise.all( + // Deduplicate appIds first + [...new Set(servicesToReport.map((svc) => svc.appId))].map( + async (appId) => ({ + [appId]: await commitStore.getCommitForApp(appId), + }), + ), + ) + ).reduce((commits, c) => ({ ...commits, ...c }), {}); + + // Assemble the state of apps + return servicesToReport.reduce( + (apps, { appId, appUuid, commit, serviceName, createdAt, ...svc }) => ({ + ...apps, + [appUuid]: { + ...(apps[appUuid] ?? {}), + // Add the release_uuid if the commit has been stored in the database + ...(commitsForApp[appId] && { release_uuid: commitsForApp[appId] }), + releases: { + ...(apps[appUuid]?.releases ?? {}), + [commit]: { + ...(apps[appUuid]?.releases[commit] ?? {}), + services: { + ...(apps[appUuid]?.releases[commit]?.services ?? {}), + [serviceName]: svc, + }, + }, + }, + }, + }), + {} as { [appUuid: string]: AppState }, + ); +} diff --git a/src/compose/composition-steps.ts b/src/compose/composition-steps.ts index 686275b2..8223b159 100644 --- a/src/compose/composition-steps.ts +++ b/src/compose/composition-steps.ts @@ -13,7 +13,7 @@ import Volume from './volume'; import { checkTruthy } from '../lib/validation'; import * as networkManager from './network-manager'; import * as volumeManager from './volume-manager'; -import { DeviceReportFields } from '../types/state'; +import { DeviceLegacyReport } from '../types/state'; import * as commitStore from './commit'; interface BaseCompositionStepArgs { @@ -135,7 +135,7 @@ interface CompositionCallbacks { fetchStart: () => void; fetchEnd: () => void; fetchTime: (time: number) => void; - stateReport: (state: DeviceReportFields) => void; + stateReport: (state: DeviceLegacyReport) => void; bestDeltaSource: (image: Image, available: Image[]) => string | null; } diff --git a/src/compose/images.ts b/src/compose/images.ts index 716ddf93..99590276 100644 --- a/src/compose/images.ts +++ b/src/compose/images.ts @@ -435,18 +435,18 @@ export const getState = async () => { const images = (await getAvailable()).map((img) => ({ ...img, status: 'Downloaded' as Image['status'], - downloadImageSuccess: null, + downloadProgress: null, })); const imagesFromRunningTasks = Object.values(runningTasks).map( (task) => task.context, ); - const runningImageIds = imagesFromRunningTasks.map((img) => img.imageId); + const runningImageNames = imagesFromRunningTasks.map((img) => img.name); // TODO: this is possibly wrong, the value from getAvailable should be more reliable // than the value from running tasks return imagesFromRunningTasks.concat( - images.filter((img) => !runningImageIds.includes(img.imageId)), + images.filter((img) => !runningImageNames.includes(img.name)), ); }; diff --git a/src/compose/service-manager.ts b/src/compose/service-manager.ts index 672a3611..414d2160 100644 --- a/src/compose/service-manager.ts +++ b/src/compose/service-manager.ts @@ -119,6 +119,7 @@ export async function getState() { if (status[service.containerId] == null) { status[service.containerId] = _.pick(service, [ 'appId', + 'appUuid', 'imageId', 'status', 'releaseId', @@ -508,7 +509,15 @@ function reportNewStatus( containerId, _.merge( { status }, - _.pick(service, ['imageId', 'appId', 'releaseId', 'commit']), + _.pick(service, [ + 'imageId', + 'appId', + 'appUuid', + 'serviceName', + 'releaseId', + 'createdAt', + 'commit', + ]), ), ); } diff --git a/src/device-state.ts b/src/device-state.ts index ed5b54cc..c2ef2170 100644 --- a/src/device-state.ts +++ b/src/device-state.ts @@ -37,11 +37,11 @@ import * as deviceConfig from './device-config'; import { ConfigStep } from './device-config'; import { log } from './lib/supervisor-console'; import { - DeviceReportFields, - DeviceStatus, + DeviceLegacyReport, + DeviceLegacyState, InstancedDeviceState, TargetState, - InstancedAppState, + DeviceState, } from './types'; import * as dbFormat from './device-state/db-format'; import * as apiKeys from './lib/api-keys'; @@ -166,7 +166,7 @@ function createDeviceStateRouter() { router.get('/v1/device', async (_req, res) => { try { - const state = await getCurrentForReport(); + const state = await getLegacyState(); const stateToSend = _.pick(state.local, [ 'api_port', 'ip_address', @@ -248,7 +248,7 @@ type DeviceStateStep = | CompositionStepT | ConfigStep; -let currentVolatile: DeviceReportFields = {}; +let currentVolatile: DeviceLegacyReport = {}; const writeLock = updateLock.writeLock; const readLock = updateLock.readLock; let maxPollTime: number; @@ -534,9 +534,10 @@ export function getTarget({ // the same format as the target state. This method, // getCurrent and getCurrentForComparison should probably get // merged into a single method -export async function getCurrentForReport(): Promise { +// @deprecated +export async function getLegacyState(): Promise { const appsStatus = await applicationManager.getLegacyState(); - const theState: DeepPartial = { + const theState: DeepPartial = { local: {}, dependent: {}, }; @@ -566,9 +567,28 @@ export async function getCurrentForReport(): Promise { } } - return theState as DeviceStatus; + return theState as DeviceLegacyState; } +// Return current state in a way that the API understands +export async function getCurrentForReport(): Promise { + const apps = await applicationManager.getState(); + + const { name, uuid } = await config.getMany(['name', 'uuid']); + + if (!uuid) { + throw new InternalInconsistencyError('No uuid found for local device'); + } + + return { + [uuid]: { + name, + apps, + }, + }; +} + +// Get the current state as object instances export async function getCurrentState(): Promise { const [name, devConfig, apps, dependent] = await Promise.all([ config.get('name'), @@ -587,9 +607,7 @@ export async function getCurrentState(): Promise { }; } -export function reportCurrentState( - newState: DeviceReportFields & Partial = {}, -) { +export function reportCurrentState(newState: DeviceLegacyReport = {}) { if (newState == null) { newState = {}; } diff --git a/src/device-state/current-state.ts b/src/device-state/current-state.ts index b562e0d7..2a4d9c25 100644 --- a/src/device-state/current-state.ts +++ b/src/device-state/current-state.ts @@ -9,7 +9,7 @@ import { InternalInconsistencyError, StatusError } from '../lib/errors'; import { getRequestInstance } from '../lib/request'; import * as sysInfo from '../lib/system-info'; -import { DeviceStatus } from '../types'; +import { DeviceLegacyState } from '../types'; import * as config from '../config'; import { SchemaTypeKey, SchemaReturn } from '../config/schema-type'; import * as eventTracker from '../event-tracker'; @@ -22,7 +22,7 @@ const INTERNAL_STATE_KEYS = [ ]; export let stateReportErrors = 0; -const lastReportedState: DeviceStatus = { +const lastReportedState: DeviceLegacyState = { local: {}, dependent: {}, }; @@ -43,7 +43,7 @@ type CurrentStateReportConf = { }; type StateReport = { - stateDiff: DeviceStatus; + stateDiff: DeviceLegacyState; conf: Omit; }; @@ -51,7 +51,9 @@ type StateReport = { * Returns an object that contains only status fields relevant for the local mode. * It basically removes information about applications state. */ -const stripDeviceStateInLocalMode = (state: DeviceStatus): DeviceStatus => { +const stripDeviceStateInLocalMode = ( + state: DeviceLegacyState, +): DeviceLegacyState => { return { local: _.cloneDeep( _.omit(state.local, 'apps', 'is_on__commit', 'logs_channel'), @@ -103,7 +105,7 @@ async function report({ stateDiff, conf }: StateReport): Promise { return true; } -function newStateDiff(stateForReport: DeviceStatus): DeviceStatus { +function newStateDiff(stateForReport: DeviceLegacyState): DeviceLegacyState { const lastReportedLocal = lastReportedState.local; const lastReportedDependent = lastReportedState.dependent; if (lastReportedLocal == null || lastReportedDependent == null) { @@ -117,7 +119,7 @@ function newStateDiff(stateForReport: DeviceStatus): DeviceStatus { const diff = { local: _.omitBy( stateForReport.local, - (val, key: keyof NonNullable) => + (val, key: keyof NonNullable) => INTERNAL_STATE_KEYS.includes(key) || _.isEqual(lastReportedLocal[key], val) || !sysInfo.isSignificantChange( @@ -128,7 +130,7 @@ function newStateDiff(stateForReport: DeviceStatus): DeviceStatus { ), dependent: _.omitBy( stateForReport.dependent, - (val, key: keyof DeviceStatus['dependent']) => + (val, key: keyof DeviceLegacyState['dependent']) => INTERNAL_STATE_KEYS.includes(key) || _.isEqual(lastReportedDependent[key], val), ), @@ -197,7 +199,7 @@ function handleRetry(retryInfo: OnFailureInfo) { async function generateStateForReport() { const { hardwareMetrics } = await config.getMany(['hardwareMetrics']); - const currentDeviceState = await deviceState.getCurrentForReport(); + const currentDeviceState = await deviceState.getLegacyState(); // If hardwareMetrics is false, send null patch for system metrics to cloud API const info = { diff --git a/src/types/state.ts b/src/types/state.ts index c2fee109..9919ccc8 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -16,7 +16,7 @@ import { import App from '../compose/app'; -export type DeviceReportFields = Partial<{ +export type DeviceLegacyReport = Partial<{ api_port: number; api_secret: string | null; ip_address: string; @@ -29,15 +29,15 @@ export type DeviceReportFields = Partial<{ update_failed: boolean; update_pending: boolean; update_downloaded: boolean; - is_on__commit: string; logs_channel: null; mac_address: string | null; }>; // This is the state that is sent to the cloud -export interface DeviceStatus { +export interface DeviceLegacyState { local?: { config?: Dictionary; + is_on__commit?: string; apps?: { [appId: string]: { services: { @@ -49,7 +49,7 @@ export interface DeviceStatus { }; }; }; - } & DeviceReportFields; + } & DeviceLegacyReport; // TODO: Type the dependent entry correctly dependent?: { [key: string]: any; @@ -57,6 +57,71 @@ export interface DeviceStatus { commit?: string; } +export type ServiceState = { + image: string; + status: string; + download_progress?: number | null; +}; + +export type ReleaseState = { + services: { + [serviceName: string]: ServiceState; + }; +}; + +export type ReleasesState = { + [releaseUuid: string]: ReleaseState; +}; + +export type AppState = { + release_uuid?: string; + releases: ReleasesState; +}; + +export type DeviceReport = { + name?: string; + status?: string; + os_version?: string | null; // TODO: Should these purely come from the os app? + os_variant?: string | null; // TODO: Should these purely come from the os app? + supervisor_version?: string; // TODO: Should this purely come from the supervisor app? + provisioning_progress?: number | null; // TODO: should this be reported as part of the os app? + provisioning_state?: string | null; // TODO: should this be reported as part of the os app? + ip_address?: string; + mac_address?: string | null; + api_port?: number; // TODO: should this be reported as part of the supervisor app? + api_secret?: string | null; // TODO: should this be reported as part of the supervisor app? + logs_channel?: string; // TODO: should this be reported as part of the supervisor app? or should it not be reported anymore at all? + memory_usage?: number; + memory_total?: number; + storage_block_device?: string; + storage_usage?: number; + storage_total?: number; + cpu_temp?: number; + cpu_usage?: number; + cpu_id?: string; + is_undervolted?: boolean; + // TODO: these are ignored by the API but are used by supervisor local API, remove? + update_failed?: boolean; + update_pending?: boolean; + update_downloaded?: boolean; +}; + +export type DeviceState = { + [deviceUuid: string]: DeviceReport & { + /** + * Used for setting dependent devices as online + */ + is_online?: boolean; + /** + * Used for setting gateway device of dependent devices + */ + parent_device?: number; + apps?: { + [appUuid: string]: AppState; + }; + }; +}; + // Return a type with a default value const withDefault = ( type: T, diff --git a/test/src/compose/application-manager.spec.ts b/test/src/compose/application-manager.spec.ts index a06020c9..79bc6441 100644 --- a/test/src/compose/application-manager.spec.ts +++ b/test/src/compose/application-manager.spec.ts @@ -4,6 +4,7 @@ import { stub } from 'sinon'; import App from '../../../src/compose/app'; import * as applicationManager from '../../../src/compose/application-manager'; import * as imageManager from '../../../src/compose/images'; +import * as serviceManager from '../../../src/compose/service-manager'; import { Image } from '../../../src/compose/images'; import Network from '../../../src/compose/network'; import * as networkManager from '../../../src/compose/network-manager'; @@ -1266,4 +1267,241 @@ describe('compose/application-manager', () => { ), ).to.have.lengthOf(1); }); + + describe('getting applications current state', () => { + let getImagesState: sinon.SinonStub; + let getServicesState: sinon.SinonStub; + + before(() => { + getImagesState = sinon.stub(imageManager, 'getState'); + getServicesState = sinon.stub(serviceManager, 'getState'); + }); + + afterEach(() => { + getImagesState.reset(); + getServicesState.reset(); + }); + + after(() => { + getImagesState.restore(); + getServicesState.restore(); + }); + + it('reports the state of images if no service is available', async () => { + getImagesState.resolves([ + { + name: 'ubuntu:latest', + commit: 'latestrelease', + appUuid: 'myapp', + serviceName: 'ubuntu', + status: 'Downloaded', + }, + { + name: 'alpine:latest', + commit: 'latestrelease', + appUuid: 'myapp', + serviceName: 'alpine', + status: 'Downloading', + downloadProgress: 50, + }, + { + name: 'fedora:latest', + commit: 'newrelease', + appUuid: 'fedora', + serviceName: 'fedora', + status: 'Downloading', + downloadProgress: 75, + }, + { + name: 'fedora:older', + commit: 'oldrelease', + appUuid: 'fedora', + serviceName: 'fedora', + status: 'Downloaded', + }, + ]); + getServicesState.resolves([]); + + expect(await applicationManager.getState()).to.deep.equal({ + myapp: { + releases: { + latestrelease: { + services: { + ubuntu: { + image: 'ubuntu:latest', + status: 'Downloaded', + }, + alpine: { + image: 'alpine:latest', + status: 'Downloading', + download_progress: 50, + }, + }, + }, + }, + }, + fedora: { + releases: { + oldrelease: { + services: { + fedora: { + image: 'fedora:older', + status: 'Downloaded', + }, + }, + }, + newrelease: { + services: { + fedora: { + image: 'fedora:latest', + status: 'Downloading', + download_progress: 75, + }, + }, + }, + }, + }, + }); + }); + + it('augments the service data with image data', async () => { + getImagesState.resolves([ + { + name: 'ubuntu:latest', + commit: 'latestrelease', + appUuid: 'myapp', + serviceName: 'ubuntu', + status: 'Downloaded', + }, + { + name: 'alpine:latest', + commit: 'latestrelease', + appUuid: 'myapp', + serviceName: 'alpine', + status: 'Downloading', + downloadProgress: 50, + }, + { + name: 'fedora:older', + commit: 'oldrelease', + appUuid: 'fedora', + serviceName: 'fedora', + status: 'Downloaded', + }, + ]); + getServicesState.resolves([ + { + commit: 'latestrelease', + appUuid: 'myapp', + serviceName: 'ubuntu', + status: 'Running', + createdAt: new Date('2021-09-01T13:00:00'), + }, + { + commit: 'oldrelease', + serviceName: 'fedora', + status: 'Stopped', + createdAt: new Date('2021-09-01T12:00:00'), + }, + { + // Service without an image should not show on the final state + appUuid: 'debian', + commit: 'otherrelease', + serviceName: 'debian', + status: 'Stopped', + createdAt: new Date('2021-09-01T12:00:00'), + }, + ]); + + expect(await applicationManager.getState()).to.deep.equal({ + myapp: { + releases: { + latestrelease: { + services: { + ubuntu: { + image: 'ubuntu:latest', + status: 'Running', + }, + alpine: { + image: 'alpine:latest', + status: 'Downloading', + download_progress: 50, + }, + }, + }, + }, + }, + fedora: { + releases: { + oldrelease: { + services: { + fedora: { + image: 'fedora:older', + status: 'Stopped', + }, + }, + }, + }, + }, + }); + }); + + it('reports handover state if multiple services are running for the same app', async () => { + getImagesState.resolves([ + { + name: 'alpine:3.13', + commit: 'latestrelease', + appUuid: 'myapp', + serviceName: 'alpine', + status: 'Downloaded', + }, + { + name: 'alpine:3.12', + commit: 'oldrelease', + appUuid: 'myapp', + serviceName: 'alpine', + status: 'Downloaded', + }, + ]); + getServicesState.resolves([ + { + commit: 'latestrelease', + appUuid: 'myapp', + serviceName: 'alpine', + status: 'Running', + createdAt: new Date('2021-09-01T13:00:00'), + }, + { + commit: 'oldrelease', + appUuid: 'myapp', + serviceName: 'alpine', + status: 'Running', + createdAt: new Date('2021-09-01T12:00:00'), + }, + ]); + + expect(await applicationManager.getState()).to.deep.equal({ + myapp: { + releases: { + latestrelease: { + services: { + alpine: { + image: 'alpine:3.13', + status: 'Awaiting handover', + }, + }, + }, + oldrelease: { + services: { + alpine: { + image: 'alpine:3.12', + status: 'Handing over', + }, + }, + }, + }, + }, + }); + }); + }); });