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: use cluster domain neutral svc dns #2655

Merged
merged 1 commit into from
Jun 22, 2023
Merged

Conversation

igor-enso
Copy link
Contributor

@igor-enso igor-enso commented Jun 12, 2023

#2657

Current implementation of NATS eventbus assumes the cluster has 'cluster.local' domain defined, which not always the case.

Solution

Use cluster domain invariant service name - evenbus-name.eventbus-namespace.svc instead of evenbus-name.eventbus-namespace.svc.cluster.local

Checklist:

@igor-enso igor-enso force-pushed the master branch 2 times, most recently from a0ecf1a to c7690de Compare June 15, 2023 10:28
@igor-enso igor-enso changed the title fix: add cluster domain neutral svc dns name fix: use cluster domain neutral svc dns Jun 15, 2023
@igor-enso igor-enso marked this pull request as ready for review June 15, 2023 10:36
@igor-enso igor-enso changed the title fix: use cluster domain neutral svc dns fix: use cluster domain neutral svc dns [#2657] Jun 15, 2023
@igor-enso igor-enso changed the title fix: use cluster domain neutral svc dns [#2657] fix: use cluster domain neutral svc dns (#2657) Jun 15, 2023
@igor-enso igor-enso changed the title fix: use cluster domain neutral svc dns (#2657) fix: use cluster domain neutral svc dns Jun 15, 2023
@igor-enso igor-enso force-pushed the master branch 3 times, most recently from 7f60eb1 to b07cab3 Compare June 15, 2023 13:53
Copy link
Member

@jmeridth jmeridth left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@igor-enso
Copy link
Contributor Author

@jmeridth can i merge this PR? (still not permitted, although approved)

@jmeridth
Copy link
Member

@igor-enso you and I do not have permissions. I'm going to add authorized folks as reviewers. I also created #2658 to hopefully alleviate this in the future.

@igor-enso
Copy link
Contributor Author

@jmeridth Thank you!

@igor-enso igor-enso force-pushed the master branch 3 times, most recently from 648f25a to 62b9b04 Compare June 19, 2023 08:55
Copy link
Member

@whynowy whynowy left a comment

Choose a reason for hiding this comment

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

Thanks, the rest looks good to me!

USERS.md Outdated Show resolved Hide resolved
Signed-off-by: Igor Makhtes <igor@enso.security>
@whynowy whynowy merged commit 5adbc75 into argoproj:master Jun 22, 2023
@ghost
Copy link

ghost commented Jun 22, 2023

Best pr ever.

whynowy pushed a commit that referenced this pull request Aug 31, 2023
Signed-off-by: Igor Makhtes <igor@enso.security>
@srinivin
Copy link

srinivin commented Sep 26, 2023

Noticed a bug : #2824

Not sure completely if this is a expected behviour, however if i rollback this PR changes in my local, not seeing the bug.

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.

4 participants