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

test: Also wait for virt-install to finish on centos-10 #1765

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

mvollmer
Copy link
Member

No description provided.

@mvollmer
Copy link
Member Author

Aha, TF centos-10-stream is green.

@mvollmer
Copy link
Member Author

@mvollmer mvollmer force-pushed the dbg-download-an-os branch from 67f81cd to 3580ad1 Compare August 13, 2024 12:12
@mvollmer mvollmer removed the no-test label Aug 13, 2024
@mvollmer mvollmer force-pushed the dbg-download-an-os branch from 3580ad1 to 53fad8a Compare August 13, 2024 12:13
@mvollmer mvollmer changed the title DBG - download an os test: Upgrade "wait for virt-install" hack to non-hack Aug 13, 2024
@mvollmer mvollmer force-pushed the dbg-download-an-os branch from 53fad8a to 4127bf5 Compare August 13, 2024 12:13
@mvollmer mvollmer requested a review from martinpitt August 13, 2024 12:14
@martinpitt
Copy link
Member

If that works, I'm all for it! I had to make it conditional back then as waiting timed out on several OSes. I've never fully understood how virt-install works -- sometimes it leaves the VM running, sometimes not, I found it rather unpredictable. Perhaps by now all OSes have a new enough virt-install.

Thanks!

@martinpitt
Copy link
Member

Yeah, what I feared -- this needs to stay conditionalized. Perhaps there's a better criterion, like the virt-install version? But for now just extending the OS list seems ok.

@mvollmer
Copy link
Member Author

Yeah, what I feared -- this needs to stay conditionalized. Perhaps there's a better criterion, like the virt-install version? But for now just extending the OS list seems ok.

Ah, I see, bummer. So the virt-install process itself sticks around forever? I have one (1) closer look and then I make this conditional.

@mvollmer mvollmer force-pushed the dbg-download-an-os branch from 4127bf5 to 7f262a2 Compare August 13, 2024 13:11
@mvollmer
Copy link
Member Author

On some OSes the created VM crashes in the
kernel and virt-install never exits, on others, the BIOS can't find a bootable
device but virt-install does exit. shrug

Maybe we should make it so that virt-install can actually produce a working VM.

@mvollmer
Copy link
Member Author

the BIOS can't find a bootable device but virt-install does exit.

The might be the state after a "successful" installation of our fake Fedora 28 OS. So virt-install is indeed done at this point.

@mvollmer mvollmer changed the title test: Upgrade "wait for virt-install" hack to non-hack test: Also wait for virt-install to finish on centos-10 Aug 13, 2024
@mvollmer mvollmer marked this pull request as ready for review August 13, 2024 13:43
@mvollmer
Copy link
Member Author

Oh, the flakes...

# well. (On some OSes the created VM crashes in the
# kernel, on others, the BIOS can't find a bootable
# device. Maybe that's the difference.)
if self.create_and_run and self.machine.image in ['opensuse-tumbleweed', 'centos-10']:
Copy link
Member

Choose a reason for hiding this comment

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

I don't have proof that this affects rhel-10-0 (or not), as the last gating run in real RHEl 10 all failed due to SELinux. But it almost surely needs the same treatment. I triggered a rhel-10-0 bots run.

Copy link
Member

Choose a reason for hiding this comment

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

The rhel-10-0 run confirms this. I suggest to write this as

self.machine.image.startswith(('opensuse-tumbleweed', 'centos-10', 'rhel-10'))

so that it also catches future rhel-10-* images.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have done that, and also increased the global timeout. I think opensuse runs into it.

@mvollmer mvollmer force-pushed the dbg-download-an-os branch from 7f262a2 to e041f37 Compare August 14, 2024 08:14
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Thanks for your investigations! The comment clarifies it a little indeed! (For our poor future selves -- this will come back and bite us in the ... a place where it hurts)

@mvollmer
Copy link
Member Author

The comment clarifies it a little indeed!

The installation is not expected to succeed, but I don't know how exactly it is expected to fail. On some OSes, a kernel is clearly started, so we must be downloading something? I think this is worth figuring out and robustifying, eventually.

@martinpitt
Copy link
Member

I reviewed the failures:

  • The virtqemud crashes in c10 and r10 are very persistent. We need to report these and hopefully find a CLI reproducer. But unrelated to this PR.
  • r10 testCopyStorageMigration is a known naughty in f40, let's copy it -- but that'll be a bots PR.
  • d-testing also fails testCreateDownloadAnOS -- given that it's also a "new" distro similar to tumbleweed and R10, my gut feeling is that we should extend the hack from this PR there. But let's do that in a follow-up.

So in summary, I think this is fine to land, it makes things incrementally better.

@martinpitt martinpitt merged commit 0aac01a into cockpit-project:main Aug 14, 2024
27 of 32 checks passed
@martinpitt
Copy link
Member

Above follow-ups: #1767 and cockpit-project/bots#6735

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants