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

Rebuild RHEL facts on Vagrant continued #302

Closed
wants to merge 18 commits into from
Closed

Rebuild RHEL facts on Vagrant continued #302

wants to merge 18 commits into from

Conversation

zilchms
Copy link
Contributor

@zilchms zilchms commented Mar 15, 2024

As @yakatz is constricted on time right now, I contiued on his PR and tried to fix the comments of the reviews while preserving the purpose of his PR. This includes a rebase on the current master branch.

Includes: #277

@zilchms
Copy link
Contributor Author

zilchms commented Mar 15, 2024

@yakatz can I get you to spend 5 minutes reviewing this real quick and giving it a thumbs up, if you think its ok? If this gets merged your PR will be merged as well, just as a heads-up :)

@yakatz
Copy link
Contributor

yakatz commented Mar 17, 2024

I will try to look at it tonight (I was also sick last week and got even further behind). Did you generate the facts using the script or make the changes by hand? Hand changes are probably fine, but will get messed up next time the script is run - I was trying to work out the bugs through the script...

@zilchms
Copy link
Contributor Author

zilchms commented Mar 18, 2024

I removed the changes that basically only changed device sizes etc by hand, those will change for everyone using the script everytime anyway (at least I think so). Some of the Virtualbox vs oVirt values might need changes, so those could be added. I tried to make everything as consistent as possible, but might have made some mistakes/ missed some stuff.
Good luck and get well soon (or stay well) :)

@yakatz
Copy link
Contributor

yakatz commented Mar 18, 2024

I just see an issue with the Vagrantfile itself

@zilchms
Copy link
Contributor Author

zilchms commented Mar 18, 2024

Ok, what issue do you see? I havent touched that, that should be your originial PRs vagrantfile I think (unless I overlooked something while rebasing)

@yakatz
Copy link
Contributor

yakatz commented Mar 18, 2024

Looks like a rebase issue then - the rhel-9 section is there twice

@yakatz
Copy link
Contributor

yakatz commented Mar 19, 2024

I am working on this again from scratch using the scripts - not touching anything manually - so that it will be easily reproducible in the future.

@yakatz
Copy link
Contributor

yakatz commented Mar 19, 2024

Fresh eyes are always good - I found the real reason the IPv6 facts are missing: these RHEL "boxes" ship with net.ipv6.conf.all.disable_ipv6 = 1. Just changing that automatically reenabled IPv6 and the facts are properly created.

@zilchms
Copy link
Contributor Author

zilchms commented Mar 19, 2024

discontinued in favor of #304

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