-
Notifications
You must be signed in to change notification settings - Fork 9
feat(installer): Device Agent Linux installer #388
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Why is this in golang rather than a native script? I may have missed if that choice had been discussed somewhere. If we're asking users to download and run a script from the internet, it's generally better for that to be a script rather than a closed binary they can't check before it is run. I'm not familiar with golang, so a code review is going to take some time to learn what/how its working. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request introduces the FlowFuse Device Agent Installer scripts written in Golang to enable a streamlined installation process for the Device Agent package on Linux systems. Key changes include new utilities for permissions and directory creation, systemd service configuration and management, and Node.js-based package installation and configuration.
Reviewed Changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
installer/go/pkg/utils/utils.go | Added Unix-specific permission and directory management functions |
installer/go/pkg/service/templates.go | Introduced systemd service template |
installer/go/pkg/service/service.go | Added service wrapper functions for install, start, stop, etc. |
installer/go/pkg/service/linux.go | Implemented Linux-specific service installation and uninstallation |
installer/go/pkg/nodejs/deviceagent.go | Added Node.js integration for installing, uninstalling and configuring the agent |
installer/go/pkg/logger/logger.go | Added a configurable logger with file and console outputs |
installer/go/pkg/config/config.go | Implemented installer configuration save/load functionality |
installer/go/main.go | Main entry point that parses flags and drives installation/uninstallation |
installer/go/cmd/install.go | High-level installation and uninstallation orchestration |
Files not reviewed (2)
- installer/go/build.sh: Language not supported
- installer/go/go.mod: Language not supported
To have a proper cross-platform solution. With native scripts, we will have to maintain 2 separate scripts - one for Unix systems (and for sure there will be plenty of case/if functions to have support for both Linux and macOS), and second, in Powershell, for Windows os.
I mentioned it a couple of times during our weekly meetings, with similar clarification, as above, on why we should go this way instead of regular shell scripts. No objections were made.
The code remains open, as a part of this public repo, and can be verified anytime by anyone.
I left proper (I hope) descriptions and comments to improve readability. |
My apologies for missing it when you brought it up previously.
That makes sense - and actually, is a good reason to go this way. |
@knolleary to clarify the above sentence - we will have a single binary for each system/architecture variation, so the result of of "build and release" pipeline will be similar to this release: https://github.com/FalcoSuessgott/golang-cli-template/releases/tag/v0.3.0 . |
Initial review conducted with Piotr. Points of note
Try out failures noted (installing on arm (PI5))
Untested
Other points (for the memory banks when doing windows)
|
Probably yes - see this thread on discourse |
Added support for sysvinit
Improved how
Added basic documentation
This is not the scope of the install script. I've added basic development information to the readme file. |
I think we have crossed wires. I am referring to the tools needed for any contrib nodes that have a binary element (like the serial port nodes). If a contrib node has a compile step it will fail likely fail due to not having |
Morning @ppawlowski I have tested latest version & can now import existing flows. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor nitpick (feel free to ignore)
installer/go/README.md
Outdated
# Clone and navigate to the installer directory | ||
cd installer/go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need the git clone ...
here (or are you leaving that implicit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added explicit clone command.
Although approved, please note, I would like a 2nd set of eyes on (I have been very close to this, so may no longer be impartial ;) ) |
@hardillb can I ask for another review, please? |
@ppawlowski just checking, I'll need to build this myself to test it? (not a problem, just checking) |
Yeah Ben, need to build. (the readme has good info) |
I ran a test build using the golang docker container
And it appears to have build the linux variants in the |
I pointed this out too but @ppawlowski has said, once all branches are merged, them too will be populated (the other PRs he has populate all dirs) It might be wise to start your testing on a decendant branch e.g. PR #413 - which is based off this. |
That's expected - this PR contains only Linux-related stuff. |
Doesn't work on debian 11 or ubuntu 20.04 (both are technically EoL for standard LTS) Basic install appears to work correctly on Ubuntu 22.04
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basic testing done, looks good.
Will review other features in their branches
I was able to install the device agent, using this installer, on:
|
This was with building the install in the golang:latest docker container hardillb@flowforge:~$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 20.04.6 LTS
Release: 20.04
Codename: focal
hardillb@flowforge:~$ cd device-agent/
hardillb@flowforge:~/device-agent$ ls
CHANGELOG.md CODE_OF_CONDUCT.md CONTRIBUTING.md docker frontend index.js installer lib LICENSE package.json package-lock.json README.md service test
hardillb@flowforge:~/device-agent$ cd installer/go/out/linux/
hardillb@flowforge:~/device-agent/installer/go/out/linux$ ./flowfuse-device-installer-linux-amd64
./flowfuse-device-installer-linux-amd64: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.34' not found (required by ./flowfuse-device-installer-linux-amd64)
./flowfuse-device-installer-linux-amd64: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.32' not found (required by ./flowfuse-device-installer-linux-amd64) |
I am not able to reproduce this error on Ubuntu 20.04 LTS. Build made on my macOS. I've created a separate issue to investigate this once we start build and release via a proper CI/CD pipeline. |
Description
This pull request introduces the FlowFuse Device Agent Installer scripts. Written in Golang, on first iteration it aims to provide smooth installation experience for installing the Device Agent package on Linux systems.
Related Issue(s)
#382
Checklist
flowforge.yml
?FlowFuse/helm
to update ConfigMap TemplateFlowFuse/CloudProject
to update values for Staging/ProductionLabels
area:migration
label