-
Notifications
You must be signed in to change notification settings - Fork 24
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
ROX-25719: Receive runtime-configuration in network channel #1870
Conversation
Includes support for enabling/disabling external IPs based on runtime config, deferring to existing feature flag if none is provided
@@ -28,7 +28,7 @@ class INetworkConnectionInfoServiceComm { | |||
|
|||
virtual sensor::NetworkConnectionInfoService::StubInterface* GetStub() = 0; | |||
|
|||
virtual std::unique_ptr<IDuplexClientWriter<sensor::NetworkConnectionInfoMessage>> PushNetworkConnectionInfoOpenStream(std::function<void(const sensor::NetworkFlowsControlMessage*)> receive_func) = 0; | |||
virtual std::unique_ptr<IDuplexClientWriter<sensor::NetworkConnectionInfoMessage>> PushNetworkConnectionInfoOpenStream(std::function<void(const sensor::MsgToCollector*)> receive_func) = 0; |
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.
Hm, I think it is dangerous to change the existing message, this will break installations which are using Collector and Sensor in different versions.
Is this something we have done before like this? What was the expierence? (e.g. support cases)
Preferably we find a solution which is backward compatible.
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.
As Sensor and Collector are deployed using the same Daemonset, we usually consider that a version mismatch can only be temporary. Introducing a new method here will make Sensor reply with a "method not found" if there is any mismatch.
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.
As Sensor and Collector are deployed using the same Daemonset
Are they? Sensor has its own Deployment.
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.
@erthalion I think @ovalenti refers to the same Secured Cluster deployment, in it they share the same version. I think its better to continue the discussion in the slack thread to not loose context.
Description
Sensor is now sending RuntimeConfiguration objects controlling External-IP. This occurs over the existing network-connection channel; however the method used was renamed to introduce a new function prototype.
This PR adapts Collector to this new interface to handle the previously existing messages, and expands it by consuming the new RuntimeConfiguration object and forwarding its content in CollectorConfig.
Checklist