Bug 1954640: Support of gatherers with different periods #399
Conversation
openshift-ci-robot
added
the
do-not-merge/work-in-progress
openshift-ci-robot
added
the
approved
|
I think it's a lot of changes in this PR. I mean it includes good ideas, but I am not sure it's the right timing to do such a big change (Maybe I would rather wait when there's no feature freeze = no bugzillas needed). I think it might also happen that we won't need this extra period gatherer (that's also the reason why I wouldn't invest such an effort right now). I know it's still WIP, but I am missing the original workloads gatherer. I was also playing little bit with this idea and maybe something like this could be sufficient, but I am not saying that's the way we should do that. |
openshift-ci
bot
added
the
needs-rebase
|
@tremes what's the problem with having bugzilla, btw?
I'd prefer the split, but we can actually keep one file by changing logic little bit. |
|
@Sergey1011010 No problem with having bugzillas. My point is that we now need a new bugzilla for every PR and if we will need some additional fixes then we need new bugzilla for each (it's always a risk with some bigger changes). I know that we didn't care much about the feature freeze in the past, but it probably makes sense not to make some bigger changes when there is this freeze. |
|
@tremes so do we want to postpone this? |
|
@Sergey1011010 I am not sure. Maybe I am too cautious. Let's see what other team members think. |
|
I've got another idea about the extra period. We could introduce a new variable e.g I think this would be more clear. |
|
@tremes I think it works only as a temporarily solution, it's not very flexible |
|
@tremes changed the logic of periods. Now we have gatherers which are added to each archive and optional gatherers which are processed only when it's there time, periods don't need to match with interval in config. Optional gatherers have periods(time.Duration) which are now defined in the file, but can be easily moved to the config. Tomorrow I'm gonna replace |
Serhii1011010
force-pushed
the
support-of-gatherers-with-different-periods
branch
3 times, most recently
from
1d67d27
to
1938946
Compare
Serhii1011010
force-pushed
the
support-of-gatherers-with-different-periods
branch
from
1938946
to
a704a1a
Compare
Serhii1011010
changed the title
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rluders, Sergey1011010, 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 |
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
13 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/test insights-operator-e2e-tests |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/test insights-operator-e2e-tests |
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@Sergey1011010: The following test failed, say
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. |
|
/test e2e-gcp-upgrade |
|
/test insights-operator-e2e-tests |
|
@Sergey1011010: The specified target(s) for
Use In response to this:
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. |
|
@Sergey1011010: All pull requests linked via external trackers have merged: Bugzilla bug 1954640 has been moved to the MODIFIED state. In response to this:
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. |
This PR mainly implements support of gatherers with different periods, but also refactores gathering logic making it more clear, easier to use and test.
Categories
Sample archive
Documentation
docs/insights-archive-sample/insights-operator/gathers.json# changed format little bitUnit Tests
pkg/controller/periodic/periodic_test.gopkg/gather/clusterconfig/clusterconfig_gatherer_test.gopkg/gather/gather_test.gopkg/gather/tasks_processing_test.gopkg/gather/workloads/workloads_gatherer_test.goPrivacy
No new data was collected
References
https://issues.redhat.com/browse/CCXDEV-4366
https://bugzilla.redhat.com/show_bug.cgi?id=???
https://access.redhat.com/solutions/???