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

Implement periodic gathering as a job in tech preview #787

Merged
merged 5 commits into from Jun 21, 2023

Conversation

tremes
Copy link
Contributor

@tremes tremes commented May 31, 2023

This reopen of #764 with adding resource requests. Please check the description of the original PR.

Categories

  • Bugfix
  • Data Enhancement
  • Feature
  • Backporting
  • Others (CI, Infrastructure, Documentation)

Sample Archive

no new data

Documentation

  • path/to/documentation.md

Unit Tests

  • path/to/file_test.go

Privacy

Yes. There are no sensitive data in the newly collected information.

Changelog

Breaking Changes

Yes/No

References

https://issues.redhat.com/browse/???
https://access.redhat.com/solutions/???

@openshift-ci openshift-ci bot requested review from ncaak and rluders May 31, 2023 12:11
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 31, 2023
@openshift-ci
Copy link

openshift-ci bot commented May 31, 2023

@tremes: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test e2e
  • /test e2e-agnostic-upgrade
  • /test e2e-metal-ipi-ovn-ipv6
  • /test images
  • /test insights-operator-e2e-tests
  • /test lint
  • /test unit

Use /test all to run all jobs.

In response to this:

/test e2e-aws-ovn-techpreview

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.

@openshift-ci
Copy link

openshift-ci bot commented May 31, 2023

@tremes: trigger 0 job(s) for the /payload-(job|aggregate) command

@tremes
Copy link
Contributor Author

tremes commented May 31, 2023

/payload-job periodic-ci-openshift-multiarch-master-nightly-4.14-ocp-e2e-aws-ovn-arm64-techpreview

@openshift-ci
Copy link

openshift-ci bot commented May 31, 2023

@tremes: trigger 0 job(s) for the /payload-(job|aggregate) command

1 similar comment
@openshift-ci
Copy link

openshift-ci bot commented May 31, 2023

@tremes: trigger 0 job(s) for the /payload-(job|aggregate) command

@DennisPeriquet
Copy link
Contributor

/payload-job periodic-ci-openshift-release-master-ci-4.14-e2e-aws-sdn-techpreview

@openshift-ci
Copy link

openshift-ci bot commented May 31, 2023

@DennisPeriquet: trigger 1 job(s) for the /payload-(job|aggregate) command

  • periodic-ci-openshift-release-master-ci-4.14-e2e-aws-sdn-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/63a82bc0-ffe6-11ed-8a6a-1f1592a5945d-0

@tremes
Copy link
Contributor Author

tremes commented Jun 1, 2023

/test e2e-gcp-ovn-techpreview

1 similar comment
@tremes
Copy link
Contributor Author

tremes commented Jun 1, 2023

/test e2e-gcp-ovn-techpreview

@tremes
Copy link
Contributor Author

tremes commented Jun 2, 2023

/retest

@tremes
Copy link
Contributor Author

tremes commented Jun 5, 2023

/payload-job periodic-ci-openshift-release-master-ci-4.14-e2e-aws-sdn-techpreview

@openshift-ci
Copy link

openshift-ci bot commented Jun 5, 2023

@tremes: trigger 1 job(s) for the /payload-(job|aggregate) command

  • periodic-ci-openshift-release-master-ci-4.14-e2e-aws-sdn-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/51e00d40-03a1-11ee-9c9b-32826c43919b-0

Copy link
Contributor

@ncaak ncaak left a comment

Choose a reason for hiding this comment

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

/lgtm

Although there are couple of places like gather_commands Gather function and operator Run function which contain high complexity (and more than 100 lines), I didn't find any dead-end.
Maybe it would be a good idea to revisit this later and try to refactor some parts and include some unit tests to them (currently seems impossible)

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 9, 2023
@openshift-ci
Copy link

openshift-ci bot commented Jun 9, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ncaak, tremes

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

@tremes
Copy link
Contributor Author

tremes commented Jun 12, 2023

Thank you @ncaak. Yes you're right. There's a follow-up prepared for the https://issues.redhat.com/browse/CCXDEV-10590 and it will hopefully refactor few things.

@neisw
Copy link
Contributor

neisw commented Jun 12, 2023

/payload-job periodic-ci-openshift-release-master-ci-4.14-e2e-aws-sdn-techpreview

@openshift-ci
Copy link

openshift-ci bot commented Jun 12, 2023

@neisw: trigger 1 job(s) for the /payload-(job|aggregate) command

  • periodic-ci-openshift-release-master-ci-4.14-e2e-aws-sdn-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/4c391050-0917-11ee-8ef7-f4567af4c5ab-0

@neisw
Copy link
Contributor

neisw commented Jun 12, 2023

/test e2e-gcp-ovn-techpreview

tremes and others added 4 commits June 13, 2023 08:01
* Run gathering as separate Pod in tech preview

* Move downloading of reports to operator part & propagate Insights Request ID

* Minor updates

* DataGather CR - very first  commit

* create a new DataGather CR and prune periodically

* read and apply gatherers config from the new CR

* Fix anonymizer

* do not duplicate gatherer status creation

* extract the job responsibility and fix contexts

* Copy Gatherer status & tests

* diskrecorder_test - narrow down the testing archive path

* Fix error reporting in periodic.go

* reorder manifest creation experiment

* status reporter must be always started for now

* rebase
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 13, 2023
@rhrmo
Copy link
Contributor

rhrmo commented Jun 13, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 13, 2023
@tremes
Copy link
Contributor Author

tremes commented Jun 13, 2023

/retest

2 similar comments
@tremes
Copy link
Contributor Author

tremes commented Jun 14, 2023

/retest

@tremes
Copy link
Contributor Author

tremes commented Jun 15, 2023

/retest

@deepsm007
Copy link
Contributor

@tremes We are concerned about the e2e-gcp-ovn-techpreview failing job. Can we confirm if this is not an issue for merging the PR?

@tremes
Copy link
Contributor Author

tremes commented Jun 15, 2023

Hi @deepsm007, I realized that our current testsuite will not pass with this tech preview upgrade. So when I am looking at the last https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_insights-operator/787/pull-ci-openshift-insights-operator-master-e2e-gcp-ovn-techpreview/1669219468393320448 then I think the failures are expected. Thanks and sorry for this inconvenience.

The periodic-ci-openshift-release-master-ci-4.14-e2e-aws-sdn-techpreview should hopefully be more indicative, because there have been already one problem (test failure - related to 0668659) that required revert of this.

@deepsm007
Copy link
Contributor

/label jira/valid-bug

@openshift-ci openshift-ci bot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Jun 15, 2023
@tremes
Copy link
Contributor Author

tremes commented Jun 15, 2023

/test ci/prow/e2e-agnostic-upgrade

@openshift-ci
Copy link

openshift-ci bot commented Jun 15, 2023

@tremes: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test e2e
  • /test e2e-agnostic-upgrade
  • /test e2e-metal-ipi-ovn-ipv6
  • /test images
  • /test insights-operator-e2e-tests
  • /test lint
  • /test unit

The following commands are available to trigger optional jobs:

  • /test e2e-gcp-ovn-techpreview

Use /test all to run all jobs.

In response to this:

/test ci/prow/e2e-agnostic-upgrade

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.

@tremes
Copy link
Contributor Author

tremes commented Jun 15, 2023

/test e2e-agnostic-upgrade

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD c0f05d3 and 2 for PR HEAD 3b100a3 in total

@tremes
Copy link
Contributor Author

tremes commented Jun 16, 2023

/test e2e

@openshift-ci
Copy link

openshift-ci bot commented Jun 16, 2023

@tremes: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-gcp-ovn-techpreview 3b100a3 link false /test e2e-gcp-ovn-techpreview

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@JoaoFula
Copy link
Contributor

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Jun 19, 2023
@tremes
Copy link
Contributor Author

tremes commented Jun 21, 2023

/test e2e

@openshift-merge-robot openshift-merge-robot merged commit bb72dd0 into openshift:master Jun 21, 2023
8 of 9 checks passed
dgoodwin added a commit to dgoodwin/insights-operator that referenced this pull request Jun 22, 2023
tremes added a commit to tremes/insights-operator that referenced this pull request Jun 23, 2023
openshift-merge-robot pushed a commit that referenced this pull request Jun 27, 2023
)

* Revert "Revert "Implement periodic gathering as a job in tech preview (#787)" (#798)"

This reverts commit cd80172.

* add priorityClass to the new Pods
@tremes tremes deleted the gather-job-tp-new branch July 10, 2023 10:14
JoaoFula pushed a commit to JoaoFula/insights-operator that referenced this pull request Jan 23, 2024
* Implement periodic gathering as a job in tech preview (openshift#764)

* Run gathering as separate Pod in tech preview

* Move downloading of reports to operator part & propagate Insights Request ID

* Minor updates

* DataGather CR - very first  commit

* create a new DataGather CR and prune periodically

* read and apply gatherers config from the new CR

* Fix anonymizer

* do not duplicate gatherer status creation

* extract the job responsibility and fix contexts

* Copy Gatherer status & tests

* diskrecorder_test - narrow down the testing archive path

* Fix error reporting in periodic.go

* reorder manifest creation experiment

* status reporter must be always started for now

* rebase

* add resource requests to the new job

* rebase

* pass linting

* rebase
JoaoFula pushed a commit to JoaoFula/insights-operator that referenced this pull request Jan 23, 2024
JoaoFula pushed a commit to JoaoFula/insights-operator that referenced this pull request Jan 23, 2024
…penshift#799)

* Revert "Revert "Implement periodic gathering as a job in tech preview (openshift#787)" (openshift#798)"

This reverts commit cd80172.

* add priorityClass to the new Pods
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged. qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants