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

Fix Virtlet restarts #867

Merged
merged 10 commits into from
Feb 21, 2019
Merged

Fix Virtlet restarts #867

merged 10 commits into from
Feb 21, 2019

Conversation

ivan4th
Copy link
Contributor

@ivan4th ivan4th commented Feb 20, 2019

  • fix virtlet restart e2e tests
  • fix Virtlet restart problem due to wrong emulator cgroups
  • fix Virtlet crash due to bogus File objects in metadata db (started to happen after Support reopen tap FD if something wrong to VM #864)
  • make virtlet restart e2e run on CircleCI
  • re-verify the changes on a real cluster

This change is Reviewable

Ivan Shvedunov added 9 commits February 18, 2019 16:47
Don't depend on CirrOS output messages.
Don't use kubectl for Virtlet pod restart tests.
Don't try to attach to a VM's console with Stdin enabled (problems due
to tty), echo messages to /dev/console instead.
The culprit were cgroups that aren't handled by libvirt.
Of those, we already handle hugetlb by moving the emulator
process out of it. Still, need to do the same for systemd
(name=systemd) and pids cgroup controllers.

The problem manifested itself when cgroup-per-qos is enabled for
kubelet. This is the default, but in current kdc it may
be disabled as a workaround for old kubelet bug. This bug
is already fixed, so the workaround is to be removed soon.
Don't store bogus File objects in the metadata db.
Copy link
Contributor

@jellonek jellonek left a comment

Choose a reason for hiding this comment

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

Reviewed 14 of 14 files at r1.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @ivan4th)


tests/e2e/common.go, line 145 at r1 (raw file):

// do asserts that function with multiple return values doesn't fail
// Considering we have func `foo(something) (something, error)`

Line above this one is missing a comma if this one should be leaved as was before or period with line below this removed.
Also as extra is now required below example of use is invalid now.

@ivan4th ivan4th changed the title [WiP] Fix Virtlet restarts Fix Virtlet restarts Feb 20, 2019
Copy link
Contributor Author

@ivan4th ivan4th left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @jellonek)


tests/e2e/common.go, line 145 at r1 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

Line above this one is missing a comma if this one should be leaved as was before or period with line below this removed.
Also as extra is now required below example of use is invalid now.

Tried to clarify it. And the example is valid, len(extra) != 0 means that the function should return at least 2 values (the last one being the error). The check just makes sure the function isn't called for a function with just one return value (e.g. error)

Copy link
Contributor

@jellonek jellonek left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r2.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained

@pigmej pigmej merged commit 3db338e into master Feb 21, 2019
@pigmej pigmej deleted the ivan4th/try-to-restore-restart-e2e branch February 21, 2019 19:21
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.

3 participants