-
Notifications
You must be signed in to change notification settings - Fork 332
[FS-1940] Start sending backend notifications through rabbitMQ and consuming them #3276
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
Changes from all commits
cecc44d
8fa3b8d
d6f2b9c
9ca845e
7a1ea7e
67483a4
93179f9
3d2f7e7
298e5ee
a122f26
f029344
8d9e69b
5c28915
6e8615e
972173b
8ec2d61
99bf675
e5726ea
cf92674
a49bf2e
aab1fda
8376ff3
e4e0cd3
75324d4
870bf9f
5125691
a348988
c81bc6b
db1b5ed
ebe5622
184def4
2eca594
1e012b0
f411c23
3f50267
49db146
43ab45f
f293252
b58139c
4d9ad6f
5eb9c44
1a7be67
38f92a7
568dbe7
5b50cde
f01f0d4
9c3181b
da343ca
d668582
2a11487
f9ab997
010c6da
ef32b84
41d4c81
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| This release introduces a new component: background-worker. This is currently | ||
| only used to forward notifications to federated backends. Enabling federation in | ||
| the wire-server helm chart automatically installs this component. | ||
|
|
||
| When federation is enabled, wire-server will require running RabbitMQ. The helm | ||
| chart in `rabbitmq` can be used to install RabbitMQ. Please refer to the | ||
| documentation at https://docs.wire.com to install RabbitMQ in Kubernetes. These | ||
| new configurations are required: | ||
|
|
||
| ```yaml | ||
| brig: | ||
| config: | ||
| rabbitmq: | ||
| host: rabbitmq | ||
| port: 5672 | ||
| vHost: / | ||
| secrets: | ||
| rabbitmq: | ||
| username: <YOUR_USERNAME> | ||
| password: <YOUR_PASSWORD> | ||
| background-worker: | ||
| config: | ||
| rabbitmq: | ||
| host: rabbitmq | ||
| port: 5672 | ||
| vHost: / | ||
| remoteDomains: [] | ||
| secrets: | ||
| rabbitmq: | ||
| username: <YOUR_USERNAME> | ||
| password: <YOUR_PASSWORD> | ||
| ``` | ||
|
|
||
| The above are the default values (except for secrets, which do not have | ||
| defaults), if they work they are not required to be configured. | ||
| `background-worker.config.remoteDomains` should contain all the remote domains | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this will change again with #3260, but let's worry about that there.
Comment on lines
+35
to
+36
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should help customers setting all the federation domain related configuration. We already have
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe #3260 is going to solve this. |
||
| with which the wire-server instance allows federating. This change is | ||
| incompatible with open-federation. | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -9,12 +9,11 @@ metadata: | |||||
| heritage: {{ .Release.Service }} | ||||||
| spec: | ||||||
| replicas: {{ .Values.replicaCount }} | ||||||
| # TODO(elland): Review this | ||||||
| strategy: | ||||||
| type: RollingUpdate | ||||||
| rollingUpdate: | ||||||
| maxUnavailable: 0 | ||||||
| maxSurge: {{ .Values.replicaCount }} | ||||||
| # Ensures only one version of the background worker is running at any given | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
is that what you mean?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, many K8s pods can run at once. This way of deploying just deletes previous version before deploying the new version. Basically does the opposite of a rolling deployment. |
||||||
| # moment. This means small downtime, but the background workers should be | ||||||
| # able to catch up. | ||||||
| type: Recreate | ||||||
| selector: | ||||||
| matchLabels: | ||||||
| app: background-worker | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,12 +9,11 @@ resources: | |
| cpu: "100m" | ||
| limits: | ||
| memory: "512Mi" | ||
| # TODO(elland): Create issue for a metrics endpoint | ||
| # FUTUREWORK: Implement metrics | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can do this in a separate PR, but we should make sure we won't forget and have a ticket: https://wearezeta.atlassian.net/browse/FS-2020 btw, which ticket is this PR about?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is this: https://wearezeta.atlassian.net/browse/FS-1940
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for creating the issue, I had asked Marco Conti to create it as I wasn't sure how to get it in right JIRA places. The one you create doesn't seem to be in the right epic. Maybe someone will notice and put it there? Is this our process?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a |
||
| # metrics: | ||
| # serviceMonitor: | ||
| # enabled: false | ||
| config: | ||
| # TODO(elland): Proper logging | ||
| logLevel: Info | ||
| logFormat: StructuredJSON | ||
| rabbitmq: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,9 @@ license-file: LICENSE | |
| build-type: Simple | ||
|
|
||
| library | ||
| -- cabal-fmt: expand src | ||
| exposed-modules: | ||
| Network.AMQP.Extended | ||
| Options.Applicative.Extended | ||
| Servant.API.Extended | ||
| Servant.API.Extended.RawM | ||
|
|
@@ -74,6 +76,7 @@ library | |
|
|
||
| build-depends: | ||
| aeson | ||
| , amqp | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this package is starting to bundle a lot of unrelated dependencies together, and inheriting them to a lot of places. but i don't think we should worry about this yet. |
||
| , base | ||
| , bytestring | ||
| , cassandra-util | ||
|
|
@@ -84,12 +87,15 @@ library | |
| , http-types | ||
| , imports | ||
| , metrics-wai | ||
| , monad-control | ||
| , optparse-applicative | ||
| , resourcet | ||
| , retry | ||
| , servant | ||
| , servant-server | ||
| , servant-swagger | ||
| , string-conversions | ||
| , text | ||
| , tinylog | ||
| , wai | ||
|
|
||
|
|
||
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.
move the yaml snippet to docs.wire.com? or if it's already there: link to it rather than redundantly copy it? on the other hand, that would complicate the situation in which we change docs.wire.com in a later release, and somebody upgrades to this release afterwards.
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.
The way docs.wire.com tells people about these options is by telling them to copy some example config from wire-server-deploy. I think that is quite useless for people doing upgrades. So, I added this yaml here. As you say it is also useful for someone doing upgrades 1 version at a time, which something could have changed in future in docs.wire.com.