-
Notifications
You must be signed in to change notification settings - Fork 92
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
Add support for repository rpms in HE setup #184
base: master
Are you sure you want to change the base?
Conversation
I need to clean up some naming in this before it is ready for a formal review. Just wanted to get it out there incase someone else hits the same issue. |
493c5d0
to
4364cef
Compare
I added a new variable to turn on this functionality. I also moved it into what I think will be a more acceptable location. I think this is ready for someone to look at. @arachmani If you have any comments or would like changes please let me know. |
ci add to whitelist |
ci test please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kresss Thank you for your contribution!
We need to support both upstream and downstream, which means we need to pass some vars to ovirt-repositories, so I think it's a bit complex to do that here.
For those cases we have these options https://github.com/oVirt/ovirt-ansible-collection/tree/master/roles/hosted_engine_setup#make-changes-in-the-engine-vm-during-the-deployment, so we can use hooks or connect to the localVM and configure it manually.
BTW, did you encounter any issue that this PR should fix?
AFAICT, localVM already has the ovirt repos.
@arachmani Yes we are having issues installing RHV with a HE in an offline environment without satellite. We tracked it all down to the repos not being present. We have offline repos setup with all the packages. We created a ovirt repos rpm file and it all works well. I think we would still have some issues if we were using satellite in offline, but I don't have that setup to do the testing. With the patch as it is submitted, you have to set the he_apply_repositories_role == true and set all of the ovirt_repositories* variables that you want. I should state that in the README line. I didn't remap all of the ovirt_repositories_* variables since I was applying the whole role. I saw in other areas where the hosted_engine_setup role calls only partial task files from other roles, but I didn't feel like that was needed in this case and could only add to confusion. I noticed in the example for the engine_setup role that you applied the repositories role first. That is what lead me down this path. I didn't know about the hooks directory at the time. In the end we expect that we would receive an RPM with this role from RH as part of our normal subscription. I would rather not have to install additional hooks into the collection structure after that before we use the roles. Having to modify the collections under /usr/share/ansible/collections is not preferred. That said, If you decide to not accept the PR, it sounds like that would be a more maintainable workaround than patching the plays. Let me know if you would like to proceed. Thank you, Seth. |
ci test please |
Hello contributor, thanks for submitting a PR for this project! I am the bot who triggers "standard-CI" builds for this project. In order to allow automated tests to run, please ask one of the project maintainers to review the code and then do one of the following:
|
ci test please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check out the changes around ansible.
As for the logic, if we need to import the role there, I'll keep it up to @arachmani
roles/hosted_engine_setup/tasks/bootstrap_local_vm/03_engine_initial_tasks.yml
Outdated
Show resolved
Hide resolved
roles/hosted_engine_setup/tasks/bootstrap_local_vm/03_engine_initial_tasks.yml
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Should I squash the commits into a single commit for a merge? |
You don't need to do that, the Github can do it automatically when mering. |
@kresss for this kind of situation you have the |
@arachmani First I have to admit that I have not tried this with upstream ovirt, only RHV 4.4. I still think there is a gap here. When we do an install with We are using RHEL & RHV. We can't use subscription mangement because we are in a disconnected network without a Satellite server. We do have copies of RH yum repos. Yes we have subscriptions ;) As for more parameters, what are you refering too? If I am missing some extra parameter that makes the offline deployment work, please share. |
The
I need to check that, I'm not familiar with the |
Yes we are using OpenSCAP via As for the extra varaibles for the repositories role, we are using these:
It is important to note that we built a custom ovirt-release-rpm which installs some repo files in Also fair warning, even if you use the repositories role like I do with this PR and everything 'works', RHV HE still fails. Due to a FIPS bug. See: https://bugzilla.redhat.com/show_bug.cgi?id=1875363 @mwperina already submitted a patch for bz 1875363, but it hasn't been released out yet. |
So, is this PR basically a workaround for the bugs we have mentioned and the fixes that didn't release yet? |
@arachmani Trying to work around the open bugs is how I got started with this PR. I didn't know about the bugs at the time, but they have been discovered along the way. I am currently waiting for RHV 4.4.4.2 to drop so I can test everything again with the latest release of ovirt-ansible-collection. I will be happy if everything works at that point and this is not needed. The question would then be, does this PR still have merit? The current HE-setup role enables users to install extra packages as part of the setup should it also enable them to configure the repositories for potential in-house packages? Or is it good enough to point those users down the hooks path? |
This PR covers a specific case when offline deployment is used with an internal repo. |
If it worked, I wouldn't be here. Then again it is kind of Red Hat's thing to say things work but then say, we have a bug open for that. Once all the bugs are resolved with RHV 4.4.4.2, I will try this again. That probably will not be for you know a few more months, but who wanted to actually be able to use the product anyways. If needed I will use the hooks, but don't tell me that it works. |
@kresss can you please resolve conflicts? |
@sandrobonazzola - I will add this to my list for Monday. |
@kresss we have 4.5 beta release coming in 4 days, if you resolve the conflicts we can try to get it in. |
The GitHub actions have a bit of problem nowadays and sometimes don't wanna start. |
@kresss can you please rebase resolving conflicts? |
@sandrobonazzola yeah I can get on this tomorrow. |
When setting up an ofline ovirt environment requires a repository to be setup. This change uses the existing ovirt.ovirt.repositories role to setup the repositories using a repositories RPM.