added handling for adding rpms to an image as well as the relevant tests #28
Conversation
|
Question:
|
pkg/build/build.go
Outdated
| @@ -85,10 +93,16 @@ func (b *Builder) prepareBuildDir() error { | |||
| b.eibBuildDir = b.buildConfig.BuildDir | |||
| } | |||
| b.combustionDir = filepath.Join(b.eibBuildDir, "combustion") | |||
| b.rpmBuildDir = filepath.Join(b.combustionDir, "rpms") | |||
There was a problem hiding this comment.
Why not the same directory like: https://documentation.suse.com/sle-micro/5.5/html/SLE-Micro-all/cha-images-combustion.html
There was a problem hiding this comment.
My gut reaction was that everything would be in a flat combustion directory. That might get a bit messy for debugging large builds, but I think we should start with that model and only make it more complex if we find a need to. So +1 to Alberto's comment.
There was a problem hiding this comment.
sounds good to me, my initial thinking was just maintaining the organization from the config dir which I commented on in another reply, but I'll make this change
pkg/build/build.go
Outdated
| @@ -23,6 +23,9 @@ type Builder struct { | |||
| eibBuildDir string | |||
| combustionDir string | |||
| combustionScripts []string | |||
| rpmBuildDir string | |||
There was a problem hiding this comment.
I would use the same directory than combustion dir
pkg/build/rpm.go
Outdated
| func (b *Builder) getRPMFileNames() error { | ||
| b.rpmSourceDir = filepath.Join(b.buildConfig.ImageConfigDir, "rpms") | ||
|
|
||
| rpms, err := os.ReadDir(b.rpmSourceDir) |
There was a problem hiding this comment.
For this one, "rpmSourceDir" refers to the user-created configuration directory. In here, I think it makes it significantly easier for us when the user knows to make an RPM dir and put all of their RPMs in there to be copied over to the image, though I'm interested to see other views
|
So a few things, first, I added a test for the situation where the rpm dir is empty, as well as handling for that, let me know how you guys feel about that. Also, because I decided to keep the rpm dir in "testData" using As for the other tests, I'm not sure if there was a more graceful way to go about what I did, so let me know if what I did was fine or if it's not convential. The main reason I didn't use |
copyFile is no longer attached to the build struct. getRPMFileNames now checks for the .rpm extension, to remove the previously unnecessary (and frankly ugly) logic. I also added more abstraction for readability. Finally, cleaned up the tests based on feedback and altered them to match changes.
Currently working on getting elemental registration through combustion working in eib. So far, I've been able to get it work with some few hacks (which are omitted from here, along with the Elemental specific stuff as it is not complete). To get there though, I needed to be able to add the RPMs to the image, so I built the handling for that as well as the tests. I’m not sure if this is how you guys would have gone about it, so I’ve made this PR a draft for now