Skip to content
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.

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

Partial fix for pod/container statuses #122

Merged
merged 2 commits into from
Nov 23, 2016
Merged

Partial fix for pod/container statuses #122

merged 2 commits into from
Nov 23, 2016

Conversation

jellonek
Copy link
Contributor

@jellonek jellonek commented Nov 22, 2016

Solves a bit issue described in #120

For containers - provides differentiation between vm not running because it was newly created/defined from vm not running because it was stopped.

In similar manner for sandboxes - if there was call to stop pod sandbox - now we are changing it's status in podstatus responses to not ready (being compatible with dockershim).

Still there is no check if stop pod sandbox was called before container/vm had really chance to clean stop - this should be added.

Still there is some issue with statuses, because after create/delete example fedora pod - virsh list shows that domain was not undefined. Next call to create same pod starts new vm (what seems to be dangerous, could overwrite some data which was not cleanly stopped) what we can see in virsh list, but whole pod is stick in:

    State:              Waiting
      Reason:           ContainerCreating

So this need further investigation.


This change is Reviewable

@ivan4th
Copy link
Contributor

ivan4th commented Nov 22, 2016

Is it possible to reproduce the problem in integration tests somehow? (this may require direct libvirt calls from integration tests themselves, but still)

@jellonek
Copy link
Contributor Author

IMO the easiest way to reproduce this in tests should be calling create, then wait for "Running" state, then call to delete pod, wait for it's removal (empty pod list in response to kubelet ListPodSandbox call) then using direct libvirt call (or external command - virsh list --all) looking if domain was really undefined.

Btw. Anothre strange thing (probably bug in libvirt) is that after pod create/delete/create/delete - virsh list --all shows nothing, but:

$ ps xa | grep qem
27332 ?        Ssl    0:02 /usr/local/bin/virtlet -v=3 -logtostderr=true -libvirt-uri=qemu+tcp://libvirt/system
29496 ?        Sl     0:07 qemu-system-x86_64 -enable-kvm -name fedora -S -machine pc-i440fx-xenial,accel=kvm,usb=off -m 1024 -realtime mlock=off -smp 8,sockets=8,cores=1,threads=1 -uuid b231314c-3934-4ec9-7e6e-ebec7efaa59c -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-fedora/monitor.sock,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -boot strict=on -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -drive file=/var/lib/libvirt/images/fedora,format=qcow2,if=none,id=drive-virtio-disk0 -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -drive file=/var/lib/virtlet/fedora__var_run_secrets_kubernetes.io_serviceaccount,format=raw,if=none,id=drive-virtio-disk2 -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk2,id=virtio-disk2 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -device usb-tablet,id=input0 -vnc 127.0.0.1:0 -device cirrus-vga,id=video0,bus=pci.0,addr=0x2 -device AC97,id=sound0,bus=pci.0,addr=0x3 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 -msg timestamp=on

@ivan4th
Copy link
Contributor

ivan4th commented Nov 23, 2016

Added followup issue for tests, let's merge this for now.


Review status: 0 of 4 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@ivan4th
Copy link
Contributor

ivan4th commented Nov 23, 2016

:lgtm:


Review status: 0 of 4 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@ivan4th ivan4th merged commit e82bfcf into master Nov 23, 2016
@jellonek jellonek deleted the jell/statuses branch November 24, 2016 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants