-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
DigitalOcean Version Doesn't Have Proper Annotations #8965
Comments
/remove-kind bug @josiahbryan But there is no proof like screenshots and configurations-snippets from a cluster that has these annotations and shows the metrics. Are you talking about the graphs of DO service or are you talking about graphs on Grafana/Prometheus etc ? Also, do all DO users want these annotations ? I am not convinced that all users of DigitalOcean would want these annotations configured out of the box from the project. |
Without these annotations, graphs on the digital ocean dashboard for the LB
show "No Data" on all the HTTP-related graphs. I would imagine that all
users would expect and want data for graphs since this is an
HTTP(S|2)-centric project (e.g. nginx) and the DO config is being offered.
Without the annotations specifying the HTTP ports, the graphs look
something like the attached screenshot. Note the "No Data".
After going round and round with DO support, they finally figured out it
was missing those annotations. Happy to provide the ticket that we
interacted on, but they linked me to those docs in the YAML comments
anyway, which document this requirement.
It's just super annoying that no mention of this is anywhere in the nginx
ingress docs (this project) because out of the box, the DO LB looks
"broken" in the graphs it's reporting, even though data flows. With the
annotations, data shows in those graphs. It's that simple. I really don't
care if you update the docs or not, I know now. I just submit this to help
future users. I do believe all users who use DO with this project would
DEFINITELY want to see data here. What do you think?
[image: image.png]
…On Tue, Aug 23, 2022 at 10:21 PM Long Wu Yuan ***@***.***> wrote:
/remove-kind bug
/kind feature
@josiahbryan <https://github.com/josiahbryan>
I think this requires editing
https://github.com/kubernetes/ingress-nginx/blob/main/hack/manifest-templates/provider/do/values.yaml
and adding the new currently non-existing other annotations there. Please
free to submit a PR.
But there is no proof like screenshots and configurations-snippets from a
cluster that has these annotations and shows the metrics. Are you talking
about the graphs of DO service or are you talking about graphs on
Grafana/Prometheus etc ?
Also, do all DO users want these annotations ? I am not convinced that all
users of DigitalOcean would want these annotations configured out of the
box from the project.
—
Reply to this email directly, view it on GitHub
<#8965 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABEZELBDLMECI5HVJ3Z2C7LV2WILVANCNFSM57NNHIPA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Josiah Bryan
+1-765-215-0511 (Phone/SMS/WhatsApp)
www.josiahbryan.com <https://www.josiahbryan.com/?utm_source=sig>
***@***.***
|
Thank you |
There is no cost implications. I'll let you submit the PR. I'm reporting this as an issue because it makes the software look broken out of the box. Good luck
and have fun!
…On Wed, Aug 24, 2022, 12:45 AM Long Wu Yuan ***@***.***> wrote:
- Are there cost implications to running those metrics ?
- Multiple DO users did not ask for those annotations to be available
out of the box so do some users of DO prefer not having those annotations
out of the box because there integrated observability like
prometheus+grafana give them the info those annotations provide. This we do
not know and maybe have to ask/survey.
- When the DO provider was added and the DO specific values file
created, did the contributor knowingly not add those annotations for above
mentioned reasons. We don't know. And its hard to find out.
- In any case, please feel free to submit a PR to add those
annotations to the DO specific values file, as I mentioned in my previous
post. If the approvers think, its ok then it will be merged, and next time
we generate static manifest for DO, the service object will have those
annotations
- The idea is not to block a change but to populate the issue with
enough information to make better decisions. If there is cost implication
and users have their own observability, then forcing this on users will not
be an improvement, hence my comments
Thank you
—
Reply to this email directly, view it on GitHub
<#8965 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABEZELB6PRVECTIJDMMHKCTV2WZHDANCNFSM57NNHIPA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
ok thanks. I will submit the PR |
Need to get specific on
Can you point me to a link for that annotation. Can't put "whatever" as value |
… On Wed, Aug 24, 2022, 12:54 AM Long Wu Yuan ***@***.***> wrote:
Need to get specific on
service.beta.kubernetes.io/do-loadbalancer-name: "<whatever>"
Can you point me to a link for that annotation. Can't put "whatever" as
value
—
Reply to this email directly, view it on GitHub
<#8965 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABEZELGJVHV2KVU6EXAL4T3V2W2LFANCNFSM57NNHIPA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Also, this one
I guess this one is for passthrough on the LB so TLS terminates on controller. Is your opinion that all users would want this |
I see that the existing LB will be renamed and also not using this annotation means it will set a name like Now, to me it seems that, an improvement would be to write about the usage of these additional annotations in docs, so that users can edit the service to their taste, after installation, as compared to forcing a name etc |
/triage accepted |
hi all. i have added service.beta.kubernetes.io/do-loadbalancer-enable-proxy-protocol: "true" in service file & use-proxy-protocol: "true" in config file because i have to whitelist some IP.. but when i apply this changes thers is all my api going to fall down and showing error---- Caused by: org.springframework.web.client.HttpClientErrorException: 500 I/O error on POST request for "https://api-ab.xyz token": Connection reset; nested exception is java.net.SocketException: Connection reset |
What happened:
No data shows for HTTP response times on DO LB Graphs
What you expected to happen:
Wrong annotations
NGINX Ingress controller version (exec into the pod and run nginx-ingress-controller --version.):
ingress-nginx_controller-v1.2.1-do.yml
Proper annotations should be included with the YAML. Here's the right annotations to include:
The text was updated successfully, but these errors were encountered: