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

fix(pkg/driverbuilder): properly export KERNELDIR in kernel-download scripts #330

Merged
merged 2 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/alinux_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ mkdir -p /tmp/kernel
mv usr/src/kernels/*/* /tmp/kernel

# exit value
echo /tmp/kernel
export KERNELDIR=/tmp/kernel
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/almalinux_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ mkdir -p /tmp/kernel
mv usr/src/kernels/*/* /tmp/kernel

# exit value
echo /tmp/kernel
export KERNELDIR=/tmp/kernel
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/amazonlinux_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,4 @@ mkdir -p /tmp/kernel
mv usr/src/kernels/*/* /tmp/kernel

# exit value
echo /tmp/kernel
export KERNELDIR=/tmp/kernel
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/archlinux_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ mkdir -p /tmp/kernel
mv usr/lib/modules/*/build/* /tmp/kernel

# exit value
echo /tmp/kernel
export KERNELDIR=/tmp/kernel
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/centos_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ mkdir -p /tmp/kernel
mv usr/src/kernels/*/* /tmp/kernel

# exit value
echo /tmp/kernel
export KERNELDIR=/tmp/kernel
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/debian_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,4 @@ sourcedir=$(find . -type d -name "{{ .KernelHeadersPattern }}" | head -n 1 | xar
sed -i 's/\/usr\/src/../g' $sourcedir/Makefile

# exit value
echo $sourcedir
export KERNELDIR=$sourcedir
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/fedora_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ mkdir -p /tmp/kernel
mv usr/src/kernels/*/* /tmp/kernel

# exit value
echo /tmp/kernel
export KERNELDIR=/tmp/kernel
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/flatcar_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@ make KCONFIG_CONFIG=/tmp/kernel.config oldconfig
make KCONFIG_CONFIG=/tmp/kernel.config modules_prepare

# exit value
echo /tmp/kernel
export KERNELDIR=/tmp/kernel
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/opensuse_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ cd /tmp/kernel-download/usr/src
sourcedir="$(find . -type d -name "linux-*-obj" | head -n 1 | xargs readlink -f)/*/default"

# exit value
echo $sourcedir
export KERNELDIR=$sourcedir
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/oracle_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ mkdir -p /tmp/kernel
mv usr/src/kernels/*/* /tmp/kernel

# exit value
echo /tmp/kernel
export KERNELDIR=/tmp/kernel
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/photonos_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,4 @@ mkdir -p /tmp/kernel
mv usr/src/linux-*headers-*/* /tmp/kernel

# exit value
echo /tmp/kernel
export KERNELDIR=/tmp/kernel
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/redhat_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ mkdir -p /tmp/kernel
mv usr/src/kernels/*/* /tmp/kernel

# exit value
echo /tmp/kernel
export KERNELDIR=/tmp/kernel
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/rocky_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ mkdir -p /tmp/kernel
mv usr/src/kernels/*/* /tmp/kernel

# exit value
echo /tmp/kernel
export KERNELDIR=/tmp/kernel
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/sles_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,4 @@ done
sourcedir="$(find . -type d -name "linux-*-obj" | head -n 1 | xargs readlink -f)/*/default"

# exit value
echo $sourcedir
export KERNELDIR=$sourcedir
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/ubuntu_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ cd /tmp/kernel-download/usr/src/
sourcedir=$(find . -type d -name "{{ .KernelHeadersPattern }}" | head -n 1 | xargs readlink -f)

# exit value
echo $sourcedir
export KERNELDIR=$sourcedir
2 changes: 1 addition & 1 deletion pkg/driverbuilder/builder/templates/vanilla_kernel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,4 @@ make KCONFIG_CONFIG=/tmp/kernel.config prepare
make KCONFIG_CONFIG=/tmp/kernel.config modules_prepare

# exit value
echo /tmp/kernel
export KERNELDIR=/tmp/kernel
10 changes: 9 additions & 1 deletion pkg/driverbuilder/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,13 @@ func (bp *DockerBuildProcessor) Start(b *builder.Build) error {
return err
}

// We make all 3 scripts executables,
// then:
// * download libs at required version
// * download and extract headers
// * each download-headers script will export KERNELDIR variable internally
// * we source download-headers.sh so that KERNELDIR is then visible to driverkit.sh
// * we finally make the actual build of the drivers
runCmd :=
`
#!/bin/bash
Expand All @@ -242,7 +249,8 @@ chmod +x /driverkit/download-headers.sh
chmod +x /driverkit/driverkit.sh

/driverkit/download-libs.sh
KERNELDIR=$(/driverkit/download-headers.sh) /driverkit/driverkit.sh
. /driverkit/download-headers.sh
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Source the download-headers script so that KERNELDIR gets set.

/driverkit/driverkit.sh
`

files := []dockerCopyFile{
Expand Down
4 changes: 2 additions & 2 deletions pkg/driverbuilder/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ func (bp *KubernetesBuildProcessor) buildModule(b *builder.Build) error {
}

// We run a script that downloads libs,
// download and extracts kernelURLs saving its output to KERNELDIR env variable,
// then downloads and extracts kernelURLs exporting KERNELDIR env variable,
// then finally runs the build script.
res = fmt.Sprintf("%s\nexport KERNELDIR=$(%s)\n%s", libsDownloadScript, kernelDownloadScript, res)
res = fmt.Sprintf("%s\n%s\n%s", libsDownloadScript, kernelDownloadScript, res)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since everything is put in a single bash script here, we don't even need to source kernel-download script.


if c.ModuleFilePath != "" {
res = fmt.Sprintf("%s\n%s", "touch "+moduleLockFile, res)
Expand Down
27 changes: 19 additions & 8 deletions pkg/driverbuilder/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package driverbuilder

import (
"bufio"
"bytes"
"context"
_ "embed"
"errors"
Expand All @@ -13,7 +14,6 @@ import (
"os/exec"
"os/user"
"path/filepath"
"strings"
"time"
)

Expand Down Expand Up @@ -67,24 +67,35 @@ func (lbp *LocalBuildProcessor) Start(b *builder.Build) error {
// if an unsupported target is passed.
// Go on skipping automatic kernel headers download.
if err == nil {
slog.Info("Trying automatic kernel headers download.")
slog.Info("Trying automatic kernel headers download")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropped some . at the end of log strings for local executor, for consistency.

kernelDownloadScript, err := builder.KernelDownloadScript(realBuilder, nil, kr)
// Patch kernel download script to echo KERNELDIR.
// We need to capture KERNELDIR to later pass it as env variable to the build.
kernelDownloadScript += "\necho $KERNELDIR"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local executor needs to fetch KERNELDIR after exec.Command runs.
The only way is to fallback at the old method of echoing KERNELDIR at the end of the script; then we will parse script stdout line by line and the last one will be our KERNELDIR path.

if err == nil {
out, err := exec.Command("bash", "-c", kernelDownloadScript).Output()
if err == nil {
path := strings.TrimSuffix(string(out), "\n")
// Scan all stdout line by line and
// store last line as KERNELDIR path.
reader := bytes.NewReader(out)
scanner := bufio.NewScanner(reader)
var path string
for scanner.Scan() {
path = scanner.Text()
}
slog.Info("Setting KERNELDIR env var", "path", path)
// add the kerneldir path to env
lbp.envMap[kernelDirEnv] = path
defer func() {
_ = os.RemoveAll("/tmp/kernel-download")
_ = os.RemoveAll(path)
}()
} else {
slog.Warn("Failed to download headers.", "err", err)
slog.Warn("Failed to download headers", "err", err)
}
}
} else {
slog.Info("Skipping kernel headers automatic download.", "err", err)
slog.Info("Skipping kernel headers automatic download", "err", err)
}
}

Expand Down Expand Up @@ -137,7 +148,7 @@ func (lbp *LocalBuildProcessor) Start(b *builder.Build) error {
srcProbePath := vv.GetProbeFullPath(c)

if len(lbp.srcDir) == 0 {
slog.Info("Downloading driver sources.")
slog.Info("Downloading driver sources")
// Download src!
libsDownloadScript, err := builder.LibsDownloadScript(c)
if err != nil {
Expand Down Expand Up @@ -168,14 +179,14 @@ func (lbp *LocalBuildProcessor) Start(b *builder.Build) error {

stdout, err := cmd.StdoutPipe()
if err != nil {
slog.Warn("Failed to pipe stdout. Trying without piping.", "err", err)
slog.Warn("Failed to pipe stdout", "err", err)
_, err = cmd.CombinedOutput()
} else {
cmd.Stderr = cmd.Stdout // redirect stderr to stdout so that we catch it
defer stdout.Close()
err = cmd.Start()
if err != nil {
slog.Warn("Failed to execute command.", "err", err)
slog.Warn("Failed to execute command", "err", err)
} else {
// print the output of the subprocess line by line
scanner := bufio.NewScanner(stdout)
Expand Down
Loading