FlowFuse / flowfuse

Build bespoke, flexible, and resilient manufacturing low-code applications with FlowFuse and Node-RED
https://flowfuse.com
Other
269 stars 63 forks source link

Define Node-RED version for Devices not bound to an Instance #2802

Closed joepavitt closed 4 months ago

joepavitt commented 1 year ago

Description

Originally found with https://github.com/flowforge/flowforge-device-agent/issues/173

Devices that are not bound to Instance do not have a way of pulling a defined Stack, as such, they default to the latest instance of Node-RED.

We should be able to configure (not sure where/how?) which version of Node-RED a Device runs.

Have you provided an initial effort estimate for this issue?

I have provided an initial effort estimate

Edited

Background

In the current FlowForge Device setup, devices that are not bound to an Instance automatically use the 'latest' version of Node-RED. This default behavior limits users who may need to run specific versions of Node-RED for development, compatibility, or testing purposes.

User Story

As a FlowFuse User who works with Devices, I want the ability to specify and change the Node-RED version (stack) for my devices, So that I can deploy different versions of Node-RED to my development devices, catering to the specific needs of each project.

Acceptance Criteria

Which customers would this be available to

All Starter + Team + Enterprise Tiers (CE)

joepavitt commented 1 year ago

As part of a bigger picture discussion potentially, but my proposal here would be:

joepavitt commented 1 year ago

fyi @MarianRaphael this feels like a big piece to consider

MarianRaphael commented 1 year ago

It should be by default simply the newest Node-RED version (3.1), in a next iteration we can add the possibility to change the stack / Node-RED version for a device.

joepavitt commented 1 year ago

Right now, it is latest by default. This issue is to tra k that iteration of enabling selection

joepavitt commented 1 year ago

I was also hoping to get https://github.com/flowforge/flowforge/issues/2802#issuecomment-1729607387 on your radar, think we should be aiming for this.

MarianRaphael commented 1 year ago

When creating a Device, you are asked which version of Node-RED you want it to run (in a similar vein to how Instances are asked for a Stack choice when created)

I believe the selection option is important to have, but not during the creation process. This should be straightforward.

We remove the concept of Devices being bound to Instances entirely, Devices and Instances should be treated as equals, always bound to an Application

As a "final step," yes, I like the idea, but not for now. Requirement: "Snapshots being deployed via a Pipeline" must be as simple / better than the current concept. So step by step.

Pushing remotely to Devices is then done in 1 of 2 ways: Tunnel Access to the Editor Snapshots being deployed via a Pipeline, where that snapshot could be created from any Instance or Device

Yes, this would be the consequence of step 2

hardillb commented 1 year ago

Devices bound to an Application also do not have a Template to inherit settings from e.g. custom node catalogues or .npmrc files.

ZJvandeWeg commented 8 months ago

cc @robmarcer @MarianRaphael given the post in Slack today

Steve-Mcl commented 5 months ago

@joepavitt @MarianRaphael can the scope of this be clarified please? It is not 100% clear to me.

for example, there is talk around "final steps" towards unifying device assignment and MVP and other approaches.

As Ben has pointed out earlier, devices do not have all the necessary groundwork to just add some settings to for specifying a node-red version like we can with instance stacks etc. doing this in a way that is a step towards achieving the eventual goal will be a significant change.

