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

HTTPProxy: add healthCheck port config #4761

Merged
merged 16 commits into from
Oct 28, 2022

Conversation

yangyy93
Copy link
Member

Add healthCheck port for httproute and tcproute

fix #4655
Signed-off-by: yy [yang.yang@daocloud.io]

Signed-off-by: yy <yang.yang@daocloud.io>
Signed-off-by: yy <yang.yang@daocloud.io>
Signed-off-by: yy <yang.yang@daocloud.io>
Signed-off-by: yy <yang.yang@daocloud.io>
Signed-off-by: yy <yang.yang@daocloud.io>
Signed-off-by: yy <yang.yang@daocloud.io>
Signed-off-by: yy <yang.yang@daocloud.io>
…contour into health-check-port

� Conflicts:
�	apis/projectcontour/v1/httpproxy.go
�	examples/contour/01-crds.yaml
�	examples/render/contour-deployment.yaml
�	examples/render/contour-gateway-provisioner.yaml
�	examples/render/contour-gateway.yaml
�	examples/render/contour.yaml
�	site/content/docs/main/config/api-reference.html
Signed-off-by: yy <yang.yang@daocloud.io>
@yangyy93 yangyy93 requested a review from a team as a code owner September 29, 2022 07:41
@yangyy93 yangyy93 requested review from tsaarni and skriss and removed request for a team September 29, 2022 07:41
@codecov
Copy link

codecov bot commented Sep 29, 2022

Codecov Report

Merging #4761 (717e62e) into main (0467b84) will increase coverage by 0.04%.
The diff coverage is 85.29%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4761      +/-   ##
==========================================
+ Coverage   76.07%   76.12%   +0.04%     
==========================================
  Files         140      140              
  Lines       16880    16953      +73     
==========================================
+ Hits        12842    12906      +64     
- Misses       3786     3790       +4     
- Partials      252      257       +5     
Impacted Files Coverage Δ
internal/dag/dag.go 96.62% <ø> (ø)
internal/dag/httpproxy_processor.go 92.68% <66.66%> (-0.47%) ⬇️
internal/envoy/v3/endpoint.go 95.31% <89.28%> (-4.69%) ⬇️
internal/xdscache/v3/endpointstranslator.go 88.23% <91.66%> (+0.40%) ⬆️
internal/dag/accessors.go 98.50% <100.00%> (+0.05%) ⬆️
internal/dag/gatewayapi_processor.go 94.72% <100.00%> (ø)
internal/dag/ingress_processor.go 97.07% <100.00%> (ø)
internal/workgroup/group.go 100.00% <0.00%> (ø)
...ernal/provisioner/objects/deployment/deployment.go 91.34% <0.00%> (+0.03%) ⬆️
...nternal/provisioner/objects/dataplane/dataplane.go 87.87% <0.00%> (+0.08%) ⬆️
... and 2 more

Signed-off-by: yy <yang.yang@daocloud.io>
@sunjayBhatia sunjayBhatia added the release-note/minor A minor change that needs about a paragraph of explanation in the release notes. label Oct 3, 2022
Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @yangyy93!

Please also add some documentation to https://github.com/projectcontour/contour/blob/main/site/content/docs/main/config/health-checks.md on this new feature.

apis/projectcontour/v1/httpproxy.go Outdated Show resolved Hide resolved
changelogs/unreleased/4761-yangyy93-minor.md Outdated Show resolved Hide resolved
changelogs/unreleased/4761-yangyy93-minor.md Outdated Show resolved Hide resolved
@skriss
Copy link
Member

skriss commented Oct 17, 2022

@yangyy93 just checking in to see if you'll be able to address the code review feedback above, thanks!

Signed-off-by: yy <yang.yang@daocloud.io>
Signed-off-by: yy <yang.yang@daocloud.io>
@yangyy93
Copy link
Member Author

Thank you for your comments, the relevant documents and comments have been revised.
@skriss @sunjayBhatia

@yangyy93 yangyy93 requested review from skriss and removed request for tsaarni October 18, 2022 07:43
Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @yangyy93, two more small comments.

internal/dag/accessors.go Outdated Show resolved Hide resolved
site/content/docs/main/config/health-checks.md Outdated Show resolved Hide resolved
Signed-off-by: yy <yang.yang@daocloud.io>
Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

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

LGTM, will leave for @sunjayBhatia to take another look

Copy link
Member

@sunjayBhatia sunjayBhatia left a comment

Choose a reason for hiding this comment

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

logic looks good, but some changes requested on the structure of the endpointstranslator tests

Signed-off-by: yy <yang.yang@daocloud.io>
@yangyy93
Copy link
Member Author

logic looks good, but some changes requested on the structure of the endpointstranslator tests

do you have any other suggestions? @sunjayBhatia

@skriss
Copy link
Member

skriss commented Oct 28, 2022

@sunjayBhatia I think this one's just awaiting another look from you

Copy link
Member

@sunjayBhatia sunjayBhatia left a comment

Choose a reason for hiding this comment

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

lgtm, I may make a follow up PR since that is a little easier to fix some test things

@sunjayBhatia sunjayBhatia merged commit 433fc40 into projectcontour:main Oct 28, 2022
sunjayBhatia added a commit to sunjayBhatia/contour that referenced this pull request Oct 28, 2022
Follow up to projectcontour#4761

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
@sunjayBhatia
Copy link
Member

follow up: #4828

skriss pushed a commit that referenced this pull request Oct 28, 2022
…4828)

* Fix merge issue and move test helpers to be unexported in test file
* Add 0 port test

Follow up to #4761

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor A minor change that needs about a paragraph of explanation in the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Contour supports using different ports for service health check
3 participants