NETOBSERV-1532: add TLS support to ebpf agent metrics config #305
Conversation
|
@msherif1234: This pull request references NETOBSERV-1532 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
msherif1234
force-pushed
the
add_metrics_tls
branch
from
3450ec1
to
aa041de
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #305 +/- ##
==========================================
- Coverage 34.04% 34.01% -0.03%
==========================================
Files 47 47
Lines 3836 3845 +9
==========================================
+ Hits 1306 1308 +2
- Misses 2444 2449 +5
- Partials 86 88 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
pkg/agent/agent.go
Outdated
| @@ -153,6 +153,10 @@ func FlowsAgent(cfg *Config) (*Flows, error) { | |||
| PromConnectionInfo: metrics.PromConnectionInfo{ | |||
| Address: cfg.MetricsServerAddress, | |||
| Port: cfg.MetricsPort, | |||
| TLS: &metrics.PromTLS{ | |||
There was a problem hiding this comment.
defined as such, PromConnectionInfo.TLS will never be nil; however, below you're checking with
if conn.TLS != nil {
err = httpServer.ListenAndServeTLS(conn.TLS.CertPath, conn.TLS.KeyPath)
}
So I guess you should check for empty cfg.MetricsTLSCertPath instead?
jotak
added
the
ok-to-test
|
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=edd4cb8 make set-agent-image |
msherif1234
force-pushed
the
add_metrics_tls
branch
from
aa041de
to
6384c0a
Compare
github-actions
bot
removed
the
ok-to-test
|
/ok-to-test |
openshift-ci
bot
added
the
ok-to-test
|
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=b466e4a make set-agent-image |
|
@msherif1234 - I tried to enable metrics with TLS with below config in flowcollector: metrics:
enable: true
server:
port: 9090
tls:
insecureSkipVerify: false
type: Autoebpf pods are landing in error state: |
@memodi there was missing mounts in the operator side I just updated the operator PR to do the proper mounts |
Signed-off-by: Mohamed Mahmoud <[email protected]>
msherif1234
force-pushed
the
add_metrics_tls
branch
from
6384c0a
to
68f00d3
Compare
github-actions
bot
removed
the
ok-to-test
|
/ok-to-test |
openshift-ci
bot
added
the
ok-to-test
|
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=e418bc9 make set-agent-image |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #305 +/- ##
==========================================
- Coverage 34.04% 33.84% -0.21%
==========================================
Files 47 47
Lines 3836 3847 +11
==========================================
- Hits 1306 1302 -4
- Misses 2444 2456 +12
- Partials 86 89 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
/label qe-approved |
|
@msherif1234: This pull request references NETOBSERV-1532 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: msherif1234 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
Add the ability to use TLS for the metrics server,
Dependencies
n/a
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.