Skip to content

Commit ad8cfd3

Browse files
ipsita-npgipsita-npg
committed
fix
1 parent 4ca5f8a commit ad8cfd3

File tree

2 files changed

+147
-11
lines changed

2 files changed

+147
-11
lines changed

‎dm-manager/pkg/device/device.go‎

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -246,15 +246,21 @@ func (dc *Controller) shouldSyncPowerStatus(invHost *computev1.HostResource) boo
246246
}
247247

248248
func (dc *Controller) shouldHandlePowerChange(invHost *computev1.HostResource) bool {
249-
if invHost.GetCurrentAmtState() != computev1.AmtState_AMT_STATE_PROVISIONED {
250-
return false
251-
}
252-
249+
hostId := invHost.GetResourceId()
250+
currentAmtState := invHost.GetCurrentAmtState()
253251
currentState := invHost.GetCurrentPowerState()
254252
desiredState := invHost.GetDesiredPowerState()
255253

254+
log.Info().Msgf("Host %v states - AMT: %v, Current Power: %v, Desired Power: %v", hostId, currentAmtState, currentState, desiredState)
255+
256+
if currentAmtState != computev1.AmtState_AMT_STATE_PROVISIONED {
257+
log.Debug().Msgf("Host %v not provisioned (AMT state: %v), skipping power change", hostId, currentAmtState)
258+
return false
259+
}
260+
256261
// Standard case: different desired and current power states
257262
if desiredState != currentState {
263+
log.Info().Msgf("Host %v standard power change: %v -> %v", hostId, currentState, desiredState)
258264
return true
259265
}
260266

@@ -265,17 +271,25 @@ func (dc *Controller) shouldHandlePowerChange(invHost *computev1.HostResource) b
265271
// Handle consecutive RESET operations
266272
if desiredState == computev1.PowerState_POWER_STATE_RESET &&
267273
currentState == computev1.PowerState_POWER_STATE_RESET {
268-
log.Info().Msgf("Allowing consecutive reset operation for host %v", invHost.GetResourceId())
274+
log.Info().Msgf("Host %v allowing consecutive RESET operation (RESET -> RESET)", hostId)
269275
return true
270276
}
271277

272278
// Handle consecutive RESET_REPEAT operations
273279
if desiredState == computev1.PowerState_POWER_STATE_RESET_REPEAT &&
274280
currentState == computev1.PowerState_POWER_STATE_RESET_REPEAT {
275-
log.Info().Msgf("Allowing consecutive reset repeat operation for host %v", invHost.GetResourceId())
281+
log.Info().Msgf("Host %v allowing consecutive RESET_REPEAT operation (RESET_REPEAT -> RESET_REPEAT)", hostId)
276282
return true
277283
}
278284

285+
// Handle mixed consecutive operations (RESET_REPEAT when current is RESET)
286+
if desiredState == computev1.PowerState_POWER_STATE_RESET_REPEAT &&
287+
currentState == computev1.PowerState_POWER_STATE_RESET {
288+
log.Info().Msgf("Host %v allowing RESET_REPEAT after RESET operation (RESET -> RESET_REPEAT)", hostId)
289+
return true
290+
}
291+
292+
log.Debug().Msgf("Host %v no power change needed (current=%v, desired=%v)", hostId, currentState, desiredState)
279293
return false
280294
}
281295

@@ -453,8 +467,24 @@ func (dc *Controller) handlePowerChange(
453467
invHost *computev1.HostResource,
454468
desiredPowerState computev1.PowerState,
455469
) (rec_v2.Directive[ID], statusv1.StatusIndication, error) {
456-
log.Info().Msgf("trying to change power state for %v from %v to %v", request.ID,
457-
invHost.GetCurrentPowerState(), desiredPowerState)
470+
hostId := request.ID.GetHostUUID()
471+
currentState := invHost.GetCurrentPowerState()
472+
473+
log.Info().Msgf("Performing power change for host %v: %v -> %v",
474+
hostId, currentState, desiredPowerState)
475+
476+
// Check if this is a consecutive reset operation
477+
// for debugging purpose only
478+
switch desiredPowerState {
479+
case computev1.PowerState_POWER_STATE_RESET_REPEAT:
480+
log.Info().Msgf("Processing consecutive RESET_REPEAT operation for host %v", hostId)
481+
case computev1.PowerState_POWER_STATE_RESET:
482+
if currentState == computev1.PowerState_POWER_STATE_RESET {
483+
log.Info().Msgf("Processing consecutive RESET operation for host %v", hostId)
484+
} else {
485+
log.Info().Msgf("Processing standard RESET operation for host %v", hostId)
486+
}
487+
}
458488

459489
err := dc.updateHost(ctx, request.ID.GetTenantID(), invHost.GetResourceId(),
460490
&fieldmaskpb.FieldMask{Paths: []string{
@@ -465,23 +495,31 @@ func (dc *Controller) handlePowerChange(
465495
PowerStatusIndicator: statusv1.StatusIndication_STATUS_INDICATION_IN_PROGRESS,
466496
})
467497
if err != nil {
468-
log.Err(err).Msgf("failed to update device info")
498+
log.Err(err).Msgf("Failed to update device info for host %v", hostId)
469499
return request.Fail(err), statusv1.StatusIndication_STATUS_INDICATION_ERROR, err
470500
}
471501

502+
// MPS action being sent
503+
mpsAction := powerMapping[desiredPowerState]
504+
log.Info().Msgf("Sending MPS power action for host %v: state=%v -> mps_action=%v",
505+
hostId, desiredPowerState, mpsAction)
506+
472507
// assumption is that we don't have to check if power actions are supported, as AMT supports it since 1.0.0
473508
// https://en.wikipedia.org/wiki/Intel_AMT_versions
474509
updatedCtx := context.WithValue(ctx, contextValue("tenantId"), request.ID.GetTenantID())
475510
powerAction, err := dc.MpsClient.PostApiV1AmtPowerActionGuidWithResponse(updatedCtx, request.ID.GetHostUUID(),
476511
mps.PostApiV1AmtPowerActionGuidJSONRequestBody{
477-
Action: powerMapping[desiredPowerState],
512+
Action: mpsAction,
478513
}, clientCallback())
479514
if err != nil {
480-
log.Err(err).Msgf("failed to send power action to MPS")
515+
log.Err(err).Msgf("Failed to send power action to MPS for host %v", hostId)
481516
return request.Fail(err),
482517
statusv1.StatusIndication_STATUS_INDICATION_ERROR, err
483518
}
484519

520+
log.Info().Msgf("MPS power action sent successfully for host %v, response code: %v",
521+
hostId, powerAction.StatusCode())
522+
485523
log.Debug().Msgf("power action response for %v with status code %v - %v", request.ID.GetHostUUID(),
486524
powerAction.HTTPResponse, string(powerAction.Body))
487525

‎dm-manager/pkg/device/device_test.go‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,3 +519,101 @@ func TestDeviceController_MPS_HeaderValidation_UserAgent(t *testing.T) {
519519

520520
mpsMock.AssertExpectations(t)
521521
}
522+
523+
// Tests for consecutive reset functionality
524+
func TestConsecutiveResetLogic(t *testing.T) {
525+
dc := &Controller{}
526+
527+
tests := []struct {
528+
name string
529+
currentAmt computev1.AmtState
530+
currentPower computev1.PowerState
531+
desiredPower computev1.PowerState
532+
expectedResult bool
533+
description string
534+
}{
535+
{
536+
name: "Standard Reset - First Time",
537+
currentAmt: computev1.AmtState_AMT_STATE_PROVISIONED,
538+
currentPower: computev1.PowerState_POWER_STATE_ON,
539+
desiredPower: computev1.PowerState_POWER_STATE_RESET,
540+
expectedResult: true,
541+
description: "First reset should be allowed (different states)",
542+
},
543+
{
544+
name: "Consecutive Reset - RESET to RESET",
545+
currentAmt: computev1.AmtState_AMT_STATE_PROVISIONED,
546+
currentPower: computev1.PowerState_POWER_STATE_RESET,
547+
desiredPower: computev1.PowerState_POWER_STATE_RESET,
548+
expectedResult: true,
549+
description: "Consecutive RESET operations should be allowed",
550+
},
551+
{
552+
name: "Consecutive Reset - RESET_REPEAT to RESET_REPEAT",
553+
currentAmt: computev1.AmtState_AMT_STATE_PROVISIONED,
554+
currentPower: computev1.PowerState_POWER_STATE_RESET_REPEAT,
555+
desiredPower: computev1.PowerState_POWER_STATE_RESET_REPEAT,
556+
expectedResult: true,
557+
description: "Consecutive RESET_REPEAT operations should be allowed",
558+
},
559+
{
560+
name: "Mixed Consecutive - RESET to RESET_REPEAT",
561+
currentAmt: computev1.AmtState_AMT_STATE_PROVISIONED,
562+
currentPower: computev1.PowerState_POWER_STATE_RESET,
563+
desiredPower: computev1.PowerState_POWER_STATE_RESET_REPEAT,
564+
expectedResult: true,
565+
description: "RESET_REPEAT after RESET should be allowed",
566+
},
567+
{
568+
name: "Not Provisioned - Should Skip",
569+
currentAmt: computev1.AmtState_AMT_STATE_UNPROVISIONED,
570+
currentPower: computev1.PowerState_POWER_STATE_ON,
571+
desiredPower: computev1.PowerState_POWER_STATE_RESET,
572+
expectedResult: false,
573+
description: "Unprovisioned devices should not handle power changes",
574+
},
575+
{
576+
name: "Same Non-Reset States",
577+
currentAmt: computev1.AmtState_AMT_STATE_PROVISIONED,
578+
currentPower: computev1.PowerState_POWER_STATE_ON,
579+
desiredPower: computev1.PowerState_POWER_STATE_ON,
580+
expectedResult: false,
581+
description: "Same non-reset states should not trigger power change",
582+
},
583+
}
584+
585+
for _, tt := range tests {
586+
t.Run(tt.name, func(t *testing.T) {
587+
// Create a mock host resource
588+
hostResource := &computev1.HostResource{
589+
ResourceId: "test-host-123",
590+
CurrentAmtState: tt.currentAmt,
591+
CurrentPowerState: tt.currentPower,
592+
DesiredPowerState: tt.desiredPower,
593+
}
594+
595+
result := dc.shouldHandlePowerChange(hostResource)
596+
assert.Equal(t, tt.expectedResult, result, tt.description)
597+
})
598+
}
599+
}
600+
601+
func TestPowerStateMapping(t *testing.T) {
602+
t.Run("RESET_REPEAT maps to correct MPS action", func(t *testing.T) {
603+
action := powerMapping[computev1.PowerState_POWER_STATE_RESET_REPEAT]
604+
expectedAction := powerReset // Should be same as regular RESET
605+
assert.Equal(t, expectedAction, action, "RESET_REPEAT should map to same MPS action as RESET")
606+
})
607+
608+
t.Run("RESET_REPEAT has correct progress state", func(t *testing.T) {
609+
progressState := powerMappingToInProgressState[computev1.PowerState_POWER_STATE_RESET_REPEAT]
610+
expectedState := "Resetting (repeat)"
611+
assert.Equal(t, expectedState, progressState, "RESET_REPEAT should have correct progress state")
612+
})
613+
614+
t.Run("RESET_REPEAT has correct idle state", func(t *testing.T) {
615+
idleState := powerMappingToIdleState[computev1.PowerState_POWER_STATE_RESET_REPEAT]
616+
expectedState := "Reset (repeat) successful"
617+
assert.Equal(t, expectedState, idleState, "RESET_REPEAT should have correct idle state")
618+
})
619+
}

0 commit comments

Comments
 (0)