search

FlowFuse / flowfuse

Connect, collect, transform, visualise, and interact with your Industrial Data in a single platform. Use FlowFuse to manage, scale and secure your Node-RED solutions.
https://flowfuse.com
Other
278 stars 63 forks source link

Auto device snapshots either not taken or not being listed in prod #3500

Closed Steve-Mcl closed 8 months ago

Steve-Mcl commented 8 months ago

Current Behavior

The new device auto snapshot feature works as designed on staging but it something is different with production.

the device log informs the user a snapshot is being taken but the table never shows any auto snapshots:

Device log

image

Device snapshots

image

Expected Behavior

Snapshots should be captured/shown

Steps To Reproduce

Add a device to an application, switch to dev mode, make a change to flows and deploy. Witness the console message [info] Capturing device snapshot, check the device snapshots table - none shown (or perhaps not actually captured?)

Environment

Have you provided an initial effort estimate for this issue?

I have provided an initial effort estimate

Steve-Mcl commented 8 months ago

Following investigation, we have found a difference in the DDL of production and staging databases.

This has came about organically due to timing and migrations.

Further details here if required

Steve 1 hour ago RE: Differences between Prod and Staging (and local dev on PG/SQLite) In grafana, I can see the error: parent: error: null value in column "UserId" of relation "ProjectSnapshots" violates not-null constraint ``` 2024-02-20T09:23:32.168283595Z stderr F name: 'SequelizeDatabaseError', 2024-02-20T09:23:32.168280555Z stderr F at async Immediate. (/usr/src/forge/app/node_modules/@flowfuse/flowfuse/forge/routes/logging/index.js:149:46) { 2024-02-20T09:23:32.168277915Z stderr F at async Object.doAutoSnapshot [as doDeviceAutoSnapshot] (/usr/src/forge/app/node_modules/@flowfuse/flowfuse/forge/db/controllers/ProjectSnapshot.js:150:30) 2024-02-20T09:23:32.168274455Z stderr F at async Object.createDeviceSnapshot (/usr/src/forge/app/node_modules/@flowfuse/flowfuse/forge/db/controllers/ProjectSnapshot.js:307:26) 2024-02-20T09:23:32.168271655Z stderr F at async ProjectSnapshot.create (/usr/src/forge/app/node_modules/sequelize/lib/model.js:1362:12) 2024-02-20T09:23:32.168264245Z stderr F at async model.save (/usr/src/forge/app/node_modules/sequelize/lib/model.js:2490:35) 2024-02-20T09:23:32.168261235Z stderr F at async PostgresQueryInterface.insert (/usr/src/forge/app/node_modules/sequelize/lib/dialects/abstract/query-interface.js:308:21) ``` 21 replies Steve 44 minutes ago This is not seen in staging (nor in local dev on PG or SQLite) Steve 41 minutes ago Looking for best approach (both how/why and the final fix): interrogate why DB structure on staging differs prod and fix that? add a "system" users to DB and have auto snapshots use the "system" userId update our auth routines so that the users launching the device editor can be passed back through to the DB something else? Steve 38 minutes ago Ben can you get the DDL from PROD and STAGING for table ProjectSnapshots please? Ben 37 minutes ago I don't understand 2, there is only 1 database use (forge) and everything should be done as that user. It has full rights in the database Ben 37 minutes ago Yes I can look at a DDL Steve 36 minutes ago re 2) as in a dummy user named "system" (or platform or forge) in the users table that we can specify the as the userId of in snapshot of an auto snapshot Ben 32 minutes ago ### production flowforge=> \d "ProjectSnapshots" Table "public.ProjectSnapshots" | Column | Type | Collation | Nullable | Default | | ------------ | ------------------------- | ---------- | --------- | ----------------------------------------------- | | id | integer | | not null | nextval('"ProjectSnapshots_id_seq"'::regclass)| | name | character varying(255) | | not null | | | settings | text | | | | | flows | text | | | | | createdAt | timestamp with time zone | | not null | | | updatedAt | timestamp with time zone | | not null | | | ProjectId | uuid | | | | | UserId | integer | | not null | | | description | text | | | | | DeviceId | integer | | | | ``` Indexes: "ProjectSnapshots_pkey" PRIMARY KEY, btree (id) Foreign-key constraints: "ProjectSnapshots_DeviceId_fkey" FOREIGN KEY ("DeviceId") REFERENCES "Devices"(id) ON UPDATE CASCADE ON DELETE SET NULL "ProjectSnapshots_ProjectId_fkey" FOREIGN KEY ("ProjectId") REFERENCES "Projects"(id) ON UPDATE CASCADE ON DELETE SET NULL "ProjectSnapshots_UserId_fkey" FOREIGN KEY ("UserId") REFERENCES "Users"(id) ON UPDATE CASCADE ON DELETE SET NULL Referenced by: TABLE ""DeviceGroups"" CONSTRAINT "DeviceGroups_targetSnapshotId_fkey" FOREIGN KEY ("targetSnapshotId") REFERENCES "ProjectSnapshots"(id) ON UPDATE CASCADE ON DELETE SET NULL TABLE ""Devices"" CONSTRAINT "Devices_activeSnapshotId_fkey" FOREIGN KEY ("activeSnapshotId") REFERENCES "ProjectSnapshots"(id) ON UPDATE CASCADE ON DELETE SET NULL TABLE ""Devices"" CONSTRAINT "Devices_targetSnapshotId_fkey" FOREIGN KEY ("targetSnapshotId") REFERENCES "ProjectSnapshots"(id) ON UPDATE CASCADE ON DELETE SET NULL TABLE ""PipelineStageDeviceGroups"" CONSTRAINT "PipelineStageDeviceGroups_targetSnapshotId_fkey" FOREIGN KEY ("targetSnapshotId") REFERENCES "ProjectSnapshots"(id) ON UPDATE CASCADE ON DELETE SET NULL ``` ### staging: flowforge=> \d "ProjectSnapshots" Table "public.ProjectSnapshots" | Column | Type | Collation | Nullable | Default | | ------------ | ------------------------- | ---------- | --------- | ----------------------------------------------- | | id | integer | | not null | nextval('"ProjectSnapshots_id_seq"'::regclass)| | name | character varying(255) | | not null | | | settings | text | | | | | flows | text | | | | | createdAt | timestamp with time zone | | not null | | | updatedAt | timestamp with time zone | | not null | | | ProjectId | uuid | | | | | UserId | integer | | | | | description | text | | | | | DeviceId | integer | | | | ``` Indexes: "ProjectSnapshots_pkey" PRIMARY KEY, btree (id) Foreign-key constraints: "ProjectSnapshots_DeviceId_fkey" FOREIGN KEY ("DeviceId") REFERENCES "Devices"(id) ON UPDATE CASCADE ON DELETE SET NULL "ProjectSnapshots_ProjectId_fkey" FOREIGN KEY ("ProjectId") REFERENCES "Projects"(id) ON UPDATE CASCADE ON DELETE SET NULL "ProjectSnapshots_UserId_fkey" FOREIGN KEY ("UserId") REFERENCES "Users"(id) ON UPDATE CASCADE ON DELETE SET NULL Referenced by: TABLE ""DeviceGroups"" CONSTRAINT "DeviceGroups_targetSnapshotId_fkey" FOREIGN KEY ("targetSnapshotId") REFERENCES "ProjectSnapshots"(id) ON UPDATE CASCADE ON DELETE SET NULL TABLE ""Devices"" CONSTRAINT "Devices_activeSnapshotId_fkey" FOREIGN KEY ("activeSnapshotId") REFERENCES "ProjectSnapshots"(id) ON UPDATE CASCADE ON DELETE SET NULL TABLE ""Devices"" CONSTRAINT "Devices_targetSnapshotId_fkey" FOREIGN KEY ("targetSnapshotId") REFERENCES "ProjectSnapshots"(id) ON UPDATE CASCADE ON DELETE SET NULL TABLE ""PipelineStageDeviceGroups"" CONSTRAINT "PipelineStageDeviceGroups_targetSnapshotId_fkey" FOREIGN KEY ("targetSnapshotId") REFERENCES "ProjectSnapshots"(id) ON UPDATE CASCADE ON DELETE SET NULL ``` Looks like prod has a "not null" on the UserId field (edited) Steve 32 minutes ago > Looks like prod has a "not null" on the UserId field As I suspected. Not seeing this in local dev either (PG/sqlite) (edited) Steve 28 minutes ago any thoughts on how this came to be? I am going to create a brand new DB (well FF is) and double check the DDL of a newly created ProjectSnapshots Ben 27 minutes ago Not without looking at the history of the ProjectSnapshots model file and the migrations Ben 27 minutes ago I vote for removing the constraint from the prod db, but only when we understand how it got there Steve 26 minutes ago The DDL of a newly created DB does not have not null specified on the userId field of ProjectSnapshot table (edited) Steve 25 minutes ago I agree on the "fix" Ben - (I am digging for a historical reason too) Steve 18 minutes ago Original migration forge/db/migrations/20220517-01-add-project-snapshot.js had `allowNull: false` ``` UserId: { type: DataTypes.INTEGER, allowNull: false, references: { model: 'Users', key: 'id' }, onDelete: 'SET NULL', onUpdate: 'CASCADE' } ``` Migration forge/db/migrations/20240202-01-initialise-database-structure.js has `allowNull: true` ``` UserId: { type: DataTypes.INTEGER, allowNull: true, defaultValue: null, references: { model: 'Users', key: 'id' }, onDelete: 'SET NULL', onUpdate: 'CASCADE' } ``` Obviously the initialise db structure script was taken from "real world" situation. As for how it got there I have not yet found. This though backs up the proposed fix Steve 10 minutes ago Looking at the history of the model (`gitk forge/db/models/ProjectSnapshot.js`), we never specified the foreign key in the belongsTo relationship and so sequelize considers it optional. in other-words, anyone firing up FF before 2022-05-17 got allowNull: false when the migration ran, anyone firing up SS after 2205-17 got allowNull: true because of the model Steve 8 minutes ago I am certain FF Prod DB has existed through every migration and Staging (in its current form) came on line after 2022-05-17 Ben 7 minutes ago Yes, staging got rebuilt early 2023 iirc

A migration to update ProjectSnapshot will be necessary to ensure it matches the desired state in forge/db/migrations/20220517-01-add-project-snapshot.js

Special care should be taken with sqlite as certain operations can cause undesired side-effects

See forge/db/migrations/20231005-01-update-projectSnapshot-constraint.js for an example of this