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

Update connect-inject with partition name #727

Merged
merged 1 commit into from
Sep 15, 2021
Merged

Update connect-inject with partition name #727

merged 1 commit into from
Sep 15, 2021

Conversation

thisisnotashwin
Copy link
Contributor

Changes proposed in this PR:

  • Update the envoy bootstrap config with the partition name so that it can correctly generate the envoy config. This was causing an error for Consul Connect
    Unexpected response code: 400 (request targets partition \"default\" which does not match agent partition \"alpha\"\n)

How I've tested this PR:

  • Unit tests
    – Deploying it to k8s and testing connect

How I expect reviewers to test this PR:

  • Code review

Checklist:

  • Tests added

@thisisnotashwin thisisnotashwin requested review from a team, ndhanushkodi and t-eckert and removed request for a team September 15, 2021 14:41
@thisisnotashwin
Copy link
Contributor Author

@freddygv this branch/PR should have the fix to the bug you found.

@thisisnotashwin thisisnotashwin marked this pull request as draft September 15, 2021 16:32
@thisisnotashwin thisisnotashwin force-pushed the update-envoy-config-with-partitions branch from bc142aa to 02a95cf Compare September 15, 2021 16:44
@thisisnotashwin thisisnotashwin changed the title Update envoy bootstrap config with partition name Update connect-inject with partition name Sep 15, 2021
@thisisnotashwin thisisnotashwin marked this pull request as ready for review September 15, 2021 16:44
t-eckert
t-eckert approved these changes Sep 15, 2021
View reviewed changes
Copy link
Contributor

@t-eckert t-eckert left a comment

Choose a reason for hiding this comment

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

Looks solid, no comments.

ndhanushkodi
ndhanushkodi approved these changes Sep 15, 2021
View reviewed changes
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.

Looks great, Ashwin! Do connect services work in non-default partitions as of this PR? 🤯 🍾

@thisisnotashwin
Copy link
Contributor Author

Looks great, Ashwin! Do connect services work in non-default partitions as of this PR? 🤯 🍾

YESSS!!!! it does!!

@thisisnotashwin thisisnotashwin merged commit 6c59d85 into partitions Sep 15, 2021
@thisisnotashwin thisisnotashwin deleted the update-envoy-config-with-partitions branch September 15, 2021 21:54
thisisnotashwin pushed a commit that referenced this pull request Sep 16, 2021
Update envoy bootstrap config with partition name (#727)
b692dd7
thisisnotashwin pushed a commit that referenced this pull request Sep 17, 2021
@thisisnotashwin
Update envoy bootstrap config with partition name (#727)
e8d10b1
lawliet89 pushed a commit to lawliet89/consul-k8s that referenced this pull request Oct 6, 2021
@lkysow
Add helm docs generation script (hashicorp#727)
6e022bf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t-eckert t-eckert t-eckert approved these changes

@ndhanushkodi ndhanushkodi ndhanushkodi 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.
3 participants
@thisisnotashwin @ndhanushkodi @t-eckert