We could of course add a page in settings for adjusting the NR version (like Ben has done in #3588) however this approach does not integrate with snapshots (i.e. is not actually a step towards the goal) and would be additional technical debt to revert when we eventually do implement application device settings properly.

I feel this needs some discussion before commencement.

joepavitt commented 5 months ago

Yeah, reading through this, it's definitely missing required details in order to implement @MarianRaphael

ZJvandeWeg commented 5 months ago

@Steve-Mcl

devices do not have all the necessary groundwork to just add some settings to for specifying a node-red version like we can with instance stacks etc. doing this in a way that is a step towards achieving the eventual goal will be a significant change.

Is it possible to generate a snapshot when creating the device? We can call it Initial Snapshot and it does include those details? I understand it might create tech debt, though it's not clear to me if that's on the instances side or remote instance side. (That is, should long term instances work like devices or devices like instances do now?).

MarianRaphael commented 5 months ago

@joepavitt @Steve-Mcl I edited the description and specified the scope. If there is more groundwork missing, we can push this back, and we can discuss this issue in the next architecture refinement meeting.

joepavitt commented 5 months ago

Developer Mode Requirement: Changing the Node-RED version on a device is only permissible if the device is in 'Developer Mode'.

Why? Surely the stack is something that we should be able to define at any point? Why should I have to move 500+ devices into developer mode and update the stack for each?

Steve-Mcl commented 5 months ago

Stack Modification Option: In the device settings, users must have the option to change the stack. This includes a list of all available Node-RED versions (stacks) that the user can select from.

Where would these be specified? Devices have no concept of stacks, they install whatever version is specified directly from NPM. If you are you suggesting we would only permit the versions of node-red that are installed for instances, then it seems oddly restrictive since one has nothing to do with the other (i.e. the CORE does not provide the installation media for a remove instance, they come directly from NPM*)

Developer Mode Requirement: Changing the Node-RED version on a device is only permissible if the device is in 'Developer Mode'.

Not sure why this is a specification. The agent explicitly prohibits updates in developer mode meaning any changes would not occur until the user removes the device from developer mode.

MarianRaphael commented 5 months ago

Why? Surely the stack is something that we should be able to define at any point? Why should I have to move 500+ devices into developer mode and update the stack for each?

In Fleet Mode this kind of change should happen though a Pipeline, Device Group etc. but not manually. This is also how you would manage 500+ devices

Steve-Mcl commented 5 months ago

In the device settings, users must have the option to change the stack

As devices dont have the same templating as instances this would be additional technical debt as discussed previously. Of course we will do what is required but I want that to be understood before commencing.

for example, should the version be stored in the snapshot (as it is in instances). If yes, this would be a workaround and would differ to the implementation of https://github.com/FlowFuse/flowfuse/issues/3588

Steve-Mcl commented 5 months ago

Why? Surely the stack is something that we should be able to define at any point? Why should I have to move 500+ devices into developer mode and update the stack for each?

In Fleet Mode this kind of change should happen though a Pipeline, Device Group etc. but not manually.

This statement suggests you expect this setting to be part of the snapshot.

See previous statement.

MarianRaphael commented 5 months ago

should the version be stored in the snapshot (as it is in instances)

Yes, Devices and Instances should behave increasingly the same way

MarianRaphael commented 5 months ago

Stack Modification Option: In the device settings, users must have the option to change the stack. This includes a list of all available Node-RED versions (stacks) that the user can select from.

Where would these be specified?

Same as for instances. (Also for compatibility reasons between both)

Steve-Mcl commented 5 months ago

should the version be stored in the snapshot (as it is in instances)

Yes, Devices and Instances should behave increasingly the same way

Stack Modification Option: In the device settings, users must have the option to change the stack. This includes a list of all available Node-RED versions (stacks) that the user can select from. Where would these be specified?

Same as for instances. (Also for compatibility reasons between both)

I feel it is time we took a serious look at how we can get there using the same template approach of instances seeing as that is the end goal (or finding closest point of convergence between both offerings). I dont feel the right solution here is to shoehorn this in as an MVP that would need re-engineering when we eventually attempt to align the instances/devices (there is already a fairly large task to converge and I suspect this would only add to divergence & technical debt as is).

I will happily admit if I am wrong but part of this would be to explore the overall concepts and identify the common ground and work towards that - all of which makes this a rather large task.

MarianRaphael commented 5 months ago

I'll push this issue back and invite for a discussion.

Steve-Mcl commented 5 months ago

Following discussion we have identified some key aspects and some unknowns around lifecycle and UX

It was determined that the device settings pages should have something similar to the "Upgrade Stack" with free text entry for setting the node-red version (bonus points for valid sevmer check). This is MVP and aligns with the current acceptance criteria set out in the OP

The NR version value set should be persisted, likely in deviceSettings e.g. targetNodeRedVersion. This permits a device pulling "starter flows" (no snapshot) to have the required node-red version injected from the core settings.

The Node-RED version should be present in a snapshot in a compatible manor to how a snapshot from an instance deployed to an application assigned device does today. This, in theory, means no device agent changes for when a snapshot is pushed (from a pipeline or the application>devices>device>snapshots page)

How/when the NR version update actually occurs on the device and the lifetime of targetNodeRedVersion (when considering how the snapshot may or may not have a node-red version) needs some exploration and touch tests to see how it feels.

Ideally, the changes would NOT require device agent changes however it was agreed that should the overall UX suffer, this is a viable option)

@MarianRaphael please confirm / correct as required. Remove blocked tag if all good.

Steve-Mcl commented 5 months ago

@MarianRaphael gentle nudge.

Steve-Mcl commented 5 months ago

@MarianRaphael @joepavitt I am going to assume this is unblocked and progress with design and discovery (unless otherwise instructed)

MarianRaphael commented 5 months ago

@MarianRaphael please confirm / correct as required. Remove blocked tag if all good.

All good

Steve-Mcl commented 5 months ago

Progress

Approach:

  1. Add editor side tab to device->settings
  2. Get/Set an object editor in DeviceSettings Table
    1. this provides a place for future editor based settings
  3. Override the starter & actual snapshots .modules['node-red'] version if editor.nodeRedVersion is set
    1. This acts like an override of any snapshot applied to the device
    2. Clearing the value reverts to using the version in the snapshot (latest for starter/whatever was stack was used for the snapshot)

Image

Caveats:

With this approach, settings hash is updated and device is notified. However, in the current device editor logic, a settings updated does NOT cause the agent to re-request the snapshot (which now has the new version). also, current device agent logic does not cause the agent to run the installation procedure

Current state (no modifications to Device agent)

When a different snapshot is applied (or event just create a new snapshot and check the "Set as target" option, the user set NR version gets applied to the snapshot object and the device actually updates.

This however is not particularly intuitive and would need to careful documentation and/or some logic in the core to determine the current agent version and instruct the user on how to make the agent update the node-red version.

Solutions

To make the device agent react to the NR ver change, we would need to do one of the following

  1. generate a new snapshot and set as target for device.
    • This would make the device agent update (without any need for the device agent to be modified) however it is not ldeal as it would potentially break device groups (the target device id changes)
  2. Update the writeConfiguration logic in the Device agent to notice when editor.noderedVersion has been provided and update the package.json / call installDependencies
  3. Add a new MQTT message type that applies a local (persisted) setting for the devices Node-RED version

Summary

Both options 2 & 3 are viable:

Steve-Mcl commented 4 months ago

re-opened until https://github.com/FlowFuse/flowfuse/pull/3766 merged