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

NETOBSERV-736: Added downstream deployment configuration for monitoring collection #282

Merged
merged 1 commit into from
Mar 7, 2023

Conversation

OlivierCazade
Copy link
Contributor

Add downstream configuration boolean.

To override this boolean run: make set-release-kind-downstream

This PR does not change the HTTPS config for the operator metrics endpoint but I did not want to overload the PR, I will do another PR about it.

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Mar 1, 2023

@OlivierCazade: This pull request references NETOBSERV-736 which is a valid jira issue.

In response to this:

Add downstream configuration boolean.

To override this boolean run: make set-release-kind-downstream

This PR does not change the HTTPS config for the operator metrics endpoint but I did not want to overload the PR, I will do another PR about it.

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 kubernetes/test-infra repository.

@@ -649,6 +653,8 @@ spec:
value: quay.io/netobserv/flowlogs-pipeline:v0.1.8
- name: RELATED_IMAGE_CONSOLE_PLUGIN
value: quay.io/netobserv/network-observability-console-plugin:v0.1.9
- name: DOWNSTREAM_DEPLOYMENT
Copy link
Contributor

Choose a reason for hiding this comment

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

would DOWNSTREAM_DEPLOYMENT be set to true in CPAAS pipeline stages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the last step for this task is to update downstream build to change DOWNSTREAM_DEPLOYMENT
We already do this kind of changes for container images such as RELATED_IMAGE_CONSOLE_PLUGIN

Copy link
Contributor

Choose a reason for hiding this comment

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

got it, thanks!. Could you open a PR for downstream setting this env to true?

we'd have to test this PR post-merge cc @jotak

main.go Show resolved Hide resolved
@memodi
Copy link
Contributor

memodi commented Mar 2, 2023

/ok-to-test

@openshift-ci openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Mar 2, 2023
@github-actions
Copy link

github-actions bot commented Mar 2, 2023

New image: ["quay.io/netobserv/network-observability-operator:21c52d0"]. It will expire after two weeks.

@memodi
Copy link
Contributor

memodi commented Mar 3, 2023

@OlivierCazade - I am not sure I am testing this the right way:

  1. I modified manager.yaml to set DOWNSTREAM_DEPLOYMENT == true
  2. deployed image from this PR
  3. applied serviceMonitor as described in PR Enable operator metrics collection #273 in NS netobserv
$ oc get servicemonitor
NAME              AGE
metrics-monitor   25m

but I am not able to see metrics for NOO when I go under Observe --> Metrics on Console, could you let me know what am I missing in above procedure? Also, this PR needs a rebase.

@memodi
Copy link
Contributor

memodi commented Mar 6, 2023

thanks @OlivierCazade , this info helped:

PR #282 is about tagging the namespace so the platform monitoring stack collect the metrics and not the user workload monitoring stack.
If you want operator metrics, yes you will need both PR, but what you should see by testing 282 alone is the metrics from FLP getting collected automatically without enabling user workload monitoring.

I am able to see FLP metrics based off changes in this PR, modified DOWNSTREAM_DEPLOYMENT to set to true in manager.yaml , will re-test for downstream build

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved QE has approved this pull request label Mar 6, 2023
jotak
jotak previously approved these changes Mar 7, 2023
Copy link
Member

@jotak jotak left a comment

Choose a reason for hiding this comment

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

lgtm (not tested)

@openshift-ci openshift-ci bot added the lgtm label Mar 7, 2023
@openshift-ci openshift-ci bot removed the lgtm label Mar 7, 2023
@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Mar 7, 2023
@codecov
Copy link

codecov bot commented Mar 7, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@176f7d9). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #282   +/-   ##
=======================================
  Coverage        ?   47.38%           
=======================================
  Files           ?       43           
  Lines           ?     4947           
  Branches        ?        0           
=======================================
  Hits            ?     2344           
  Misses          ?     2395           
  Partials        ?      208           
Flag Coverage Δ
unittests 47.38% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@jotak
Copy link
Member

jotak commented Mar 7, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Mar 7, 2023
@OlivierCazade
Copy link
Contributor Author

/approve

@openshift-ci
Copy link

openshift-ci bot commented Mar 7, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: OlivierCazade

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved label Mar 7, 2023
@openshift-merge-robot openshift-merge-robot merged commit f45e21f into netobserv:main Mar 7, 2023
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

5 participants