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

Fix healthcheck ports #2378

Merged
merged 1 commit into from
Feb 11, 2022
Merged

Fix healthcheck ports #2378

merged 1 commit into from
Feb 11, 2022

Conversation

ciarams87
Copy link
Contributor

Proposed changes

Don't set the upstream port as the default port for healthchecks - healthchecks will default to the port of the server instead (see https://nginx.org/en/docs/http/ngx_http_upstream_hc_module.html#health_check).

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto master
  • I will ensure my PR is targeting the master branch and pulling from my branch from my own fork

@github-actions github-actions bot added bug An issue reporting a potential bug documentation Pull requests/issues for documentation labels Jan 24, 2022
@ciarams87 ciarams87 force-pushed the fix-healthcheck-port branch from cff1708 to fb7ba08 Compare January 25, 2022 09:05
@ciarams87 ciarams87 force-pushed the fix-healthcheck-port branch from 4c2fbcf to cec916e Compare January 26, 2022 09:37
@codecov-commenter
Copy link

Codecov Report

Merging #2378 (cec916e) into master (7ffcd5c) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2378      +/-   ##
==========================================
+ Coverage   53.66%   53.68%   +0.02%     
==========================================
  Files          48       48              
  Lines       14201    14199       -2     
==========================================
+ Hits         7621     7623       +2     
+ Misses       6340     6338       -2     
+ Partials      240      238       -2     
Impacted Files Coverage Δ
internal/configs/version2/http.go 0.00% <ø> (ø)
internal/configs/transportserver.go 93.84% <100.00%> (-0.04%) ⬇️
internal/configs/virtualserver.go 95.32% <100.00%> (-0.01%) ⬇️
internal/k8s/configuration.go 95.86% <0.00%> (+0.38%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7efc0e7...cec916e. Read the comment docs.

@ciarams87 ciarams87 force-pushed the fix-healthcheck-port branch from cec916e to 737bf1a Compare February 7, 2022 13:17
@ciarams87 ciarams87 force-pushed the fix-healthcheck-port branch from 737bf1a to ed45743 Compare February 7, 2022 13:19
@ciarams87 ciarams87 requested a review from haywoodsh February 7, 2022 15:06
@ciarams87 ciarams87 merged commit f239812 into master Feb 11, 2022
@ciarams87 ciarams87 deleted the fix-healthcheck-port branch February 11, 2022 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue reporting a potential bug documentation Pull requests/issues for documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants