Skip to content

Commit

Permalink
fix(vm): fix generating wrong statistic (#414)
Browse files Browse the repository at this point in the history 
fix generating wrong statistic
---------
Signed-off-by: yaroslavborbat <[email protected]>
  • Loading branch information
@yaroslavborbat
yaroslavborbat authored Nov 25, 2024
1 parent c18db31 commit e081e9a
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 238 deletions.
117 changes: 40 additions & 77 deletions images/virtualization-artifact/pkg/controller/vm/internal/statistic.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package internal
import (
"context"
"math"
"sort"
"strconv"
"time"

Expand Down Expand Up @@ -200,44 +199,55 @@ func (h *StatisticHandler) syncStats(current, changed *virtv2.VirtualMachine, kv
if current == nil || changed == nil {
return
}
phaseChanged := current.Status.Phase != changed.Status.Phase

var stats virtv2.VirtualMachineStats

if current.Status.Stats != nil {
stats = *current.Status.Stats.DeepCopy()
}
pts := NewPhaseTransitions(stats.PhasesTransitions, current.Status.Phase, changed.Status.Phase)
pts.Sort()
stats.PhasesTransitions = pts.Items

launchTimeDuration := stats.LaunchTimeDuration
stats.PhasesTransitions = pts

var emptyOSInfo virtv1.VirtualMachineInstanceGuestOSInfo
launchTimeDuration := stats.LaunchTimeDuration

switch changed.Status.Phase {
case virtv2.MachinePending:
case virtv2.MachinePending, virtv2.MachineStopped:
launchTimeDuration.WaitingForDependencies = nil
launchTimeDuration.VirtualMachineStarting = nil
launchTimeDuration.GuestOSAgentStarting = nil
case virtv2.MachineStarting:
launchTimeDuration.VirtualMachineStarting = nil
launchTimeDuration.GuestOSAgentStarting = nil

if phaseChanged {
for i := len(pts) - 1; i > 0; i-- {
pt := pts[i]
ptPrev := pts[i-1]
if pt.Phase == virtv2.MachineStarting && ptPrev.Phase == virtv2.MachinePending {
launchTimeDuration.WaitingForDependencies = &metav1.Duration{Duration: pt.Timestamp.Sub(pts[i-1].Timestamp.Time)}
break
}
}
}
case virtv2.MachineRunning:
if kvvmi != nil && emptyOSInfo == current.Status.GuestOSInfo {
if kvvmi != nil && osInfoIsEmpty(kvvmi.Status.GuestOSInfo) {
launchTimeDuration.GuestOSAgentStarting = nil
}
}

for i, pt := range pts.Items {
switch pt.Phase {
case virtv2.MachineStarting:
if i > 0 && pts.Items[i-1].Phase == phasePreviousPhase[pt.Phase] {
launchTimeDuration.WaitingForDependencies = &metav1.Duration{Duration: pt.Timestamp.Sub(pts.Items[i-1].Timestamp.Time)}
}
case virtv2.MachineRunning:
if i > 0 && pts.Items[i-1].Phase == phasePreviousPhase[pt.Phase] {
launchTimeDuration.VirtualMachineStarting = &metav1.Duration{Duration: pt.Timestamp.Sub(pts.Items[i-1].Timestamp.Time)}
}
if kvvmi != nil && emptyOSInfo == current.Status.GuestOSInfo && emptyOSInfo != kvvmi.Status.GuestOSInfo && !pt.Timestamp.IsZero() {
launchTimeDuration.GuestOSAgentStarting = &metav1.Duration{Duration: time.Now().Truncate(time.Second).Sub(pt.Timestamp.Time)}
for i := len(pts) - 1; i > 0; i-- {
pt := pts[i]
ptPrev := pts[i-1]

if pt.Phase == virtv2.MachineRunning {
if phaseChanged && ptPrev.Phase == virtv2.MachineStarting {
launchTimeDuration.VirtualMachineStarting = &metav1.Duration{Duration: pt.Timestamp.Sub(pts[i-1].Timestamp.Time)}
}
if kvvmi != nil && osInfoIsEmpty(current.Status.GuestOSInfo) && !osInfoIsEmpty(kvvmi.Status.GuestOSInfo) && !pt.Timestamp.IsZero() {
launchTimeDuration.GuestOSAgentStarting = &metav1.Duration{Duration: time.Now().Truncate(time.Second).Sub(pt.Timestamp.Time)}
}
break
}
}
}
Expand All @@ -246,69 +256,22 @@ func (h *StatisticHandler) syncStats(current, changed *virtv2.VirtualMachine, kv
changed.Status.Stats = &stats
}

var phasePreviousPhase = map[virtv2.MachinePhase]virtv2.MachinePhase{
virtv2.MachineRunning: virtv2.MachineStarting,
virtv2.MachineStarting: virtv2.MachinePending,
virtv2.MachineStopped: virtv2.MachineStopping,
}

type PhaseTransitions struct {
Items []virtv2.VirtualMachinePhaseTransitionTimestamp
func osInfoIsEmpty(info virtv1.VirtualMachineInstanceGuestOSInfo) bool {
var emptyOSInfo virtv1.VirtualMachineInstanceGuestOSInfo
return emptyOSInfo == info
}

func NewPhaseTransitions(phaseTransitions []virtv2.VirtualMachinePhaseTransitionTimestamp, oldPhase, newPhase virtv2.MachinePhase) PhaseTransitions {
func NewPhaseTransitions(phaseTransitions []virtv2.VirtualMachinePhaseTransitionTimestamp, oldPhase, newPhase virtv2.MachinePhase) []virtv2.VirtualMachinePhaseTransitionTimestamp {
now := metav1.NewTime(time.Now().Truncate(time.Second))

phasesTransitionsMap := make(map[virtv2.MachinePhase]virtv2.VirtualMachinePhaseTransitionTimestamp, len(phaseTransitions))
for _, pt := range phaseTransitions {
phasesTransitionsMap[pt.Phase] = pt
}
if _, found := phasesTransitionsMap[newPhase]; !found || oldPhase != newPhase {
phasesTransitionsMap[newPhase] = virtv2.VirtualMachinePhaseTransitionTimestamp{
if oldPhase != newPhase {
phaseTransitions = append(phaseTransitions, virtv2.VirtualMachinePhaseTransitionTimestamp{
Phase: newPhase,
Timestamp: now,
}
}
p := newPhase
t := now.Add(-1 * time.Second)
// Since we are setting up phases based on kvvm, we may skip some of them.
// But we need to know some timestamps to generate statistics.
// Add the missing phases.
for {
if previousPhase, found := phasePreviousPhase[p]; found {
if _, found = phasesTransitionsMap[previousPhase]; !found {
phasesTransitionsMap[previousPhase] = virtv2.VirtualMachinePhaseTransitionTimestamp{
Phase: previousPhase,
Timestamp: metav1.NewTime(t),
}
t = t.Add(-1 * time.Second)
}
p = previousPhase
continue
}
break
})
}
phasesTransitionsSlice := make([]virtv2.VirtualMachinePhaseTransitionTimestamp, len(phasesTransitionsMap))
i := 0
for _, p := range phasesTransitionsMap {
phasesTransitionsSlice[i] = p
i++
if len(phaseTransitions) > 5 {
return phaseTransitions[len(phaseTransitions)-5:]
}
return PhaseTransitions{Items: phasesTransitionsSlice}
}

func (pt *PhaseTransitions) Sort() {
sort.Sort(pt)
}

func (pt *PhaseTransitions) Len() int {
return len(pt.Items)
}

func (pt *PhaseTransitions) Less(i, j int) bool {
return pt.Items[j].Timestamp.After(pt.Items[i].Timestamp.Time)
}

func (pt *PhaseTransitions) Swap(i, j int) {
pt.Items[i], pt.Items[j] = pt.Items[j], pt.Items[i]
return phaseTransitions
}
162 changes: 1 addition & 161 deletions images/virtualization-artifact/pkg/controller/vm/internal/statistic_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"context"
"fmt"
"testing"
"time"

"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
Expand All @@ -37,13 +36,6 @@ import (
virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
)

func oldTime() time.Time {
t, _ := time.Parse(time.RFC3339, "2006-01-02T15:04:05+07:00")
_, offset := time.Now().Zone()
diff := time.Duration(offset) * time.Second
return t.Add(-diff).In(time.Local)
}

func TestStatisticHandler(t *testing.T) {
scheme := apiruntime.NewScheme()
for _, f := range []func(*apiruntime.Scheme) error{
Expand All @@ -66,113 +58,7 @@ func TestStatisticHandler(t *testing.T) {
expect func(vm *virtv2.VirtualMachine) error
}{
{
name: "test1: virtualmachine running, if nil statistics",
getObjects: func() []client.Object {
return []client.Object{
createPod(namespacedName),
createVM(namespacedName, virtv2.MachinePending, nil, virtv1.VirtualMachineInstanceGuestOSInfo{}),
createKVVMI(namespacedName, virtv1.Running, virtv1.VirtualMachineInstanceGuestOSInfo{Name: "test"}),
}
},
mutateChanged: func(vm *virtv2.VirtualMachine) {
if vm != nil {
vm.Status.Phase = virtv2.MachineRunning
}
},
expect: func(vm *virtv2.VirtualMachine) error {
if vm == nil || vm.Status.Stats == nil {
return fmt.Errorf("expected vm or stats to not be nil")
}
expectPhasesTransitions := []virtv2.MachinePhase{
virtv2.MachinePending,
virtv2.MachineStarting,
virtv2.MachineRunning,
}
if err := checkPhasesTransitions(t, expectPhasesTransitions, vm); err != nil {
return err
}
require.NotNil(t, vm.Status.Stats)
require.NotNil(t, vm.Status.Stats.LaunchTimeDuration.WaitingForDependencies)
require.NotNil(t, vm.Status.Stats.LaunchTimeDuration.VirtualMachineStarting)
require.NotNil(t, vm.Status.Stats.LaunchTimeDuration.GuestOSAgentStarting)
return nil
},
},
{
name: "test2: virtualmachine running, statistic should no change",
getObjects: func() []client.Object {
info := virtv1.VirtualMachineInstanceGuestOSInfo{Name: "test"}
return []client.Object{
createPod(namespacedName),
createVM(namespacedName, virtv2.MachineRunning, createStatisticNoChange(), info),
createKVVMI(namespacedName, virtv1.Running, info),
}
},
mutateChanged: func(vm *virtv2.VirtualMachine) {},
expect: func(vm *virtv2.VirtualMachine) error {
if vm == nil || vm.Status.Stats == nil {
return fmt.Errorf("expected vm or stats to not be nil")
}
stats := createStatisticNoChange()
require.Equal(t, stats.PhasesTransitions, vm.Status.Stats.PhasesTransitions)
require.Equal(t, stats.LaunchTimeDuration, vm.Status.Stats.LaunchTimeDuration)
return nil
},
},
{
name: "test3: .Stats.LaunchTimeDuration.WaitingForDependencies was should changed",
getObjects: func() []client.Object {
info := virtv1.VirtualMachineInstanceGuestOSInfo{}
return []client.Object{
createPod(namespacedName),
createVM(namespacedName,
virtv2.MachinePending,
&virtv2.VirtualMachineStats{
PhasesTransitions: []virtv2.VirtualMachinePhaseTransitionTimestamp{
{
Phase: virtv2.MachinePending,
Timestamp: metav1.NewTime(oldTime()),
},
{
Phase: virtv2.MachineStarting,
Timestamp: metav1.NewTime(oldTime().Add(10 * time.Second)),
},
},
LaunchTimeDuration: virtv2.VirtualMachineLaunchTimeDuration{
WaitingForDependencies: &metav1.Duration{Duration: 10 * time.Second},
},
},
info),
createKVVMI(namespacedName, virtv1.Scheduling, info),
}
},
mutateChanged: func(vm *virtv2.VirtualMachine) {
if vm == nil {
return
}
vm.Status.Phase = virtv2.MachineStarting
},
expect: func(vm *virtv2.VirtualMachine) error {
if vm == nil || vm.Status.Stats == nil {
return fmt.Errorf("expected vm or stats to not be nil")
}
expectPhasesTransitions := []virtv2.MachinePhase{
virtv2.MachinePending,
virtv2.MachineStarting,
}
if err := checkPhasesTransitions(t, expectPhasesTransitions, vm); err != nil {
return err
}
require.NotNil(t, vm.Status.Stats)
wfd := vm.Status.Stats.LaunchTimeDuration.WaitingForDependencies
require.NotNil(t, wfd)
require.Greater(t, wfd.Duration, 10*time.Second)

return nil
},
},
{
name: "test4: check generated .status.resources",
name: "test1: check generated .status.resources",
getObjects: func() []client.Object {
return []client.Object{
createPod(namespacedName),
Expand Down Expand Up @@ -213,52 +99,6 @@ func TestStatisticHandler(t *testing.T) {
}
}

func checkPhasesTransitions(t *testing.T, expectPhasesTransitions []virtv2.MachinePhase, vm *virtv2.VirtualMachine) error {
if vm == nil || vm.Status.Stats == nil {
return fmt.Errorf("expected vm or stats to not be nil")
}
var pts []virtv2.MachinePhase
timestamp := oldTime().Add(-24 * time.Hour)
for _, pt := range vm.Status.Stats.PhasesTransitions {
if pt.Timestamp.After(timestamp) {
timestamp = pt.Timestamp.Time
} else {
return fmt.Errorf("wrong sort phases")
}
pts = append(pts, pt.Phase)
}
require.Equal(t, expectPhasesTransitions, pts)
return nil
}

func createStatisticNoChange() *virtv2.VirtualMachineStats {
return &virtv2.VirtualMachineStats{
PhasesTransitions: []virtv2.VirtualMachinePhaseTransitionTimestamp{
{
Phase: virtv2.MachinePending,
Timestamp: metav1.Time{Time: oldTime()},
},
{
Phase: virtv2.MachineStarting,
Timestamp: metav1.Time{Time: oldTime().Add(1 * time.Second)},
},
{
Phase: virtv2.MachineRunning,
Timestamp: metav1.Time{Time: oldTime().Add(2 * time.Second)},
},
{
Phase: virtv2.MachineStopping,
Timestamp: metav1.Time{Time: oldTime().Add(3 * time.Second)},
},
},
LaunchTimeDuration: virtv2.VirtualMachineLaunchTimeDuration{
WaitingForDependencies: &metav1.Duration{Duration: 1 * time.Second},
VirtualMachineStarting: &metav1.Duration{Duration: 1 * time.Second},
GuestOSAgentStarting: &metav1.Duration{Duration: 1 * time.Second},
},
}
}

func createPod(vmKey types.NamespacedName) *corev1.Pod {
return &corev1.Pod{
TypeMeta: metav1.TypeMeta{
Expand Down

0 comments on commit e081e9a

Please sign in to comment.