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.

Sign up for GitHub

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

Jump to bottom

Test: prepare for Fedora atomic #2231

Closed
wants to merge 10 commits into from
Closed

Test: prepare for Fedora atomic #2231

wants to merge 10 commits into from

Conversation

dperpeet
Copy link
Contributor

  • support qcow2 images instead of tarball (can't extract kernel from atomic image)
  • add init iso for atomic image (boot)
  • update check-login to work with atomic
  • fix check-login to change pam smarter, and also /etc/pam.d/sshd
  • add timeout to expect_reload in testlib

@dperpeet dperpeet changed the title WIP Test: prepare for Fedora atomic Test: prepare for Fedora atomic Apr 30, 2015
@dperpeet dperpeet changed the title Test: prepare for Fedora atomic WIP Test: prepare for Fedora atomic May 1, 2015
@dperpeet dperpeet force-pushed the fedora_atomic branch 4 times, most recently from a6cb9a9 to 6edef18 Compare May 8, 2015 14:18
@dperpeet
Copy link
Contributor Author

dperpeet commented May 8, 2015

Using libguestfs to update the files during vm-install didn't prove successful. Instead, we copy rpms to the vm (as we always do) and extract the files from there to the filesystem.

@dperpeet
Copy link
Contributor Author

dperpeet commented May 8, 2015

successful tests on atomic:
check-accounts
check-dashboard
check-docker
check-journal (remount rw for test service)
check-kubernetes
check-login
check-menu
check-multi-machine (remount rw for magic inserted into web pages)
check-networking
check-pages
check-reauthorize
check-roles
check-services (remount rw for test service)
check-session
check-shutdown-restart
check-storage
check-system-info
check-terminal

skipped:
check-connection (cockpit-testing, TLS)
check-dbus (test-server libssh dep)
check-loopback (the container uses this anyway)
check-realms (missing bind-utils on atomic host - nslookup, realm commands)

@stefwalter
Copy link
Contributor

check-dashboard check-multi-machine

Lack of /var/lib/cockpit? If so, that's a real bug.

check-dbus check-reauthorize

Need cockpit-test-assets

@dperpeet
Copy link
Contributor Author

dperpeet commented May 9, 2015

Need cockpit-test-assets

thanks for catching that, test-assets are now included as well

@dperpeet dperpeet force-pushed the fedora_atomic branch 5 times, most recently from 613644d to bc30292 Compare May 29, 2015 11:34
mvollmer
mvollmer reviewed May 29, 2015
View reviewed changes
test/check-login
@@ -77,11 +62,15 @@ class TestLogin(MachineCase):

# Try to login as user with correct password
login ("user", "abcdefg")
b.wait_text("#login-error-message", "Permission denied")
if 'atomic' in m.os:
b.wait_in_text("body", "connect or authenticate: terminated")
Copy link
Member

Choose a reason for hiding this comment

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

Could we fix Cockpit to also produce "Permission denied" on Atomic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This probably relates to using ssh under the hood. I agree: a proper error message would be good here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

follow-up issue #2402

@dperpeet dperpeet force-pushed the fedora_atomic branch 4 times, most recently from 9a22dd3 to 4122a73 Compare June 3, 2015 00:52
mvollmer
mvollmer reviewed Jun 4, 2015
View reviewed changes
test/testvm.py
@@ -158,7 +188,8 @@ def build(self, args):

def run_setup_script(self, script, args):
"""Prepare a test image further by running some commands in it."""
self.start(maintain=True)
# run this on the original image, not the one in the run directory
self.start(maintain=True, original=True)
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, this looks wrong. The original image in $TEST_DATA/images should never be changed, except by vm-save and vm-download.

This is so that vm-create+vm-install+check-verify can produce new images in ./run without disturbing the 'golden' original image in $TEST_DATA/images. Only once we have decided that the new image is good to go will we run ./vm-save manually to replace the old golden image with a new golden image.

(With hubbot, getting a new golden image published is a bit more convoluted, but we could clean that up, actually.)

All activity should happen with the image in ./run. When starting a VM with the image in ./run, the changes made from within that VM are either saved back to the image (during vm-create and vm-install) or thrown away (during check-*).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're right - I didn't consider our hubbot setup. In my testing environment, I don't clean up ./run before I create a new image.

mvollmer
mvollmer reviewed Jun 4, 2015
View reviewed changes
test/testvm.py
# this image will be created on top of the new base image
if os.path.exists(self._image_image):
os.unlink(self._image_image)
subprocess.check_call([ bootstrap_script, self._image_original, self.arch ])
Copy link
Member

Choose a reason for hiding this comment

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

Using _image_original is wrong.

@mvollmer
Copy link
Member

mvollmer commented Jun 4, 2015

check-roles fails because /var/log/lastlog does not exist. Adding

m.execute("touch /var/log/lastlog")

at the beginning of testDynamic makes it work.

However, we should change cockpit-accounts.js to survive without /var/log/lastlog. I think simply not opening the "unexpected error" dialog should be enough.

@mvollmer
Copy link
Member

mvollmer commented Jun 4, 2015

#2397 should fix check-roles.

@mvollmer mvollmer added the blocked Don't land until something else happens first (see task list) label Jun 4, 2015
@dperpeet dperpeet removed the blocked Don't land until something else happens first (see task list) label Jun 4, 2015
@dperpeet dperpeet changed the title WIP Test: prepare for Fedora atomic Test: prepare for Fedora atomic Jun 5, 2015
@dperpeet dperpeet removed the needswork label Jun 5, 2015
@dperpeet dperpeet mentioned this pull request Jun 5, 2015
Closed
@dperpeet
Copy link
Contributor Author

dperpeet commented Jun 5, 2015

all previous comments addressed or posted as follow-up

stefwalter
stefwalter reviewed Jun 5, 2015
View reviewed changes
test/testlib.py
"""Start Cockpit.

Cockpit is not running when the test virtual machine starts up, to
allow you to make modifications before it starts.
"""
self.machine.execute("systemctl start cockpit-testing.socket")
if "atomic" in self.machine.os:
# HACK
Copy link
Contributor

Choose a reason for hiding this comment

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

@dperpeet Have you filed a bug about this upstream? If so, can you include the link here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@dperpeet dperpeet closed this in 03cb7e3 Jun 8, 2015
@dperpeet dperpeet deleted the fedora_atomic branch June 12, 2015 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@dperpeet @stefwalter @mvollmer