Implement periodic gathering as a job in tech preview #764

tremes · 2023-04-05T10:40:44Z

This changes the periodic data gathering to run as a Kubernetes jobs. This includes using and implementing a new datagather.insights.openshift.io custom resource. A new NewGatherAndUpload command was added to pkg/cmd/start/start.go, but it's really implemented in pkg/controller/gather_commands.go (which is not really new, but renamed from gather_job.go.

The Kubernetes job is created in the pkg/controller/periodic/job.go and is responsible for the following:

run the data gathering
recording the data as an archive - this is the change in pkg/recorder/diskrecorder/diskrecorder.go - we are only interested in the last archive (simply because there won't be more)
uploading the data - these are the changes in pkg/insights/insightsuploader/insightsuploader.go and pkg/insights/insightsclient/requests.go, because we need to know and use the insightsRequestID

Job is executed periodically by the new method GatherJob() in pkg/controller/periodic/periodic.go. Each job also requires a new DataGather CR (created in pkg/controller/periodic/periodic.go by method createNewDataGatherCR(ctx context.Context, disabledGatherers []string, dataPolicy insightsv1alpha1.DataPolicy)). This custom resource reflects the same values, which user can configure in insightsdatagather.config.openshift.io resource) and this is the responsibility of the createDataGatherAttributeValues() ([]string, insightsv1alpha1.DataPolicy) method in pkg/controller/periodic/periodic.go.

Finally the gatherers status needs to "copied" from the corresponding DataGather resource to the insightsoperator.operator.openshift.io resource - this is done in copyDataGatherStatusToOperatorStatus(ctx context.Context, dgName string) (*v1.InsightsOperator, error) in pkg/controller/periodic/periodic.go.

Sample Archive

No new data

Documentation

No docs update yet

Unit Tests

pkg/controller/periodic/job_test.go - a new test
pkg/controller/status/gatherer_status_test.go - a new test
pkg/controller/periodic/periodic_test.go - updated and extended
pkg/gather/gather_test.go - updated

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/???

tremes · 2023-04-05T10:41:48Z

pkg/controller/gather_commands.go

+			Reason:             "AsExpected",
+			LastTransitionTime: metav1.Now(),
+		},
+	}


This status updating is temporary and will be addressed in https://issues.redhat.com/browse/CCXDEV-10590

rluders · 2023-04-18T12:46:36Z

Checked the code and tested it.
/lgtm

openshift-ci · 2023-04-18T12:49:18Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rluders, 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:

~~OWNERS~~ [rluders,tremes]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ikerreyes · 2023-04-21T07:38:47Z

/label qe-approved

rluders · 2023-05-09T08:55:39Z

pkg/controller/periodic/job.go

+		},
+		Spec: batchv1.JobSpec{
+			// backoff limit is 0 - we dont' want to restart the gathering immediately in case of failure
+			BackoffLimit: new(int32),


Not actually a problem, but I think that we may also want to add TTL... I was testing locally, and at least on my cluster, the completed jobs started to stack up.

Do you mean this https://github.com/kubernetes/api/blob/v0.27.1/batch/v1/types.go#L302 ? Yes we keep the finished jobs on purpose and that's why there's this https://github.com/openshift/insights-operator/pull/764/files#diff-dec12f0cf28a91a837b46e755b46c081c9014121cdb770b2cd8ac916af29c8deR393 pruning method. Jobs older than 24h are removed....so I think there's currently no reason to set the TTL attribute.

I don't know why, but I tried to run the job twice in less than 30 min and it gave a message saying that it couldn't create the job because it already existed. I think that I should try to reproduce it again. 🤔

User is not supposed to create or run the job manually. Creating the periodic-gather jobs is the operator responsibility.

OK, I tested it again, and it is working as expected. I think that the cluster that I was playing with was broken.

rluders · 2023-05-15T06:50:16Z

I want to clarify one thing about the jobs first.
/remove-lgtm

rluders · 2023-05-15T14:06:31Z

/lgtm

…uest ID

tremes · 2023-05-22T06:18:41Z

/retest

DennisPeriquet · 2023-05-26T14:54:48Z

/lgtm
/label jira/valid-bug

DennisPeriquet · 2023-05-26T14:56:18Z

manually tagging this PR for "Feature Merge Queue"

DennisPeriquet · 2023-05-26T15:09:05Z

/override ci/prow/e2e-metal-ipi-ovn-ipv6
/override ci/prow/insights-operator-e2e-tests

openshift-ci · 2023-05-26T15:09:19Z

@DennisPeriquet: Overrode contexts on behalf of DennisPeriquet: ci/prow/e2e-metal-ipi-ovn-ipv6, ci/prow/insights-operator-e2e-tests

In response to this:

/override ci/prow/e2e-metal-ipi-ovn-ipv6
/override ci/prow/insights-operator-e2e-tests

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.

DennisPeriquet · 2023-05-26T15:15:59Z

/override ci/prow/e2e
/override ci/prow/e2e-agnostic-upgrade

openshift-ci · 2023-05-26T15:16:07Z

@DennisPeriquet: Overrode contexts on behalf of DennisPeriquet: ci/prow/e2e, ci/prow/e2e-agnostic-upgrade

In response to this:

/override ci/prow/e2e
/override 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.

openshift-ci · 2023-05-26T15:22:31Z

@tremes: all tests passed!

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.

stbenjam · 2023-05-30T13:47:07Z

This is causing all techpreview jobs to fail. https://issues.redhat.com/browse/OCPBUGS-14270

…ift#764)" This reverts commit bae9698.

tremes · 2023-05-30T13:50:48Z

yeah I just found out that. Sorry. Let me revert it

…preview (openshift#764)" This reverts commit bae9698.

tremes · 2023-05-30T13:53:28Z

Revert PR is #785

* 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

* Implement periodic gathering as a job in tech preview (#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

* 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

…preview (openshift#764)" This reverts commit bae9698.

* 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

openshift-ci bot requested review from ncaak and rluders April 5, 2023 10:41

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 5, 2023

tremes commented Apr 5, 2023

View reviewed changes

tremes changed the title ~~Implement periodic gathering as a job in tech preview~~ WIP Implement periodic gathering as a job in tech preview Apr 5, 2023

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 5, 2023

tremes changed the title ~~WIP Implement periodic gathering as a job in tech preview~~ Implement periodic gathering as a job in tech preview Apr 6, 2023

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 6, 2023

rluders approved these changes Apr 18, 2023

View reviewed changes

openshift-ci bot assigned rluders Apr 18, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 18, 2023

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Apr 21, 2023

rluders reviewed May 9, 2023

View reviewed changes

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label May 15, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 15, 2023

tremes added 10 commits May 17, 2023 09:30

Run gathering as separate Pod in tech preview

092f07d

Move downloading of reports to operator part & propagate Insights Req…

b560b56

…uest ID

Minor updates

defb341

DataGather CR - very first commit

22db645

create a new DataGather CR and prune periodically

6b2d8cc

read and apply gatherers config from the new CR

d19ba28

Fix anonymizer

9970243

do not duplicate gatherer status creation

c09123c

extract the job responsibility and fix contexts

960c668

Copy Gatherer status & tests

6397932

tremes force-pushed the gather-job-tp branch from c828b06 to 387f524 Compare May 17, 2023 07:38

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label May 17, 2023

openshift-ci bot assigned DennisPeriquet May 26, 2023

openshift-ci bot added lgtm Indicates that a PR is ready to be merged. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels May 26, 2023

openshift-merge-robot merged commit bae9698 into openshift:master May 26, 2023

stbenjam added a commit to stbenjam/insights-operator that referenced this pull request May 30, 2023

Revert "Implement periodic gathering as a job in tech preview (opensh…

ad980b2

…ift#764)" This reverts commit bae9698.

tremes added a commit to tremes/insights-operator that referenced this pull request May 30, 2023

OCPBUGS-14270: Revert "Implement periodic gathering as a job in tech …

4fac705

…preview (openshift#764)" This reverts commit bae9698.

tremes mentioned this pull request May 30, 2023

OCPBUGS-14270: Revert "Implement periodic gathering as a job in tech … #785

Merged

5 tasks

tremes mentioned this pull request May 31, 2023

Implement periodic gathering as a job in tech preview #787

Merged

5 tasks

tremes mentioned this pull request Jun 23, 2023

Implement periodic gathering as a job in tech preview & latest fix #799

Merged

5 tasks

tremes deleted the gather-job-tp branch July 10, 2023 10:14

JoaoFula pushed a commit to JoaoFula/insights-operator that referenced this pull request Jan 23, 2024

OCPBUGS-14270: Revert "Implement periodic gathering as a job in tech …

942af51

…preview (openshift#764)" This reverts commit bae9698.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement periodic gathering as a job in tech preview #764

Implement periodic gathering as a job in tech preview #764

tremes commented Apr 5, 2023

tremes Apr 5, 2023

rluders commented Apr 18, 2023

openshift-ci bot commented Apr 18, 2023

ikerreyes commented Apr 21, 2023

rluders May 9, 2023

tremes May 15, 2023

rluders May 15, 2023

tremes May 15, 2023

rluders May 15, 2023

rluders commented May 15, 2023

rluders commented May 15, 2023

tremes commented May 22, 2023

DennisPeriquet commented May 26, 2023

DennisPeriquet commented May 26, 2023

DennisPeriquet commented May 26, 2023

openshift-ci bot commented May 26, 2023

DennisPeriquet commented May 26, 2023

openshift-ci bot commented May 26, 2023

openshift-ci bot commented May 26, 2023

stbenjam commented May 30, 2023

tremes commented May 30, 2023

tremes commented May 30, 2023

Implement periodic gathering as a job in tech preview #764

Implement periodic gathering as a job in tech preview #764

Conversation

tremes commented Apr 5, 2023

Categories

Sample Archive

Documentation

Unit Tests

Privacy

Changelog

Breaking Changes

References

tremes Apr 5, 2023

Choose a reason for hiding this comment

rluders commented Apr 18, 2023

openshift-ci bot commented Apr 18, 2023

ikerreyes commented Apr 21, 2023

rluders May 9, 2023

Choose a reason for hiding this comment

tremes May 15, 2023

Choose a reason for hiding this comment

rluders May 15, 2023

Choose a reason for hiding this comment

tremes May 15, 2023

Choose a reason for hiding this comment

rluders May 15, 2023

Choose a reason for hiding this comment

rluders commented May 15, 2023

rluders commented May 15, 2023

tremes commented May 22, 2023

DennisPeriquet commented May 26, 2023

DennisPeriquet commented May 26, 2023

DennisPeriquet commented May 26, 2023

openshift-ci bot commented May 26, 2023

DennisPeriquet commented May 26, 2023

openshift-ci bot commented May 26, 2023

openshift-ci bot commented May 26, 2023

stbenjam commented May 30, 2023

tremes commented May 30, 2023

tremes commented May 30, 2023