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

server: ensure that service-defaults meta is incorporated into the discovery chain response #12511

Merged
merged 3 commits into from
Mar 30, 2022

Conversation

rboyer
Copy link
Member

@rboyer rboyer commented Mar 3, 2022

Also add a new "Default" field to the discovery chain response to clients (cc @johncowen )

TODO:

@rboyer rboyer requested review from erichaberkorn and a team March 3, 2022 17:42
@rboyer rboyer self-assigned this Mar 3, 2022
@github-actions github-actions bot added the theme/api Relating to the HTTP API interface label Mar 3, 2022
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 3, 2022 17:44 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 3, 2022 17:44 Inactive
@rboyer rboyer force-pushed the expose-service-meta-on-disco-chain branch from e94d91b to 0441e7c Compare March 3, 2022 17:50
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 3, 2022 17:50 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 3, 2022 17:50 Inactive
@rboyer rboyer changed the base branch from main to fix-disco-chain-not-found March 3, 2022 17:51
@vercel vercel bot temporarily deployed to Preview – consul March 3, 2022 22:51 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 3, 2022 22:51 Inactive
Base automatically changed from fix-disco-chain-not-found to main March 3, 2022 22:54
@rboyer rboyer force-pushed the expose-service-meta-on-disco-chain branch from 0f61568 to 65c1032 Compare March 3, 2022 22:56
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 3, 2022 22:56 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 3, 2022 22:56 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 3, 2022 23:27 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 3, 2022 23:27 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 3, 2022 23:33 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 3, 2022 23:33 Inactive
Copy link
Contributor

@erichaberkorn erichaberkorn left a comment

Choose a reason for hiding this comment

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

The service meta portion of this looks good to me 👍 I reviewed the other part too, but don't have enough context to do a proper review.

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 12, 2022

CLA assistant check
All committers have signed the CLA.

…scovery chain response

Also add a new "Default" field to the discovery chain response to clients
@rboyer rboyer force-pushed the expose-service-meta-on-disco-chain branch from 508ad93 to 64830bd Compare March 24, 2022 20:16
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 24, 2022 20:16 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 24, 2022 20:16 Inactive
Copy link
Member

@mkeeler mkeeler left a comment

Choose a reason for hiding this comment

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

Code looks great. Just the two minor things.

agent/consul/discoverychain/compile.go Outdated Show resolved Hide resolved
@@ -109,9 +109,17 @@ type CompiledDiscoveryChain struct {
// non-customized versions.
CustomizationHash string

// Default indicates if this discovery chain is based on no
Copy link
Member

Choose a reason for hiding this comment

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

Should the API docs be updated to account for these changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

IIRC we don't currently document the full disco chain response at all.

@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 25, 2022 15:52 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 25, 2022 15:52 Inactive
@rboyer rboyer requested a review from mkeeler March 25, 2022 15:54
@rboyer rboyer requested a review from a team as a code owner March 28, 2022 15:09
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 28, 2022 15:09 Inactive
@rboyer rboyer merged commit ac5bea8 into main Mar 30, 2022
@rboyer rboyer deleted the expose-service-meta-on-disco-chain branch March 30, 2022 15:04
@hc-github-team-consul-core
Copy link
Collaborator

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/617584.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/api Relating to the HTTP API interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants