Skip to content

fix: show actual event kinds in Queue mode #706

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

Closed
wants to merge 7 commits into from
Closed

fix: show actual event kinds in Queue mode #706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8c48270
fix: show actual event kinds in Queue mode
Oct 20, 2022
1d949ef
feat: emit pod events on drain
Nov 2, 2022
d1fe29d
fix: satisfy PR requested change
Oct 13, 2022
2aaf71b
feat: emit pod events on drain
Nov 2, 2022
de453e1
fix: satisfy PR requested change
Oct 13, 2022
38a9d7a
Merge remote-tracking branch 'origin/main' into rtorrents/fix_sqs_kind
Nov 2, 2022
7e74679
fix: typo
Nov 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cmd/node-termination-handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,13 +196,13 @@ func main() {

for _, fn := range monitoringFns {
go func(monitor monitor.Monitor) {
log.Info().Str("event_type", monitor.Kind()).Msg("Started monitoring for events")
log.Info().Str("monitor_type", monitor.Kind()).Msg("Started monitoring for events")
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use the original field name event_type to preserve backwards compatibility, even though that's technically not correct. I'm open to adding another field named monitor_type as well, to make the meaning more clear, but we need to keep the existing field name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So you want to keep the event_type field but change its value? Old values here were consts like RebalanceRecommendationKind = "REBALANCE_RECOMMENDATION" while new ones are consts like RebalanceRecommendationMonitorKind = "REBALANCE_RECOMMENDATION_MONITOR". With that we are keeping the field name but changing its value so we're effectively introducing breaking changes anyway when it comes to people parsing such fields (I guess they are also parsing the field value, not just the mere presence of the field with any value).

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right. I meant that we should keep the original value, too. So event_type should be REBALANCE_RECOMMENDATION, SCHEDULED_EVENT, SPOT_ITN, or SQS_TERMINATE, rather than the newer *_MONITOR values returned by MonitorKind().

var previousErr error
var duplicateErrCount int
for range time.Tick(time.Second * 2) {
err := monitor.Monitor()
if err != nil {
log.Warn().Str("event_type", monitor.Kind()).Err(err).Msg("There was a problem monitoring for events")
log.Warn().Str("monitor_type", monitor.Kind()).Err(err).Msg("There was a problem monitoring for events")
metrics.ErrorEventsInc(monitor.Kind())
recorder.Emit(nthConfig.NodeName, observability.Warning, observability.MonitorErrReason, observability.MonitorErrMsgFmt, monitor.Kind())
if previousErr != nil && err.Error() == previousErr.Error() {
Expand Down Expand Up @@ -242,6 +242,7 @@ func main() {
log.Info().
Str("event-id", event.EventID).
Str("kind", event.Kind).
Str("monitor", event.Monitor).
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's call this aws-event-details or perhaps event-type-details, and have it reflect the underlying AWS event. kind will continue to refer to the monitor type as it did in the past, to preserve backwards compatibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd just call this aws-event-type or aws-event-kind. IMO adding details here is confusing because the field only contains one single detail.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. Let's go with aws-event-type.

Str("node-name", event.NodeName).
Str("instance-id", event.InstanceID).
Str("provider-id", event.ProviderID).
Copy link
Contributor

Choose a reason for hiding this comment

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

On line 252 when we emit the Kubernetes event, we need to keep the "reason" argument the same as it was before (observability.GetReasonForKind(event.Kind)).

However, let's add another argument at the end, after event.Description that includes the AWSEventDetails. I'd suggest we use the "screaming snake case" names without conversion, but I'm open to doing a conversion like GetReasonForKind() does right now if you disagree.

Copy link
Contributor Author

@trutx trutx Nov 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Will do, but I have to respectfully say I'm against this one. In all other cases the old and wrong field, data, log line or whatever can be kept and coexist with a new and fixed one. In this particular case like you pointed out there's only one reason, so either we use the right or the wrong reason. Kubernetes event reasons are the cornerstone of event-based Kubernetes monitoring, and we're choosing to leave it wrong. I understand backwards compatibility but in this case here we're not talking about an enhancement or a new feature but a fix, and I think not fixing things that are known to be wrong is not the best way to go.

That said, I think the best option is to leave the recorder.Emit() call in 252 as is and instead modify the Description field generation in all monitors. I.e., we would change line 93 in spot-itn-monitor.go to something like

Description:  fmt.Sprintf("Spot ITN received. Instance will be interrupted at %s. AWS event type: %s \n", instanceAction.Time, monitor.SpotITNKind),

or maybe

Description:  fmt.Sprintf("Spot ITN received (%s). Instance will be interrupted at %s \n", monitor.SpotITNKind, instanceAction.Time),

WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you also intending to keep the return values of observability.GetReasonForKind() as they were before, such that the only change to the argument values in recorder.Emit() would be this updated Description field? If so, I am in favor of that plan.

I like your first suggestion (AWS event type: ... at the end of the Description).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, if we are to keep the old and wrong reason there is no other option than reverting the changes in GetReasonForKind() so they return the old values.

Expand Down
15 changes: 7 additions & 8 deletions pkg/monitor/rebalancerecommendation/rebalance-recommendation-monitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ import (
"github.com/aws/aws-node-termination-handler/pkg/node"
)

const (
// RebalanceRecommendationKind is a const to define a Rebalance Recommendation kind of event
RebalanceRecommendationKind = "REBALANCE_RECOMMENDATION"
)
// RebalanceRecommentadionMonitorKind is a const to define this monitor kind
const RebalanceRecommendationMonitorKind = "REBALANCE_RECOMMENDATION_MONITOR"

// RebalanceRecommendationMonitor is a struct definition which facilitates monitoring of rebalance recommendations from IMDS
type RebalanceRecommendationMonitor struct {
Expand All @@ -50,15 +48,15 @@ func (m RebalanceRecommendationMonitor) Monitor() error {
if err != nil {
return err
}
if interruptionEvent != nil && interruptionEvent.Kind == RebalanceRecommendationKind {
if interruptionEvent != nil && interruptionEvent.Kind == monitor.RebalanceRecommendationKind {
m.InterruptionChan <- *interruptionEvent
}
return nil
}

// Kind denotes the kind of event that is processed
// Kind denotes the kind of monitor
func (m RebalanceRecommendationMonitor) Kind() string {
return RebalanceRecommendationKind
return RebalanceRecommendationMonitorKind
}

// checkForRebalanceRecommendation Checks EC2 instance metadata for a rebalance recommendation
Expand Down Expand Up @@ -86,7 +84,8 @@ func (m RebalanceRecommendationMonitor) checkForRebalanceRecommendation() (*moni

return &monitor.InterruptionEvent{
EventID: fmt.Sprintf("rebalance-recommendation-%x", hash.Sum(nil)),
Kind: RebalanceRecommendationKind,
Kind: monitor.RebalanceRecommendationKind,
Monitor: RebalanceRecommendationMonitorKind,
StartTime: noticeTime,
NodeName: nodeName,
Description: fmt.Sprintf("Rebalance recommendation received. Instance will be cordoned at %s \n", rebalanceRecommendation.NoticeTime),
Expand Down
3 changes: 2 additions & 1 deletion pkg/monitor/rebalancerecommendation/rebalance-recommendation-monitor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ func TestMonitor_Success(t *testing.T) {

go func() {
result := <-drainChan
h.Equals(t, rebalancerecommendation.RebalanceRecommendationKind, result.Kind)
h.Equals(t, monitor.RebalanceRecommendationKind, result.Kind)
h.Equals(t, rebalancerecommendation.RebalanceRecommendationMonitorKind, result.Monitor)
h.Equals(t, expFormattedTime, result.StartTime.String())
h.Assert(t, strings.Contains(result.Description, startTime),
"Expected description to contain: "+startTime+" but is actually: "+result.Description)
Expand Down
11 changes: 6 additions & 5 deletions pkg/monitor/scheduledevent/scheduled-event-monitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import (
)

const (
// ScheduledEventKind is a const to define a scheduled event kind of interruption event
ScheduledEventKind = "SCHEDULED_EVENT"
// ScheduledEventMonitorKind is a const to define this monitor kind
ScheduledEventMonitorKind = "SCHEDULED_EVENT_MONITOR"
scheduledEventStateCompleted = "completed"
scheduledEventStateCanceled = "canceled"
scheduledEventDateFormat = "2 Jan 2006 15:04:05 GMT"
Expand Down Expand Up @@ -69,9 +69,9 @@ func (m ScheduledEventMonitor) Monitor() error {
return nil
}

// Kind denotes the kind of event that is processed
// Kind denotes the kind of monitor
func (m ScheduledEventMonitor) Kind() string {
return ScheduledEventKind
return ScheduledEventMonitorKind
}

// checkForScheduledEvents Checks EC2 instance metadata for a scheduled event requiring a node drain
Expand Down Expand Up @@ -101,7 +101,8 @@ func (m ScheduledEventMonitor) checkForScheduledEvents() ([]monitor.Interruption
}
events = append(events, monitor.InterruptionEvent{
EventID: scheduledEvent.EventID,
Kind: ScheduledEventKind,
Kind: monitor.ScheduledEventKind,
Monitor: ScheduledEventMonitorKind,
Description: fmt.Sprintf("%s will occur between %s and %s because %s\n", scheduledEvent.Code, scheduledEvent.NotBefore, scheduledEvent.NotAfter, scheduledEvent.Description),
State: scheduledEvent.State,
NodeName: m.NodeName,
Expand Down
7 changes: 4 additions & 3 deletions pkg/monitor/scheduledevent/scheduled-event-monitor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ func TestMonitor_Success(t *testing.T) {
go func() {
result := <-drainChan
h.Equals(t, scheduledEventId, result.EventID)
h.Equals(t, scheduledevent.ScheduledEventKind, result.Kind)
h.Equals(t, monitor.ScheduledEventKind, result.Kind)
h.Equals(t, scheduledevent.ScheduledEventMonitorKind, result.Monitor)
h.Equals(t, scheduledEventState, result.State)
h.TimeWithinRange(t, result.StartTime, oneSecondAgo(), time.Now())
h.Equals(t, expScheduledEventEndTimeFmt, result.EndTime.String())
Expand Down Expand Up @@ -130,7 +131,7 @@ func TestMonitor_CanceledEvent(t *testing.T) {
go func() {
result := <-cancelChan
h.Equals(t, scheduledEventId, result.EventID)
h.Equals(t, scheduledevent.ScheduledEventKind, result.Kind)
h.Equals(t, monitor.ScheduledEventKind, result.Kind)
h.Equals(t, state, result.State)
h.TimeWithinRange(t, result.StartTime, oneSecondAgo(), time.Now())
h.Equals(t, expScheduledEventEndTimeFmt, result.EndTime.String())
Expand Down Expand Up @@ -257,7 +258,7 @@ func TestMonitor_EndTimeParseFail(t *testing.T) {
go func() {
result := <-drainChan
h.Equals(t, scheduledEventId, result.EventID)
h.Equals(t, scheduledevent.ScheduledEventKind, result.Kind)
h.Equals(t, monitor.ScheduledEventKind, result.Kind)
h.Equals(t, scheduledEventState, result.State)
h.TimeWithinRange(t, result.StartTime, oneSecondAgo(), time.Now())
h.Equals(t, expScheduledEventStartTimeFmt, result.EndTime.String())
Expand Down
15 changes: 7 additions & 8 deletions pkg/monitor/spotitn/spot-itn-monitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ import (
"github.com/aws/aws-node-termination-handler/pkg/node"
)

const (
// SpotITNKind is a const to define a Spot ITN kind of interruption event
SpotITNKind = "SPOT_ITN"
)
// SpotITNMonitorKind is a const to define this monitor kind
const SpotITNMonitorKind = "SPOT_ITN_MONITOR"

// SpotInterruptionMonitor is a struct definition which facilitates monitoring of spot ITNs from IMDS
type SpotInterruptionMonitor struct {
Expand All @@ -52,15 +50,15 @@ func (m SpotInterruptionMonitor) Monitor() error {
if err != nil {
return err
}
if interruptionEvent != nil && interruptionEvent.Kind == SpotITNKind {
if interruptionEvent != nil && interruptionEvent.Kind == monitor.SpotITNKind {
m.InterruptionChan <- *interruptionEvent
}
return nil
}

// Kind denotes the kind of event that is processed
// Kind denotes the kind of monitor
func (m SpotInterruptionMonitor) Kind() string {
return SpotITNKind
return SpotITNMonitorKind
}

// checkForSpotInterruptionNotice Checks EC2 instance metadata for a spot interruption termination notice
Expand Down Expand Up @@ -88,7 +86,8 @@ func (m SpotInterruptionMonitor) checkForSpotInterruptionNotice() (*monitor.Inte

return &monitor.InterruptionEvent{
EventID: fmt.Sprintf("spot-itn-%x", hash.Sum(nil)),
Kind: SpotITNKind,
Kind: monitor.SpotITNKind,
Monitor: SpotITNMonitorKind,
StartTime: interruptionTime,
NodeName: nodeName,
Description: fmt.Sprintf("Spot ITN received. Instance will be interrupted at %s \n", instanceAction.Time),
Expand Down
3 changes: 2 additions & 1 deletion pkg/monitor/spotitn/spot-itn-monitor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ func TestMonitor_Success(t *testing.T) {

go func() {
result := <-drainChan
h.Equals(t, spotitn.SpotITNKind, result.Kind)
h.Equals(t, monitor.SpotITNKind, result.Kind)
h.Equals(t, spotitn.SpotITNMonitorKind, result.Monitor)
h.Equals(t, expFormattedTime, result.StartTime.String())
h.Assert(t, strings.Contains(result.Description, startTime),
"Expected description to contain: "+startTime+" but is actually: "+result.Description)
Expand Down
3 changes: 2 additions & 1 deletion pkg/monitor/sqsevent/asg-lifecycle-event.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ func (m SQSMonitor) asgTerminationToInterruptionEvent(event *EventBridgeEvent, m

interruptionEvent := monitor.InterruptionEvent{
EventID: fmt.Sprintf("asg-lifecycle-term-%x", event.ID),
Kind: SQSTerminateKind,
Kind: monitor.ASGLifecycleKind,
Monitor: SQSMonitorKind,
AutoScalingGroupName: lifecycleDetail.AutoScalingGroupName,
StartTime: event.getTime(),
NodeName: nodeInfo.Name,
Expand Down
3 changes: 2 additions & 1 deletion pkg/monitor/sqsevent/ec2-state-change-event.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ func (m SQSMonitor) ec2StateChangeToInterruptionEvent(event *EventBridgeEvent, m
}
interruptionEvent := monitor.InterruptionEvent{
EventID: fmt.Sprintf("ec2-state-change-event-%x", event.ID),
Kind: SQSTerminateKind,
Kind: monitor.StateChangeKind,
Monitor: SQSMonitorKind,
StartTime: event.getTime(),
NodeName: nodeInfo.Name,
IsManaged: nodeInfo.IsManaged,
Expand Down
3 changes: 2 additions & 1 deletion pkg/monitor/sqsevent/rebalance-recommendation-event.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ func (m SQSMonitor) rebalanceRecommendationToInterruptionEvent(event *EventBridg
}
interruptionEvent := monitor.InterruptionEvent{
EventID: fmt.Sprintf("rebalance-recommendation-event-%x", event.ID),
Kind: SQSTerminateKind,
Kind: monitor.RebalanceRecommendationKind,
Monitor: SQSMonitorKind,
AutoScalingGroupName: nodeInfo.AsgName,
StartTime: event.getTime(),
NodeName: nodeInfo.Name,
Expand Down
3 changes: 2 additions & 1 deletion pkg/monitor/sqsevent/scheduled-change-event.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ func (m SQSMonitor) scheduledEventToInterruptionEvents(event *EventBridgeEvent,
// Begin drain immediately for scheduled change events to avoid disruptions in cases such as degraded hardware
interruptionEvent := monitor.InterruptionEvent{
EventID: fmt.Sprintf("aws-health-scheduled-change-event-%x", event.ID),
Kind: SQSTerminateKind,
Kind: monitor.ScheduledEventKind,
Monitor: SQSMonitorKind,
AutoScalingGroupName: nodeInfo.AsgName,
StartTime: time.Now(),
NodeName: nodeInfo.Name,
Expand Down
3 changes: 2 additions & 1 deletion pkg/monitor/sqsevent/spot-itn-event.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ func (m SQSMonitor) spotITNTerminationToInterruptionEvent(event *EventBridgeEven
}
interruptionEvent := monitor.InterruptionEvent{
EventID: fmt.Sprintf("spot-itn-event-%x", event.ID),
Kind: SQSTerminateKind,
Kind: monitor.SpotITNKind,
Monitor: SQSMonitorKind,
AutoScalingGroupName: nodeInfo.AsgName,
StartTime: event.getTime(),
NodeName: nodeInfo.Name,
Expand Down
14 changes: 7 additions & 7 deletions pkg/monitor/sqsevent/sqs-monitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ import (
)

const (
// SQSTerminateKind is a const to define an SQS termination kind of interruption event
SQSTerminateKind = "SQS_TERMINATE"
// SQSMonitorKind is a const to define this monitor kind
SQSMonitorKind = "SQS_MONITOR"
// ASGTagName is the name of the instance tag whose value is the AutoScaling group name
ASGTagName = "aws:autoscaling:groupName"
)
Expand Down Expand Up @@ -71,9 +71,9 @@ func (s skip) Unwrap() error {
return s.err
}

// Kind denotes the kind of event that is processed
// Kind denotes the kind of monitor
func (m SQSMonitor) Kind() string {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm opening to renaming this method to MonitorKind() so that it's more clear internally that we are referring to the monitor type, not the event type. Similar change on the IMDS-based monitors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, sounds good. Will do.

return SQSTerminateKind
return SQSMonitorKind
}

// Monitor continuously monitors SQS for events and coordinates processing of the events
Expand Down Expand Up @@ -219,9 +219,9 @@ func (m SQSMonitor) processInterruptionEvents(interruptionEventWrappers []Interr
log.Debug().Str("instance-id", eventWrapper.InterruptionEvent.InstanceID).Msg("dropping interruption event for unmanaged node")
dropMessageSuggestionCount++

case eventWrapper.InterruptionEvent.Kind == SQSTerminateKind:
// Successfully processed SQS message into a SQSTerminateKind interruption event
log.Debug().Msgf("Sending %s interruption event to the interruption channel", SQSTerminateKind)
case eventWrapper.InterruptionEvent.Monitor == SQSMonitorKind:
// Successfully processed SQS message into a eventWrapper.InterruptionEvent.Kind interruption event
log.Debug().Msgf("Sending %s interruption event to the interruption channel", eventWrapper.InterruptionEvent.Kind)
m.InterruptionChan <- *eventWrapper.InterruptionEvent

default:
Expand Down
32 changes: 23 additions & 9 deletions pkg/monitor/sqsevent/sqs-monitor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,15 @@ var rebalanceRecommendationEvent = sqsevent.EventBridgeEvent{
}`),
}

func TestKind(t *testing.T) {
h.Assert(t, sqsevent.SQSMonitor{}.Kind() == sqsevent.SQSTerminateKind, "SQSMonitor kind should return the kind constant for the event")
func TestMonitorKind(t *testing.T) {
h.Assert(t, sqsevent.SQSMonitor{}.Kind() == sqsevent.SQSMonitorKind, "SQSMonitor kind should return the kind constant for the monitor")
}

func TestMonitor_EventBridgeSuccess(t *testing.T) {
spotItnEventNoTime := spotItnEvent
spotItnEventNoTime.Time = ""
i := 0
expectedResultKinds := []string{monitor.SpotITNKind, monitor.ASGLifecycleKind, monitor.SpotITNKind, monitor.RebalanceRecommendationKind}
for _, event := range []sqsevent.EventBridgeEvent{spotItnEvent, asgLifecycleEvent, spotItnEventNoTime, rebalanceRecommendationEvent} {
msg, err := getSQSMessageFromEvent(event)
h.Ok(t, err)
Expand Down Expand Up @@ -159,7 +161,8 @@ func TestMonitor_EventBridgeSuccess(t *testing.T) {

select {
case result := <-drainChan:
h.Equals(t, sqsevent.SQSTerminateKind, result.Kind)
h.Equals(t, expectedResultKinds[i], result.Kind)
h.Equals(t, sqsevent.SQSMonitorKind, result.Monitor)
h.Equals(t, result.NodeName, dnsNodeName)
h.Assert(t, result.PostDrainTask != nil, "PostDrainTask should have been set")
h.Assert(t, result.PreDrainTask != nil, "PreDrainTask should have been set")
Expand All @@ -168,7 +171,7 @@ func TestMonitor_EventBridgeSuccess(t *testing.T) {
default:
h.Ok(t, fmt.Errorf("Expected an event to be generated"))
}

i++
}
}

Expand Down Expand Up @@ -243,7 +246,8 @@ func TestMonitor_AsgDirectToSqsSuccess(t *testing.T) {

select {
case result := <-drainChan:
h.Equals(t, sqsevent.SQSTerminateKind, result.Kind)
h.Equals(t, monitor.ASGLifecycleKind, result.Kind)
h.Equals(t, sqsevent.SQSMonitorKind, result.Monitor)
h.Equals(t, result.NodeName, dnsNodeName)
h.Assert(t, result.PostDrainTask != nil, "PostDrainTask should have been set")
h.Assert(t, result.PreDrainTask != nil, "PreDrainTask should have been set")
Expand Down Expand Up @@ -330,16 +334,20 @@ func TestMonitor_DrainTasks(t *testing.T) {
err := sqsMonitor.Monitor()
h.Ok(t, err)

i := 0
expectedResultKinds := []string{monitor.SpotITNKind, monitor.ASGLifecycleKind, monitor.RebalanceRecommendationKind}
for _, event := range testEvents {
t.Run(event.DetailType, func(st *testing.T) {
result := <-drainChan
h.Equals(st, sqsevent.SQSTerminateKind, result.Kind)
h.Equals(st, expectedResultKinds[i], result.Kind)
h.Equals(st, sqsevent.SQSMonitorKind, result.Monitor)
h.Equals(st, result.NodeName, dnsNodeName)
h.Assert(st, result.PostDrainTask != nil, "PostDrainTask should have been set")
h.Assert(st, result.PreDrainTask != nil, "PreDrainTask should have been set")
err := result.PostDrainTask(result, node.Node{})
h.Ok(st, err)
})
i++
}
}

Expand Down Expand Up @@ -378,7 +386,8 @@ func TestMonitor_DrainTasks_Delay(t *testing.T) {

t.Run(asgLifecycleEvent.DetailType, func(st *testing.T) {
result := <-drainChan
h.Equals(st, sqsevent.SQSTerminateKind, result.Kind)
h.Equals(st, monitor.ASGLifecycleKind, result.Kind)
h.Equals(st, sqsevent.SQSMonitorKind, result.Monitor)
h.Equals(st, result.NodeName, dnsNodeName)
h.Assert(st, result.PostDrainTask != nil, "PostDrainTask should have been set")
h.Assert(st, result.PreDrainTask != nil, "PreDrainTask should have been set")
Expand Down Expand Up @@ -425,12 +434,15 @@ func TestMonitor_DrainTasks_Errors(t *testing.T) {
h.Ok(t, err)

count := 0
i := 0
expectedResultKinds := []string{monitor.SpotITNKind, monitor.ASGLifecycleKind, monitor.RebalanceRecommendationKind}
done := false
for !done {
select {
case result := <-drainChan:
count++
h.Equals(t, sqsevent.SQSTerminateKind, result.Kind)
h.Equals(t, expectedResultKinds[i], result.Kind)
h.Equals(t, sqsevent.SQSMonitorKind, result.Monitor)
h.Equals(t, result.NodeName, dnsNodeName)
h.Assert(t, result.PostDrainTask != nil, "PostDrainTask should have been set")
h.Assert(t, result.PreDrainTask != nil, "PreDrainTask should have been set")
Expand All @@ -439,6 +451,7 @@ func TestMonitor_DrainTasks_Errors(t *testing.T) {
default:
done = true
}
i++
}
h.Equals(t, count, 3)
}
Expand Down Expand Up @@ -479,7 +492,8 @@ func TestMonitor_DrainTasksASGFailure(t *testing.T) {

select {
case result := <-drainChan:
h.Equals(t, sqsevent.SQSTerminateKind, result.Kind)
h.Equals(t, monitor.ASGLifecycleKind, result.Kind)
h.Equals(t, sqsevent.SQSMonitorKind, result.Monitor)
h.Equals(t, result.NodeName, dnsNodeName)
h.Assert(t, result.PostDrainTask != nil, "PostDrainTask should have been set")
h.Assert(t, result.PreDrainTask != nil, "PreDrainTask should have been set")
Expand Down
Loading