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.

Sign up for GitHub

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

Jump to bottom

Conditional gathering #433

Merged
merged 8 commits into from
Jun 28, 2021
Merged

Conditional gathering #433

merged 8 commits into from
Jun 28, 2021

Conversation

Serhii1011010
Copy link
Contributor

@Serhii1011010 Serhii1011010 commented May 20, 2021 •
edited by openshift-ci bot
Loading

This PR implements a new gatherer needed by CCXDEV-4431. Right now it collects logs and image streams of namespace "openshift-cluster-samples-operator" if alert "SamplesImagestreamImportFailing" is firing, but it designed in a way that it can be further extended to run X gatherer with Y params on Z condition.

Categories

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

Sample Archive

  • docs/insights-archive-sample/conditional/namespaces/openshift-cluster-samples-operator/imagestreams/example.json
  • docs/insights-archive-sample/conditional/namespaces/openshift-cluster-samples-operator/pods/cluster-samples-operator-8ffb9b45f-49mjr/containers/cluster-samples-operator-watch/logs/last-100-lines.log
  • docs/insights-archive-sample/conditional/namespaces/openshift-cluster-samples-operator/pods/cluster-samples-operator-8ffb9b45f-49mjr/containers/cluster-samples-operator/logs/last-100-lines.log

Documentation

  • path/to/documentation.md

Unit Tests

  • pkg/gatherers/conditional/conditional_gatherer_test.go

Privacy

Yes. The collected image streams are anonymized here https://github.com/openshift/insights-operator/pull/433/files#diff-3244b427f7e3f71fcf8c9788e7648b1ae57233dd685b1cb61807698c1bcddf75R70

Changelog

Breaking Changes

It's not a breaking change, but some code can potentially misfunction because the new format for function names in metadata was added, for example conditional/image_stream_definitions_of_namespace/namespace=openshift-cluster-samples-operator where the last part is the gatherer parameters

References

https://issues.redhat.com/browse/CCXDEV-4431
https://bugzilla.redhat.com/show_bug.cgi?id=???
https://access.redhat.com/solutions/???

@openshift-ci 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 May 20, 2021
@openshift-ci openshift-ci bot requested review from rluders and tremes May 20, 2021 20:53
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 20, 2021
@Serhii1011010 Serhii1011010 force-pushed the conditional-gathering branch 3 times, most recently from 7520a4f to 4705cba Compare May 25, 2021 18:12
@Serhii1011010 Serhii1011010 changed the title [WIP] Conditional gathering Conditional gathering May 25, 2021
@openshift-ci 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 May 25, 2021
@Serhii1011010 Serhii1011010 changed the title Conditional gathering Bug 1966153: Conditional gathering May 31, 2021
@openshift-ci openshift-ci bot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels May 31, 2021
@openshift-ci
Copy link

openshift-ci bot commented May 31, 2021

@Sergey1011010: This pull request references Bugzilla bug 1966153, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.8.0) matches configured target release for branch (4.8.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @psimovec

In response to this:

Bug 1966153: Conditional gathering

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 openshift-ci bot requested a review from psimovec May 31, 2021 14:23
@Serhii1011010 Serhii1011010 force-pushed the conditional-gathering branch from 4705cba to 3778b05 Compare June 4, 2021 14:58
@Serhii1011010
Copy link
Contributor Author

/retest

tremes
tremes reviewed Jun 14, 2021
View reviewed changes
pkg/gatherers/conditional/conditional_gatherer.go
}

// GatherConditionalGathererRules stores the gathering rules in insights-operator/conditional-gatherer-rules.json
func (g *Gatherer) GatherConditionalGathererRules(context.Context) ([]record.Record, []error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This will go to our docs/gathered-data.md as I just noticed :) Not sure we want it there....but probably not a big deal. WDYT @jholecek-rh pls? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

We want it there, imho.

I'm just worried about the terminology. Insights rule developers are part of the primary audience for docs/gathered-data.md. They will have to pause to realize that it is not talking about Insights rules (like I did right now). I just cannot think of a better name...

Copy link
Contributor

Choose a reason for hiding this comment

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

Good points @jholecek-rh. Thanks!

@openshift-ci
Copy link

openshift-ci bot commented Jun 14, 2021

@Sergey1011010: PR needs rebase.

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 openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 14, 2021
@Serhii1011010 Serhii1011010 force-pushed the conditional-gathering branch from 00ea498 to e87db1d Compare June 15, 2021 12:26
@Serhii1011010 Serhii1011010 changed the title Bug 1966153: Conditional gathering Conditional gathering Jun 15, 2021
@openshift-ci
Copy link

openshift-ci bot commented Jun 15, 2021

@Sergey1011010: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

In response to this:

Conditional gathering

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-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

19 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@tremes
Copy link
Contributor

tremes commented Jun 24, 2021

Waiting for openshift/origin#26264
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 24, 2021
@tremes
Copy link
Contributor

tremes commented Jun 28, 2021

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 28, 2021
@tremes
Copy link
Contributor

tremes commented Jun 28, 2021

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit f9ac922 into openshift:master Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tremes tremes tremes left review comments

@jholecek-rh jholecek-rh jholecek-rh left review comments

@rluders rluders Awaiting requested review from rluders

@psimovec psimovec Awaiting requested review from psimovec

Assignees

@tremes tremes

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Serhii1011010 @tremes @openshift-ci-robot @mfojtik @openshift-bot @jholecek-rh @openshift-merge-robot