-
Notifications
You must be signed in to change notification settings - Fork 82
Conversation
Though now I checked that the master branch still has the "issue" for which I created the fork from a past revision in the first place: While the package installs, the python module does not load:
Since deepmerge still doesn't seem to be part of the EL8 standard repositories, there seem to be really three options:
|
FWIW, this is why I prefer the curl based install, it installs the lib via pip. Anyway, I originally did not use deep merge, so I know it's possible not to use it. If you want to open a PR to remove its use, that's fine. Or I can do it. But it has to be backwards compatible with the current format of the metadata, and more importantly, the instance data file. |
This patch updates the Makefile and Dockerfile.rpmbuild to support building the RPM for CentOS 8. This patch also includes a few, small addendums required to make the previous patches work for CentOS 8.
That is probably the easy way, but does not work too nicely with disconnected install (no direct access to internet), like if a company uses internal package repositories/mirrors for software. Bypassing the OS package manager also makes it harder to identify installed software (for security/compliance reasons) or update in case of security issues. Technically it is possible to use pre/post scripts in an RPM to download libraries or do pretty much anything but that is generally a really bad idea and breaks some of the above mentioned use cases.
If the library is easy enough to replace with an in-repo implementation, that would in my opinion be the cleanest way to solve the issue. I haven't looked into the details of how it is used or what it would take to replace it so at least for now I won't promise to make that change. I noticed that there was also some interesting in moving this provider into the cloud-init core and these dependencies were also blocking that. If that is an approach you're interested in going for in the future, removing deepmerge could be a step towards that as well |
Do you think we should block this PR until the deep merge issue is resolved? I am fine merging this as-is and then not publishing some RPM until the issue is resolved, or updating the README to include the proviso. What do you think? |
I don't really have a strong opinion towards either option. While the RPM SPEC alone is not particularly useful, including it doesn't seem too harmful either. |
I think maybe the best option here is to use util.mergemanydict(). This removes the deepmerge dependency and brings the module in line with many of the built in datasource modules. |
This issue is being closed because this datasource has been merged into cloud-init (canonical/cloud-init#953):
In order to participate in the growth of this datasource moving forward, please:
Once again, many thanks to the wonderful community that has grown around this datasource, and I look forward to seeing everyone in the new cloud-init forums! |
Add a SPEC file which can be used to build RPMs for CentOS and RHEL 8.
Based on request from @akutz to upstream changes in https://github.com/varesa/cloud-init-vmware-guestinfo/tree/el8 (19ee229)
This PR drops the README change (not relevant) and squashes the other two commits into one (with the other one being a minor fix)
Verified that this slightly modified version succesfully builds an RPM using akutz@35f0351 and that the resulting package installs in a
centos:8
containerEDIT: Appended to include akutz@35f0351 as well