Skip to content

Commit

Permalink
launcher: fix prints for the domain events
Browse files Browse the repository at this point in the history 
In the previous version, the launcher was storing the previous status
and reason values from the domain, and then fetching the new state.

However, the domain status was always empty in the eventCallback when it
was called by the case 'eventChan' because the domain is constructed by
the function NewDomainFromName which doesn't populate the Status field.

This resulted in wrong prints. For example, we were always printing the
domain status, even if it didn't changed with verbosity:2, and the old
status and reason were always empty strings.

Example of the print when the domain pass from Paused to Running because
the migration completed:
  - old wrong print:
  {"component":"virt-launcher","level":"info","msg":"kubevirt domain status:
  Paused(3):Migration(2)","pos":"client.go:296","timestamp":"2025-01-09T12:31
  :55.568979Z"}
  [...]
  {"component":"virt-launcher","level":"info","msg":"kubevirt domain status:
  Running(1):Unknown(2)","pos":"client.go:296","timestamp":"2025-01-09T12:31
  :55.572320Z"

  - new correct print:
  {"component":"virt-launcher","level":"info","msg":"kubevirt domain status:
  Running(Paused) reason: Unknown(Migration)","pos":"client.go:251","timestamp
  ":"2025-01-09T12:26:18.616955Z"}

Signed-off-by: Alice Frosi <[email protected]>
  • Loading branch information
@alicefr
alicefr committed Jan 13, 2025
1 parent 02db160 commit c9317a1
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 22 deletions.
43 changes: 27 additions & 16 deletions pkg/virt-launcher/notify-client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,26 @@ func newWatchEventError(err error) watch.Event {
return watch.Event{Type: watch.Error, Object: &metav1.Status{Status: metav1.StatusFailure, Message: err.Error()}}
}

func eventCallback(c cli.Connection, domain *api.Domain, libvirtEvent libvirtEvent, client *Notifier, events chan watch.Event,
type eventCaller struct {
domainStatus api.LifeCycle
domainStatusChangeReason api.StateChangeReason
}

func (e *eventCaller) printStatus(status *api.DomainStatus) {
v := 2
if status.Status == e.domainStatus && status.Reason == e.domainStatusChangeReason {
// Status hasn't changed so log only in higher verbosity.
v = 3
}
log.Log.V(v).Infof("kubevirt domain status: %v(%v) reason: %v(%v)", status.Status, e.domainStatus, status.Reason, e.domainStatusChangeReason)
}

func (e *eventCaller) updateStatus(status *api.DomainStatus) {
e.domainStatus = status.Status
e.domainStatusChangeReason = status.Reason
}

func (e *eventCaller) eventCallback(c cli.Connection, domain *api.Domain, libvirtEvent libvirtEvent, client *Notifier, events chan watch.Event,
interfaceStatus []api.InterfaceStatus, osInfo *api.GuestOSInfo, vmi *v1.VirtualMachineInstance, fsFreezeStatus *api.FSFreeze,
metadataCache *metadata.Cache) {

Expand All @@ -252,12 +271,6 @@ func eventCallback(c cli.Connection, domain *api.Domain, libvirtEvent libvirtEve
} else {
defer d.Free()

// Remember current status before it will be changed.
var (
prevStatus = domain.Status.Status
prevReason = domain.Status.Reason
)

// No matter which event, try to fetch the domain xml
// and the state. If we get a IsNotFound error, that
// means that the VirtualMachineInstance was removed.
Expand Down Expand Up @@ -289,12 +302,8 @@ func eventCallback(c cli.Connection, domain *api.Domain, libvirtEvent libvirtEve
domain.Spec = *spec
}

if domain.Status.Status == prevStatus && domain.Status.Reason == prevReason {
// Status hasn't changed so log only in higher verbosity.
log.Log.V(3).Infof("kubevirt domain status: %v(%v):%v(%v)", domain.Status.Status, status, domain.Status.Reason, reason)
} else {
log.Log.Infof("kubevirt domain status: %v(%v):%v(%v)", domain.Status.Status, status, domain.Status.Reason, reason)
}
e.printStatus(&domain.Status)
e.updateStatus(&domain.Status)
}

switch domain.Status.Reason {
Expand Down Expand Up @@ -414,12 +423,14 @@ func (n *Notifier) StartDomainNotifier(
var interfaceStatuses []api.InterfaceStatus
var guestOsInfo *api.GuestOSInfo
var fsFreezeStatus *api.FSFreeze
var eventCaller eventCaller

for {
select {
case event := <-eventChan:
metadataCache.ResetNotification()
domainCache = util.NewDomainFromName(event.Domain, vmi.UID)
eventCallback(domainConn, domainCache, event, n, deleteNotificationSent, interfaceStatuses, guestOsInfo, vmi, fsFreezeStatus, metadataCache)
eventCaller.eventCallback(domainConn, domainCache, event, n, deleteNotificationSent, interfaceStatuses, guestOsInfo, vmi, fsFreezeStatus, metadataCache)
log.Log.Infof("Domain name event: %v", domainCache.Spec.Name)
if event.AgentEvent != nil {
if event.AgentEvent.State == libvirt.CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_STATE_CONNECTED {
Expand All @@ -434,7 +445,7 @@ func (n *Notifier) StartDomainNotifier(
guestOsInfo = agentUpdate.DomainInfo.OSInfo
fsFreezeStatus = agentUpdate.DomainInfo.FSFreezeStatus

eventCallback(domainConn, domainCache, libvirtEvent{}, n, deleteNotificationSent,
eventCaller.eventCallback(domainConn, domainCache, libvirtEvent{}, n, deleteNotificationSent,
interfaceStatuses, guestOsInfo, vmi, fsFreezeStatus, metadataCache)
case <-reconnectChan:
n.SendDomainEvent(newWatchEventError(fmt.Errorf("Libvirt reconnect, domain %s", domainName)))
Expand All @@ -447,7 +458,7 @@ func (n *Notifier) StartDomainNotifier(
util.DomainFromNamespaceName(domainCache.ObjectMeta.Namespace, domainCache.ObjectMeta.Name),
vmi.UID,
)
eventCallback(
eventCaller.eventCallback(
domainConn,
domainCache,
libvirtEvent{},
Expand Down
16 changes: 10 additions & 6 deletions pkg/virt-launcher/notify-client/notify_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ var _ = Describe("Notify", func() {
var mockDomain *cli.MockVirDomain
var mockCon *cli.MockConnection
var ctrl *gomock.Controller
var e *eventCaller

BeforeEach(func() {
ctrl = gomock.NewController(GinkgoT())
Expand All @@ -76,6 +77,7 @@ var _ = Describe("Notify", func() {
stopped = false
shareDir, err = os.MkdirTemp("", "kubevirt-share")
Expect(err).ToNot(HaveOccurred())
e = &eventCaller{}

go func() {
notifyserver.RunServer(shareDir, stop, eventChan, nil, nil)
Expand Down Expand Up @@ -107,7 +109,7 @@ var _ = Describe("Notify", func() {
mockDomain.EXPECT().GetName().Return("test", nil).AnyTimes()
mockDomain.EXPECT().GetXMLDesc(gomock.Eq(libvirt.DomainXMLFlags(0))).Return(string(x), nil)

eventCallback(mockCon, util.NewDomainFromName("test", "1234"), libvirtEvent{Event: &libvirt.DomainEventLifecycle{Event: event}}, client, deleteNotificationSent, nil, nil, nil, nil, metadataCache)
e.eventCallback(mockCon, util.NewDomainFromName("test", "1234"), libvirtEvent{Event: &libvirt.DomainEventLifecycle{Event: event}}, client, deleteNotificationSent, nil, nil, nil, nil, metadataCache)

timedOut := false
timeout := time.After(2 * time.Second)
Expand Down Expand Up @@ -138,7 +140,7 @@ var _ = Describe("Notify", func() {
mockDomain.EXPECT().GetState().Return(libvirt.DOMAIN_NOSTATE, -1, libvirt.Error{Code: libvirt.ERR_NO_DOMAIN})
mockDomain.EXPECT().GetName().Return("test", nil).AnyTimes()

eventCallback(mockCon, util.NewDomainFromName("test", "1234"), libvirtEvent{Event: &libvirt.DomainEventLifecycle{Event: libvirt.DOMAIN_EVENT_UNDEFINED}}, client, deleteNotificationSent, nil, nil, nil, nil, metadataCache)
e.eventCallback(mockCon, util.NewDomainFromName("test", "1234"), libvirtEvent{Event: &libvirt.DomainEventLifecycle{Event: libvirt.DOMAIN_EVENT_UNDEFINED}}, client, deleteNotificationSent, nil, nil, nil, nil, metadataCache)

timedOut := false
timeout := time.After(2 * time.Second)
Expand Down Expand Up @@ -178,7 +180,7 @@ var _ = Describe("Notify", func() {
},
}

eventCallback(mockCon, util.NewDomainFromName("test", "1234"), libvirtEvent{}, client, deleteNotificationSent, interfaceStatus, nil, nil, nil, metadataCache)
e.eventCallback(mockCon, util.NewDomainFromName("test", "1234"), libvirtEvent{}, client, deleteNotificationSent, interfaceStatus, nil, nil, nil, metadataCache)

timedOut := false
timeout := time.After(2 * time.Second)
Expand Down Expand Up @@ -209,7 +211,7 @@ var _ = Describe("Notify", func() {
Name: guestOsName,
}

eventCallback(mockCon, util.NewDomainFromName("test", "1234"), libvirtEvent{}, client, deleteNotificationSent, nil, &osInfoStatus, nil, nil, metadataCache)
e.eventCallback(mockCon, util.NewDomainFromName("test", "1234"), libvirtEvent{}, client, deleteNotificationSent, nil, &osInfoStatus, nil, nil, metadataCache)

timedOut := false
timeout := time.After(2 * time.Second)
Expand Down Expand Up @@ -239,7 +241,7 @@ var _ = Describe("Notify", func() {
Status: fsFrozenStatus,
}

eventCallback(mockCon, util.NewDomainFromName("test", "1234"), libvirtEvent{}, client, deleteNotificationSent, nil, nil, nil, &fsFreezeStatus, metadataCache)
e.eventCallback(mockCon, util.NewDomainFromName("test", "1234"), libvirtEvent{}, client, deleteNotificationSent, nil, nil, nil, &fsFreezeStatus, metadataCache)

timedOut := false
timeout := time.After(2 * time.Second)
Expand All @@ -265,6 +267,7 @@ var _ = Describe("Notify", func() {
var client *Notifier
var recorder *record.FakeRecorder
var vmiStore cache.Store
var e *eventCaller

BeforeEach(func() {
stop = make(chan struct{})
Expand All @@ -278,6 +281,7 @@ var _ = Describe("Notify", func() {
recorder.IncludeObject = true
vmiInformer, _ := testutils.NewFakeInformerFor(&v1.VirtualMachineInstance{})
vmiStore = vmiInformer.GetStore()
e = &eventCaller{}

go func() {
notifyserver.RunServer(shareDir, stop, eventChan, recorder, vmiStore)
Expand Down Expand Up @@ -341,7 +345,7 @@ var _ = Describe("Notify", func() {
eventReason := "IOerror"
eventMessage := "VM Paused due to not enough space on volume: "
metadataCache := metadata.NewCache()
eventCallback(mockCon, domain, libvirtEvent{}, client, deleteNotificationSent, nil, nil, vmi, nil, metadataCache)
e.eventCallback(mockCon, domain, libvirtEvent{}, client, deleteNotificationSent, nil, nil, vmi, nil, metadataCache)
event := <-recorder.Events
Expect(event).To(Equal(fmt.Sprintf("%s %s %s involvedObject{kind=VirtualMachineInstance,apiVersion=kubevirt.io/v1}", eventType, eventReason, eventMessage)))
})
Expand Down

0 comments on commit c9317a1

Please sign in to comment.