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

Allows service targetPort modificaion #2789

Merged
merged 2 commits into from
Nov 5, 2020

Conversation

bewiwi
Copy link
Contributor

@bewiwi bewiwi commented Oct 21, 2020

What this PR does / why we need it:
For now, it's appears that It's not possible to redirect request to loki service to a sidecar to do some HTTP authentication.

In the values.yml we have an example of sidecar to do a reverse proxy. We can add extraPorts that refer to this sidecar but it's not possible to update the targetPort in the loki service definition because the port name is hardcoded (http-metrics).
In conclusion, we can't use the sidecar as a proxy for requests coming to this service.

This PR will allow the modification of the loki targetPort to be able to define an other one

Special notes for your reviewer:
Nothing special, just move an hardcoded value to values.yaml

Checklist

  • Documentation added
  • Tests updated

@bewiwi bewiwi marked this pull request as ready for review October 21, 2020 15:06
@codecov-io
Copy link

Codecov Report

Merging #2789 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2789      +/-   ##
==========================================
- Coverage   61.42%   61.39%   -0.03%     
==========================================
  Files         177      177              
  Lines       14373    14373              
==========================================
- Hits         8828     8825       -3     
- Misses       4738     4740       +2     
- Partials      807      808       +1     
Impacted Files Coverage Δ
pkg/promtail/targets/file/filetarget.go 64.08% <0.00%> (-2.12%) ⬇️

@bewiwi bewiwi changed the title All modifservice port Allows service targerPort modificaion Oct 21, 2020
@bewiwi bewiwi changed the title Allows service targerPort modificaion Allows service targetPort modificaion Oct 21, 2020
@bewiwi bewiwi force-pushed the fix/helm/ingress_service_port branch from 60c6ada to e69ac90 Compare October 26, 2020 12:39
Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

@cyriltovena cyriltovena merged commit 3b89624 into grafana:master Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants