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.

Sign up for GitHub

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

Jump to bottom

Add Peer to serviceDefaults #1853

Merged
merged 2 commits into from
Feb 3, 2023
Merged

Add Peer to serviceDefaults #1853

merged 2 commits into from
Feb 3, 2023

Conversation

DanStough
Copy link
Contributor

Changes proposed in this PR:

  • Adds a new Peer field to serviceDefaults.upstreamConig.Overrides that disambiguates overrides for local vs. peered services.
  • Fixes CRD generation for peeringDialer and peeringAcceptor

How I've tested this PR:
Unit tests.

How I expect reviewers to test this PR:
👁️ 👄 👁️

Checklist:

  • Tests added
  • CHANGELOG entry added

@DanStough DanStough requested review from a team, malizz and wilkermichael and removed request for a team January 26, 2023 23:21
@DanStough DanStough force-pushed the dans/NET-2039/add-peer-to-service-defaults-override branch from a81f573 to a084026 Compare January 26, 2023 23:24
wilkermichael
wilkermichael reviewed Jan 26, 2023
View reviewed changes
Copy link
Contributor

@wilkermichael wilkermichael left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! Great work.

I'm still kinda fresh with these kinds of changes, so going to tag another former K8s member just to double check

control-plane/api/v1alpha1/servicedefaults_types.go Show resolved Hide resolved
thisisnotashwin
thisisnotashwin approved these changes Jan 27, 2023
View reviewed changes
Copy link
Contributor

@thisisnotashwin thisisnotashwin left a comment

Choose a reason for hiding this comment

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

This is delightful! Left a comment about a missing scenario but I don't need to do another review after that!

control-plane/api/v1alpha1/servicedefaults_types.go Show resolved Hide resolved
hack/copy-crds-to-chart/main.go Show resolved Hide resolved
@DanStough DanStough force-pushed the dans/NET-2039/add-peer-to-service-defaults-override branch from a084026 to 55af66b Compare January 27, 2023 16:43
@DanStough
Copy link
Contributor Author

DanStough commented Jan 27, 2023 •
edited
Loading

I will hold off merging this until the API changes are in Consul main.

@DanStough DanStough force-pushed the dans/NET-2039/add-peer-to-service-defaults-override branch from 55af66b to 60a7424 Compare February 3, 2023 15:57
@DanStough DanStough force-pushed the dans/NET-2039/add-peer-to-service-defaults-override branch from 60a7424 to d2b8c9c Compare February 3, 2023 15:57
@DanStough DanStough merged commit f562a09 into main Feb 3, 2023
@DanStough DanStough deleted the dans/NET-2039/add-peer-to-service-defaults-override branch February 3, 2023 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thisisnotashwin thisisnotashwin thisisnotashwin approved these changes

@malizz malizz malizz approved these changes

@wilkermichael wilkermichael wilkermichael approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DanStough @thisisnotashwin @malizz @wilkermichael