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

connect: skip service registration only for duplicate services on k8s #581

Merged
merged 1 commit into from
Aug 2, 2021

Conversation

ishustava
Copy link
Contributor

@ishustava ishustava commented Jul 30, 2021

Previously, we would skip service registration if we find a duplicate service
even when the 'k8s-namespace' meta key is not present on the service.
However, that is not correct behavior since that would ignore any service
instance that could exist on other platforms (e.g. VMs) and that
doesn't have this meta key. We should instead only check services that
have the 'k8s-namespace' meta key.

How I've tested this PR:

  • unit tests

How I expect reviewers to test this PR:

  • code review

Checklist:

  • Tests added (removed in this case 😄)
  • CHANGELOG entry added (HashiCorp engineers only, community PRs should not add a changelog entry)

@ishustava ishustava added type/bug Something isn't working area/connect Related to Connect service mesh, e.g. injection labels Jul 30, 2021
@ishustava ishustava requested review from a team, ndhanushkodi and kschoche and removed request for a team July 30, 2021 00:49
Copy link
Contributor

@kschoche kschoche left a comment

Choose a reason for hiding this comment

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

Looks great!

Copy link
Contributor

@ndhanushkodi ndhanushkodi left a comment

Choose a reason for hiding this comment

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

Great catch, looks good!

@ishustava ishustava force-pushed the ishustava/skip-service-reg-fix branch from 7efe643 to 67bb890 Compare August 2, 2021 18:31
…re on k8s

Previously, we would skip service registration if we find a duplicate service
even when the 'k8s-namespace' meta key is not present on the service.
However, that is not correct behavior since that would ignore any service
instance that could exist on other platforms (e.g. VMs) and that
doesn't have this meta key. We should instead only check services that
have the 'k8s-namespace' meta key.
@ishustava ishustava force-pushed the ishustava/skip-service-reg-fix branch from 67bb890 to af0b16f Compare August 2, 2021 18:32
@ishustava ishustava merged commit b87f00c into master Aug 2, 2021
@ishustava ishustava deleted the ishustava/skip-service-reg-fix branch August 2, 2021 18:47
ndhanushkodi pushed a commit to ndhanushkodi/consul-k8s that referenced this pull request Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connect Related to Connect service mesh, e.g. injection type/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants