-
Notifications
You must be signed in to change notification settings - Fork 139
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
actions: debootstrap: Add parent-suite property to indicate which suite a downstream is based on #424
base: main
Are you sure you want to change the base?
actions: debootstrap: Add parent-suite property to indicate which suite a downstream is based on #424
Changes from all commits
787a56f
cb59e3c
351da8c
0c58396
5a838e5
7b6ce52
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 |
---|---|---|
|
@@ -125,9 +125,12 @@ jobs: | |
test: | ||
- { name: "recipes", case: "recipes" } | ||
- { name: "templating", case: "templating", variables: " -t escaped:\\$ba\\'d\\$gers\\ snakes" } | ||
- { name: "debian (amd64)", case: "debian", variables: "-t architecture:amd64" } | ||
- { name: "debian (arm64)", case: "debian", variables: "-t architecture:arm64" } | ||
- { name: "debian (armhf)", case: "debian", variables: "-t architecture:armhf" } | ||
- { name: "debian (bookworm amd64)", case: "debian", variables: "-t architecture:amd64 -t suite:bookworm" } | ||
- { name: "debian (bookworm arm64)", case: "debian", variables: "-t architecture:arm64 -t suite:bookworm" } | ||
- { name: "debian (bookworm armhf)", case: "debian", variables: "-t architecture:armhf -t suite:bookworm" } | ||
- { name: "debian (trixie amd64)", case: "debian", variables: "-t architecture:amd64 -t suite:trixie" } | ||
- { name: "debian (trixie arm64)", case: "debian", variables: "-t architecture:arm64 -t suite:trixie" } | ||
- { name: "debian (trixie armhf)", case: "debian", variables: "-t architecture:armhf -t suite:trixie" } | ||
exclude: | ||
- backend: nofakemachine | ||
test: { name: "partitioning", case: "partitioning" } | ||
|
@@ -136,6 +139,12 @@ jobs: | |
test: { name: "arch", case: "arch" } | ||
- backend: kvm | ||
test: { name: "apertis", case: "apertis" } | ||
- backend: kvm | ||
test: { name: "kali", case: "kali" } | ||
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. Does kali not build without this? Fwiw i'm wary about adding rolling releases as they might break CI too easily 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. Hello Sjoerd,
Nope, it doesn't, since it's based on Debian testing. I tried with latest debos from the Docker Hub, just to be sure:
I suppose all derivatives based on Debian testing/sid are affected. Note that we can (and we do) workaround by adding this just after the debootstrap step:
So it's not a dealbreaker, we can live without the MR. But it's nicer when we don't have to use cryptic workarounds.
Debootstrapping Kali rolling is just like debootstrapping Debian testing, only two packages in the set are forked, the rest is just Debian testing. Usually You might want to use the mirror Thanks 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. I'd like to keep the Kali test in - with the mirror change @elboulangero mentions above. @sjoerdsimons are you happy with that ? |
||
- backend: kvm | ||
test: { name: "apertis v2022", case: "apertis", variables: "-t suite:v2022" } | ||
obbardc marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- backend: kvm | ||
test: { name: "apertis v2024dev3", case: "apertis", variables: "-t suite:v2024dev3 -t parent_suite:bookworm" } | ||
obbardc marked this conversation as resolved.
Show resolved
Hide resolved
|
||
name: ${{matrix.test.name}} on ${{matrix.backend}} | ||
runs-on: ${{ matrix.backend == 'kvm' && 'kvm' || 'ubuntu-latest' }} | ||
steps: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,10 +46,19 @@ Example: | |
- certificate -- client certificate stored in file to be used for downloading packages from the server. | ||
|
||
- private-key -- provide the client's private key in a file separate from the certificate. | ||
|
||
- parent-suite -- release code name which this suite is based on. Useful for downstreams which do | ||
not use debian codenames for their suite names (e.g. "stable"). | ||
Comment on lines
+50
to
+51
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. Fwiw this could be used for non-debian parents as well.. E.g. |
||
|
||
- script -- the full path of the script to use to build the target rootfs. (e.g. `/usr/share/debootstrap/scripts/kali`) | ||
If unspecified, the property will be automatically determined in the following order, | ||
with the path "/usr/share/debootstrap/scripts/" prepended: | ||
`suite` property, `parent-suite` property then `unstable`. | ||
*/ | ||
package actions | ||
|
||
import ( | ||
"errors" | ||
"fmt" | ||
"io" | ||
"log" | ||
|
@@ -64,6 +73,7 @@ import ( | |
|
||
type DebootstrapAction struct { | ||
debos.BaseAction `yaml:",inline"` | ||
ParentSuite string `yaml:"parent-suite"` | ||
Suite string | ||
Mirror string | ||
Variant string | ||
|
@@ -74,6 +84,7 @@ type DebootstrapAction struct { | |
Components []string | ||
MergedUsr bool `yaml:"merged-usr"` | ||
CheckGpg bool `yaml:"check-gpg"` | ||
Script string | ||
} | ||
|
||
func NewDebootstrapAction() *DebootstrapAction { | ||
|
@@ -115,6 +126,10 @@ func (d *DebootstrapAction) Verify(context *debos.DebosContext) error { | |
return fmt.Errorf("suite property not specified") | ||
} | ||
|
||
if len(d.ParentSuite) == 0 { | ||
d.ParentSuite = d.Suite | ||
} | ||
|
||
files := d.listOptionFiles(context) | ||
|
||
// Check if all needed files exists | ||
|
@@ -163,9 +178,9 @@ func (d *DebootstrapAction) RunSecondStage(context debos.DebosContext) error { | |
return err | ||
} | ||
|
||
// Guess if suite is something before usr-is-merged was introduced | ||
func (d *DebootstrapAction) isLikelyOldSuite() bool { | ||
switch strings.ToLower(d.Suite) { | ||
// Check if suite is something before usr-is-merged was introduced | ||
func shouldExcludeUsrIsMerged(suite string) bool { | ||
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. We should flip this function around so the default is fwiw debootstrap has the following list: no need to deal with |
||
switch strings.ToLower(suite) { | ||
case "sid", "unstable": | ||
return false | ||
case "testing": | ||
|
@@ -181,6 +196,10 @@ func (d *DebootstrapAction) isLikelyOldSuite() bool { | |
} | ||
} | ||
|
||
func getDebootstrapScriptPath(script string) string { | ||
return path.Join("/usr/share/debootstrap/scripts/", script) | ||
} | ||
|
||
func (d *DebootstrapAction) Run(context *debos.DebosContext) error { | ||
cmdline := []string{"debootstrap"} | ||
|
||
|
@@ -226,16 +245,41 @@ func (d *DebootstrapAction) Run(context *debos.DebosContext) error { | |
cmdline = append(cmdline, fmt.Sprintf("--variant=%s", d.Variant)) | ||
} | ||
|
||
// workaround for https://github.com/go-debos/debos/issues/361 | ||
if d.isLikelyOldSuite() { | ||
log.Println("excluding usr-is-merged as package is not in suite") | ||
if shouldExcludeUsrIsMerged(d.ParentSuite) { | ||
log.Printf("excluding usr-is-merged as package is not in parent suite %s\n", d.ParentSuite) | ||
cmdline = append(cmdline, "--exclude=usr-is-merged") | ||
} | ||
|
||
cmdline = append(cmdline, d.Suite) | ||
cmdline = append(cmdline, context.Rootdir) | ||
cmdline = append(cmdline, d.Mirror) | ||
cmdline = append(cmdline, "/usr/share/debootstrap/scripts/unstable") | ||
|
||
if len(d.Script) > 0 { | ||
if _, err := os.Stat(d.Script); err != nil { | ||
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. this should be done in verify not at run time |
||
return fmt.Errorf("cannot find debootstrap script %s", d.Script) | ||
} | ||
} else { | ||
/* Auto determine debootstrap script to use from d.Suite, falling back to | ||
d.ParentSuite if it doesn't exist. Finally, fallback to unstable if a | ||
script for the parent suite does not exist. */ | ||
for _, s := range []string{d.Suite, d.ParentSuite, "unstable"} { | ||
d.Script = getDebootstrapScriptPath(s) | ||
if _, err := os.Stat(d.Script); err == nil { | ||
break | ||
} else { | ||
log.Printf("cannot find debootstrap script %s\n", d.Script) | ||
|
||
/* Unstable should always be available so error out if not */ | ||
if s == "unstable" { | ||
return errors.New("cannot find debootstrap script for unstable") | ||
} | ||
} | ||
} | ||
|
||
log.Printf("using debootstrap script %s\n", d.Script) | ||
} | ||
|
||
cmdline = append(cmdline, d.Script) | ||
|
||
/* Make sure /etc/apt/apt.conf.d exists inside the fakemachine otherwise | ||
debootstrap prints a warning about the path not existing. */ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
--- | ||
# Test building a non-debian distribution based on unstable such as kali-rolling | ||
# to ensure bootstrapping suites that debootstrap won't internally know about | ||
# works | ||
{{- $architecture := or .architecture "amd64" }} | ||
|
||
architecture: {{ $architecture }} | ||
|
||
actions: | ||
- action: debootstrap | ||
suite: kali-rolling | ||
parent-suite: testing | ||
components: | ||
- main | ||
mirror: https://http.kali.org/kali/ | ||
variant: minbase | ||
keyring-package: kali-archive-keyring | ||
keyring-file: kali-archive-keyring.gpg | ||
|
||
- action: apt | ||
description: Install some base packages | ||
packages: | ||
- procps |
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.
this massively extends the number of test jobs; Does testing trixie on arm64/armhf give us extra data? Also i'm a tad wary about it breaking CI non-deterministically
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.
as was done to tune our runs; for trixie for now please just build amd64 and only on kvm
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.
yeah probably best to remove trixie