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(manager): disable metrics server from controller runtime #473

Merged
merged 1 commit into from
Aug 11, 2020

Conversation

akhilerm
Copy link
Contributor

Signed-off-by: Akhil Mohan [email protected]

Why is this PR required? What issue does it fix?:
NDM was crashing on systems were port 8080 is already in use. NDM daemonset uses the host network and if the port 8080 on host machine is being used, NDM crashed. This was because controller-runtime by default tries to start a server at port 8080 as part of initialization, and if it fails, the initialization errors out.

What this PR does?:
Disables the default metrics server in daemonset and operator.

Does this PR require any upgrade changes?:
No

If the changes in this PR are manually verified, list down the scenarios covered::
Run the NDM daemonset on a machine were port 8080 was already in use and no crashes were observed.

Any additional information for your reviewer? :
Mention if this PR is part of any design or a continuation of previous PRs

Checklist:

  • Fixes #
  • PR Title follows the convention of <type>(<scope>): <subject>
  • Has the change log section been updated?
  • Commit has unit tests
  • Commit has integration tests
  • (Optional) Are upgrade changes included in this PR? If not, mention the issue/PR to track:
  • (Optional) If documentation changes are required, which issue on https://github.com/openebs/openebs-docs is used to track them:

latest controller runtime started a metrics server at port 8080
by default. This caused NDM to crash on systems where the port was
already in use.

Signed-off-by: Akhil Mohan <[email protected]>
@akhilerm akhilerm requested a review from kmova August 10, 2020 17:45
@akhilerm akhilerm added the bug bug to existing feature label Aug 10, 2020
@codecov-commenter
Copy link

codecov-commenter commented Aug 10, 2020

Codecov Report

Merging #473 into master will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #473   +/-   ##
=======================================
  Coverage   40.53%   40.53%           
=======================================
  Files          71       71           
  Lines        3488     3488           
=======================================
  Hits         1414     1414           
  Misses       1966     1966           
  Partials      108      108           
Impacted Files Coverage Δ
cmd/ndm_daemonset/controller/controller.go 19.14% <0.00%> (ø)

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 367ab51...c916c98. Read the comment docs.

@kmova kmova merged commit 43508b2 into openebs-archive:master Aug 11, 2020
@akhilerm akhilerm deleted the fix-port branch December 13, 2021 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bug to existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants