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

fix(eventbus): set nats routes with pod DNS names. Fixes #1026 #1033

Merged
merged 1 commit into from
Jan 23, 2021

Conversation

whynowy
Copy link
Member

@whynowy whynowy commented Jan 23, 2021

Signed-off-by: Derek Wang [email protected]

Checklist:

Fixes #1026

Old config:

http: 8222
include ./auth.conf
cluster {
  port: 6222
  routes [
   nats://eventbus-default-stan-svc:6222
  ]
  cluster_advertise: $CLUSTER_ADVERTISE
  connect_retries: 10
}
streaming {
  xxxxxx
}

New config:

http: 8222
include ./auth.conf
cluster {
  port: 6222
  routes: [
   nats://eventbus-default-stan-0.eventbus-default-stan-svc.argo-events.svc:6222,nats://eventbus-default-stan-1.eventbus-default-stan-svc.argo-events.svc:6222,nats://eventbus-default-stan-2.eventbus-default-stan-svc.argo-events.svc:6222
  ]
  cluster_advertise: $CLUSTER_ADVERTISE
  connect_retries: 10
}
streaming {
  xxxxx
}

Tested in a real environment, routes change in the configmap will be gradually picked up when the NATS pods restart, and there's no impact even when mixed old and new config are used by different replicas.

Copy link
Contributor

@alexec alexec left a comment

Choose a reason for hiding this comment

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

note that my reviews assume that you understand the topic, and you've written the requisite tests - I don't do QA on PRs for people

@whynowy whynowy merged commit 4c93e07 into argoproj:master Jan 23, 2021
@whynowy whynowy deleted the nats_conf branch January 23, 2021 19:56
juliev0 pushed a commit to juliev0/argo-events that referenced this pull request Mar 29, 2022
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.

Eventbus NATS routes
3 participants