Remove sampler #27960
Remove sampler #27960
Conversation
openshift-ci
bot
added
the
approved
|
Job Failure Risk Analysis for sha: a9be206 |
|
/hold not working rigth yet. |
openshift-ci
bot
added
the
do-not-merge/hold
|
Job Failure Risk Analysis for sha: f1cb284
|
|
Job Failure Risk Analysis for sha: 91933d0
|
|
node lifecycle looks right. pending pods with starting open timeframes look wrong. |
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/retest |
| openInterval(state, "Reboot", event.From) | ||
| intervals = append(intervals, nodeStateTracker.closeInterval(nodeLocator, drainState, simpleCondition(monitorapi.Info, monitorapi.NodeUpdateReason, fmt.Sprintf(msgPhaseDrain, roles)), event.From)...) | ||
| intervals = append(intervals, nodeStateTracker.closeInterval(nodeLocator, osUpdateState, simpleCondition(monitorapi.Info, monitorapi.NodeUpdateReason, fmt.Sprintf(msgPhaseOSUpdate, roles)), event.From)...) | ||
| nodeStateTracker.openInterval(nodeLocator, rebootState, event.From) |
There was a problem hiding this comment.
Is it necessary to have this embedded logic about one reason implying closing intervals for other reasons? Any changes in the product and we're immediately broken. Could we instead just close any matching open interval?
|
|
||
| "github.com/openshift/origin/pkg/monitor/monitorapi" | ||
| "k8s.io/apimachinery/pkg/util/sets" | ||
| ) |
There was a problem hiding this comment.
Unit tests might be good here, I know it's getting tested at a higher level but are we certain it's enough coverage?
|
@deads2k: The following tests 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. |
|
Job Failure Risk Analysis for sha: cf696a7
|
|
/lgtm Looks to be working well enough. We can refine if needed but this will unblock #27947 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, dgoodwin 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 |
openshift-merge-robot
merged commit b864965
into
openshift:master
Remove the sampler and replace with observing the beginning and end. It eliminates extraneous events and makes it possible to have maps inside of the intervals themselves, which for annotations feels very natural. Without this, conditions cannot have map[string]string annotations.