-
Notifications
You must be signed in to change notification settings - Fork 28
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
added handling for adding rpms to an image as well as the relevant tests #28
Changes from 1 commit
8a56a02
41fb255
bd3705c
34be403
ba9c4dc
fa5e60b
5d8e2ca
688302c
2f489d9
53c7d68
a68fab2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,9 @@ type Builder struct { | |
eibBuildDir string | ||
combustionDir string | ||
combustionScripts []string | ||
rpmBuildDir string | ||
rpmFileNames []string | ||
rpmSourceDir string | ||
} | ||
|
||
func New(imageConfig *config.ImageConfig, buildConfig *config.BuildConfig) *Builder { | ||
|
@@ -48,6 +51,11 @@ func (b *Builder) Build() error { | |
return fmt.Errorf("generating combustion script: %w", err) | ||
} | ||
|
||
err = b.copyRPMs() | ||
if err != nil { | ||
return fmt.Errorf("copying RPMs over: %w", err) | ||
} | ||
|
||
switch b.imageConfig.Image.ImageType { | ||
case config.ImageTypeISO: | ||
err = b.buildIsoImage() | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
||
err := os.MkdirAll(b.combustionDir, os.ModePerm) | ||
if err != nil { | ||
return fmt.Errorf("creating the build directory structure: %w", err) | ||
return fmt.Errorf("creating the build directory structure for combustion: %w", err) | ||
atanasdinov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
err = os.MkdirAll(b.rpmBuildDir, os.ModePerm) | ||
if err != nil { | ||
return fmt.Errorf("creating the build directory structure for rpms: %w", err) | ||
} | ||
|
||
return nil | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
package build | ||
|
||
import ( | ||
"fmt" | ||
"io" | ||
"os" | ||
"path/filepath" | ||
) | ||
|
||
func (b *Builder) getRPMFileNames() error { | ||
atanasdinov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
b.rpmSourceDir = filepath.Join(b.buildConfig.ImageConfigDir, "rpms") | ||
atanasdinov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
rpms, err := os.ReadDir(b.rpmSourceDir) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
if err != nil { | ||
return fmt.Errorf("reading rpm source dir: %w", err) | ||
} | ||
|
||
for _, rpmFile := range rpms { | ||
b.rpmFileNames = append(b.rpmFileNames, rpmFile.Name()) | ||
} | ||
return nil | ||
} | ||
|
||
func (b *Builder) copyRPMs() error { | ||
err := b.getRPMFileNames() | ||
if err != nil { | ||
return fmt.Errorf("getting rpm file names: %w", err) | ||
} | ||
|
||
for _, rpm := range b.rpmFileNames { | ||
sourcePath := filepath.Join(b.rpmSourceDir, rpm) | ||
destPath := filepath.Join(b.rpmBuildDir, rpm) | ||
|
||
sourceFile, err := os.Open(sourcePath) | ||
if err != nil { | ||
return fmt.Errorf("opening rpm source path: %w", err) | ||
} | ||
defer sourceFile.Close() | ||
|
||
destFile, err := os.Create(destPath) | ||
if err != nil { | ||
return fmt.Errorf("opening rpm dest path: %w", err) | ||
} | ||
defer destFile.Close() | ||
|
||
_, err = io.Copy(destFile, sourceFile) | ||
if err != nil { | ||
return fmt.Errorf("copying rpm: %w", err) | ||
} | ||
} | ||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
package build | ||
|
||
import ( | ||
"os" | ||
"path/filepath" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
"github.com/stretchr/testify/require" | ||
"github.com/suse-edge/edge-image-builder/pkg/config" | ||
) | ||
|
||
func TestGetRPMFileNames(t *testing.T) { | ||
// Setup | ||
bc := config.BuildConfig{ | ||
ImageConfigDir: "../config/testdata", | ||
atanasdinov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
builder := New(nil, &bc) | ||
err := builder.prepareBuildDir() | ||
require.NoError(t, err) | ||
defer os.Remove(builder.eibBuildDir) | ||
|
||
// Test | ||
err = builder.getRPMFileNames() | ||
|
||
// Verify | ||
require.NoError(t, err) | ||
|
||
assert.Contains(t, builder.rpmFileNames, "rpm1.rpm") | ||
assert.Contains(t, builder.rpmFileNames, "rpm2.rpm") | ||
} | ||
|
||
func TestCopyRPMs(t *testing.T) { | ||
// Setup | ||
bc := config.BuildConfig{ | ||
ImageConfigDir: "../config/testdata", | ||
} | ||
builder := New(nil, &bc) | ||
err := builder.prepareBuildDir() | ||
require.NoError(t, err) | ||
defer os.Remove(builder.eibBuildDir) | ||
|
||
// Test | ||
err = builder.copyRPMs() | ||
|
||
// Verify | ||
require.NoError(t, err) | ||
|
||
_, err = os.Stat(filepath.Join(builder.rpmBuildDir, "rpm1.rpm")) | ||
require.NoError(t, err) | ||
|
||
_, err = os.Stat(filepath.Join(builder.rpmBuildDir, "rpm2.rpm")) | ||
require.NoError(t, err) | ||
} |
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.
I would use the same directory than combustion dir
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