fix(unikernels): skip network args in mewz commandstring when no network#674
fix(unikernels): skip network args in mewz commandstring when no network#674mdryaan wants to merge 2 commits into
Conversation
✅ Deploy Preview for urunc canceled.
|
| func TestMewzCommandStringNoNetwork(t *testing.T) { | ||
| t.Parallel() | ||
| m := &Mewz{} | ||
| result, err := m.CommandString() | ||
| require.NoError(t, err) | ||
| assert.Equal(t, "", result, "CommandString should return empty string when no network is configured") | ||
| } | ||
|
|
||
| func TestMewzCommandStringWithNetwork(t *testing.T) { | ||
| t.Parallel() | ||
| m := &Mewz{ | ||
| Net: MewzNet{ | ||
| Address: "10.0.0.2", | ||
| Mask: 24, | ||
| Gateway: "10.0.0.1", | ||
| }, | ||
| } | ||
| result, err := m.CommandString() | ||
| require.NoError(t, err) | ||
| assert.Equal(t, "ip=10.0.0.2/24 gateway=10.0.0.1", result) | ||
| } |
There was a problem hiding this comment.
Please mere these tests. They are unit tests for the same function.
| var parts []string | ||
| if m.Net.Address != "" { | ||
| parts = append(parts, fmt.Sprintf("ip=%s/%d", m.Net.Address, m.Net.Mask)) | ||
| parts = append(parts, fmt.Sprintf("gateway=%s", m.Net.Gateway)) | ||
| } | ||
| return strings.Join(parts, " "), nil |
There was a problem hiding this comment.
No reason to create a slice. It is a simple return of a specific string. So we can just wrap the previous Sprintf in an if statement.
There was a problem hiding this comment.
Merged into a single table-driven test with two subtests.
|
Thanks @cmainas for the review and feedback! PTAL |
|
Hello @mdryaan , the PR looks good, but we need to coordinate adding yourself in https://github.com/urunc-dev/urunc/blob/main/.github/contributors.yaml This should be done in one of your open PRs and we should merge this first. |
Hi @cmainas Already added myself in #676, you can merge that first and then this PR |
|
Hello @mdryaan , unfortunately another rebase is necessary. |
Mewz.CommandString() unconditionally formatted ip=%s/%d and gateway=%s even when no network was configured, producing "ip=/0 gateway= " as kernel boot parameters. Mewz does not handle this case and panics. Guard the network args behind an m.Net.Address != "" check, matching the existing pattern in Hermit.CommandString(). Add unit tests covering both the no-network and with-network cases. Signed-off-by: mdryaan <[email protected]> Signed-off-by: mdryaan <[email protected]>
Signed-off-by: mdryaan <[email protected]>
mdryaan
force-pushed
the
fix/mewz-commandstring-no-network
branch
from
d02e914 to
d26e7f4
Compare
|
@cmainas Done, rebased over main. |
|
Hello @mdryaan , this PR introduces a new unit test which can not be executed directly. You will need to add it here https://github.com/urunc-dev/urunc/blob/main/Makefile#L234 as the other unit tests for the unikontainers sub directories. |
2 participants
Description
Mewz.CommandString()unconditionally formattedip=%s/%d gateway=%seven when no network was configured, producing"ip=/0 gateway= "as kernel boot parameters. Mewz does not handle this and panics at startup.Guard the network args behind an
m.Net.Address != ""check, matching the pattern already used byHermit.CommandString(). Add unit tests covering both the no-network and with-network cases.Related issues
How was this tested?
Added two unit tests in
pkg/unikontainers/unikernels/mewz_test.go:TestMewzCommandStringNoNetwork: assertsCommandString()returns""when no network is configured.TestMewzCommandStringWithNetwork: assertsCommandString()returns"ip=10.0.0.2/24 gateway=10.0.0.1"when network is configured.Before fix (bug reproduced):
After fix:
LLM usage
N/A
Checklist
make lint).make test_ctr,make test_nerdctl,make test_docker,make test_crictl).