Skip to content
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

Enable WebSocket Affinity in the device agent #213

Merged
merged 5 commits into from
Jan 17, 2024
Merged

Enable WebSocket Affinity in the device agent #213

merged 5 commits into from
Jan 17, 2024

Conversation

hardillb
Copy link
Contributor

@hardillb hardillb commented Dec 5, 2023

part of FlowFuse/flowfuse#2514

Description

As part of work to enable horizontal scaling of the Forge App, this allows the Device agent to use a cookie to bind to a specific instance (using Nginx Affinity mapping to start with).

Reuses the affinity cookie if present, shares the affinity cookie with the forge platform if available.

Related Issue(s)

FlowFuse/flowfuse#2514

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

  • Backport needed? -> add the backport label
  • Includes a DB migration? -> add the area:migration label

part of FlowFuse/flowfuse#2514

Reuses the affinity cookie if present, shares the affinity cookie
with the forge platform if available.
@hardillb hardillb requested a review from knolleary December 5, 2023 14:25
@hardillb hardillb self-assigned this Dec 5, 2023
@hardillb
Copy link
Contributor Author

hardillb commented Dec 5, 2023

Need to work out how to test this

@hardillb
Copy link
Contributor Author

hardillb commented Dec 5, 2023

I've used the default Cookie name here, we may want to set it to a FF specific value.

@hardillb hardillb marked this pull request as ready for review December 22, 2023 10:44
Copy link
Member

@knolleary knolleary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As proposed in https://github.com/FlowFuse/flowfuse/pull/3173/files#r1445027729 - lets use FFSESSION for the cookie name

@hardillb hardillb requested a review from knolleary January 8, 2024 17:28
socket.on('upgrade', (evt) => {
if (evt.headers && evt.headers['set-cookie']) {
const cookies = evt.headers['set-cookie']
cookies.forEach(cookie => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hardillb Are we certain that evt.headers['set-cookie'] is always going to be an array? I don't readily have the means to verify this locally.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll double check the WS code, but if I can't find a definitive answer I'll add some defensive code

@hardillb hardillb requested a review from knolleary January 16, 2024 09:51
@knolleary knolleary merged commit f3ccb21 into main Jan 17, 2024
2 checks passed
@knolleary knolleary deleted the affinity branch January 17, 2024 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants