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

Anycable #1859

Merged
merged 7 commits into from
Jan 25, 2025
Merged

Anycable #1859

merged 7 commits into from
Jan 25, 2025

Conversation

ryanmelt
Copy link
Member

Replaces ActionCable with Anycable

Anycable requires adding two new processes to each of our API containers.

Anycable-go is the websocket server and what initially accepts connections. It also listens for broadcasts on both the redis pub/sub stream anycable and on an http interface at port 8090.

There is also a Ruby RPC server that receives the ActionCable actions and handles the ruby part of our code. This is very similar to our original ActionCable code but did require a few tweaks to be anycable compatible. Most notably it does not support @instance variables because Channel objects are not long lived. This required switching to using class variables.

closes #1826

Copy link

codecov bot commented Jan 24, 2025

Codecov Report

Attention: Patch coverage is 15.68627% with 86 lines in your changes missing coverage. Please review.

Project coverage is 79.55%. Comparing base (e319cdb) to head (0b87910).
Report is 53 commits behind head on main.

Files with missing lines Patch % Lines
...mos-script-runner-api/app/models/running_script.rb 5.26% 36 Missing ⚠️
...r-api/app/controllers/running_script_controller.rb 0.00% 9 Missing ⚠️
...d-tlm-api/app/channels/autonomic_events_channel.rb 16.66% 5 Missing ⚠️
...md-tlm-api/app/channels/calendar_events_channel.rb 16.66% 5 Missing ⚠️
...-cmd-tlm-api/app/channels/limits_events_channel.rb 16.66% 5 Missing ⚠️
...osmos-cmd-tlm-api/app/channels/messages_channel.rb 16.66% 5 Missing ⚠️
...-cmd-tlm-api/app/channels/system_events_channel.rb 16.66% 5 Missing ⚠️
...md-tlm-api/app/channels/timeline_events_channel.rb 16.66% 5 Missing ⚠️
...-cmd-tlm-api/app/channels/config_events_channel.rb 20.00% 4 Missing ⚠️
...smos-cmd-tlm-api/app/channels/streaming_channel.rb 75.00% 2 Missing ⚠️
... and 4 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1859      +/-   ##
==========================================
- Coverage   79.56%   79.55%   -0.02%     
==========================================
  Files         517      519       +2     
  Lines       40739    40763      +24     
==========================================
+ Hits        32414    32428      +14     
- Misses       8325     8335      +10     
Flag Coverage Δ
python 84.22% <ø> (-0.04%) ⬇️
ruby-api 48.61% <16.00%> (-0.04%) ⬇️
ruby-backend 82.57% <0.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -41,7 +41,7 @@ def stop
if running_script
target_name = running_script['name'].split('/')[0]
return unless authorization('script_run', target_name: target_name)
ActionCable.server.broadcast("cmd-running-script-channel:#{params[:id]}", "stop")
running_script_publish("cmd-running-script-channel:#{params[:id]}", "stop")
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is running_script_publish defined? I only see running_script_anycable_publish

@@ -38,6 +38,17 @@
SCRIPT_API = 'script-api'
RUNNING_SCRIPTS = 'running-scripts'

def running_script_publish(channel_name, data)
Copy link
Member Author

Choose a reason for hiding this comment

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

Running script publish is here

Copy link
Member

Choose a reason for hiding this comment

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

Why isn't it all anycable?

Copy link
Member Author

Choose a reason for hiding this comment

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

The anycable stream is for broadcasting out to clients.
We also use pub/sub for communicating with running scripts and that one isn't "anycable"

Copy link
Member

@jmthomas jmthomas left a comment

Choose a reason for hiding this comment

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

So the anycable-go-linux files allow it to run directly in the container. These are precompiled? Where did you get these and how do we update them?

running = Store.smembers("running-scripts")
if running is None:
running = []
running_script_anycable_publish(
Copy link
Member

Choose a reason for hiding this comment

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

wrong indentation

Copy link
Member Author

Choose a reason for hiding this comment

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

? I don't think so.

Copy link
Member Author

Choose a reason for hiding this comment

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

??? I don't see any problem here.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry you're right ... it just looked weird to me

@@ -38,6 +38,17 @@
SCRIPT_API = 'script-api'
RUNNING_SCRIPTS = 'running-scripts'

def running_script_publish(channel_name, data)
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't it all anycable?

@@ -1,5 +1,5 @@
user healthcheck on nopass -@all +cluster|info +ping
user openc3 on #022bd57403439b2a3ec0c081cdd35d40a199bbd4ee6fc0e5113edd4fe1c10071 allkeys allchannels -@all +@read +@write +@pubsub +@connection +@transaction +info
user scriptrunner on #e808c74e210256ee7cf3ec165271544167de776d526f7fa94243e5cdcc08b0c1 resetkeys resetchannels ~running-script* ~*script-locks ~*script-breakpoints ~*openc3_log_messages &_action_cable_internal &script-api:* -@all +@read +@write +@pubsub +@hash +@connection
user scriptrunner on #e808c74e210256ee7cf3ec165271544167de776d526f7fa94243e5cdcc08b0c1 resetkeys resetchannels ~running-script* ~*script-locks ~*script-breakpoints ~*openc3_log_messages &__anycable__ &_action_cable_internal &script-api:* -@all +@read +@write +@pubsub +@hash +@connection
Copy link
Member

Choose a reason for hiding this comment

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

Will have to make sure to get this change in the project files

Copy link
Member Author

Choose a reason for hiding this comment

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

Sync should update them.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sync will update this.

@@ -73,6 +78,11 @@ RUN apk update \
yaml-dev \
&& gem update --system 3.3.14 \
&& gem install rake \
# These need to be installed with the --platform flag to avoid errors on linux-musl
# See: https://github.com/protocolbuffers/protobuf/issues/16853#issuecomment-2583135716
# Should be fixed April 2025
Copy link
Member

Choose a reason for hiding this comment

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

There's a chance this gets fixed earlier judging by the comments but I wouldn't hold my breath

Copy link
Member Author

Choose a reason for hiding this comment

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

Its affecting everyone on alpine so I hope they fix it.

rule: PathPrefix(`/openc3-api/cable`)
service: service-api-cable
priority: 10
# Route to the openc3 cmd/tlm api websockets
Copy link
Member

Choose a reason for hiding this comment

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

wrong comment, should be script api websockets

rule: PathPrefix(`/openc3-api/cable`)
service: service-api-cable
priority: 10
# Route to the openc3 cmd/tlm api websockets
Copy link
Member

Choose a reason for hiding this comment

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

comment

@ryanmelt
Copy link
Member Author

I'm currently compiling the anycable-go binaries from source to fix the go CVEs. Hopefully we can find a better way of installing in the future.

@ryanmelt ryanmelt merged commit 8990750 into main Jan 25, 2025
27 of 28 checks passed
@ryanmelt ryanmelt deleted the anycable branch January 25, 2025 17:52
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.

Switch from ActionCable to AnyCable
3 participants