openshift-kni / performance-addon-operators

Operators related to optimizing OpenShift clusters for applications sensitive to cpu and network latency
Apache License 2.0
46 stars 60 forks source link

Gather NRO related data #919

Closed jlojosnegros closed 2 years ago

jlojosnegros commented 2 years ago

Need to gather data regarding new NUMA resources operator but currently must-gather does not collect this data. Different approachs were discussed but in the end we have decided to collect NRO data here to avoid creating new must-gather images.

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 2564


Totals Coverage Status
Change from base Build 2558: 0.0%
Covered Lines: 1644
Relevant Lines: 2719

💛 - Coveralls
jlojosnegros commented 2 years ago

/cc @fromanirh @marioferh

jlojosnegros commented 2 years ago

nice! how was this tested?

I ran it against a cluster Shereen kindly lent to me with NRO already installed. I have also ran it against a "kind" cluster after running e2e tests but without cleaning up so it had crds instances

ffromani commented 2 years ago

nice! how was this tested?

I ran it against a cluster Shereen kindly lent to me with NRO already installed. I have also ran it against a "kind" cluster after running e2e tests but without cleaning up so it had crds instances

[EDIT already answered, moving on the next subject] ok, so we don't have e2e coverage.

ffromani commented 2 years ago

I'm trying to think how a e2e test could look like for this change. We should probably have the test running in the NROP CI (not PAO's), and should run like nightly or weekly - since it cannot run on PR changes - different repo.

[EDIT] like, for GH actions: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule maybe we can do something like this for prow

jlojosnegros commented 2 years ago

thanks, LGTM. I'll verify ASAP, but let's file a issue to track lack of e2e tests, so we can merge (and then add the e2e tests).

Issue created #920

ffromani commented 2 years ago

thanks, LGTM. I'll verify ASAP, but let's file a issue to track lack of e2e tests, so we can merge (and then add the e2e tests).

Issue created #920

I think it's more fitting for the numaresources operator, though. We're somehow halfway, but most of the resources, the pieces and the infra are more readily available on NRO CI than setting up a completely alien flow in the PAO CI.

ffromani commented 2 years ago

/override ci/prow/e2e-gcp /override ci/prow/e2e-gcp-operator-upgrade still haven't had the chance to verify

openshift-ci[bot] commented 2 years ago

@fromanirh: Overrode contexts on behalf of fromanirh: ci/prow/e2e-gcp, ci/prow/e2e-gcp-operator-upgrade

In response to [this](https://github.com/openshift-kni/performance-addon-operators/pull/919#issuecomment-1185302011): >/override ci/prow/e2e-gcp >/override ci/prow/e2e-gcp-operator-upgrade >still haven't had the chance to verify Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
ffromani commented 2 years ago

/lgtm /approve verified!

openshift-ci[bot] commented 2 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fromanirh, jlojosnegros

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: - ~~[OWNERS](https://github.com/openshift-kni/performance-addon-operators/blob/master/OWNERS)~~ [fromanirh] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment