Skip to content

Commit e668637

Browse files
committed
fix(pr): address review feedback
Signed-off-by: Ayush <ayushsrisks@gmail.com>
1 parent 3d69997 commit e668637

File tree

8 files changed

+23
-72
lines changed

8 files changed

+23
-72
lines changed

docs/hypervisor-support.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,14 +157,15 @@ Cloud Hypervisor can be installed by downloading a pre-built binary from the
157157
ARCH="$(uname -m)"
158158
VERSION="v43.0"
159159
release_url="https://github.com/cloud-hypervisor/cloud-hypervisor/releases"
160-
curl -L ${release_url}/download/${VERSION}/cloud-hypervisor-static-${ARCH} -o cloud-hypervisor
160+
if [ "$ARCH" = "x86_64" ]; then
161+
curl -L ${release_url}/download/${VERSION}/cloud-hypervisor-static -o cloud-hypervisor
162+
else
163+
curl -L ${release_url}/download/${VERSION}/cloud-hypervisor-static-${ARCH} -o cloud-hypervisor
164+
fi
161165
chmod +x cloud-hypervisor
162166
sudo mv cloud-hypervisor /usr/local/bin/
163167
```
164168

165-
It is important to note that `urunc` expects to find the `cloud-hypervisor`
166-
binary located in the `$PATH` and named `cloud-hypervisor`.
167-
168169
#### Cloud Hypervisor and `urunc`
169170

170171
In the case of [Cloud Hypervisor](https://www.cloudhypervisor.org/), `urunc`
@@ -176,13 +177,12 @@ filesystems between the host and guest.
176177

177178
Supported unikernel frameworks with `urunc`:
178179

179-
- [Unikraft](../unikernel-support#unikraft)
180180
- [Linux](../unikernel-support#linux)
181181

182182
An example unikernel:
183183

184184
```bash
185-
sudo nerdctl run --rm -ti --runtime io.containerd.urunc.v2 harbor.nbfc.io/nubificus/urunc/nginx-cloud-hypervisor-unikraft-initrd:latest
185+
sudo nerdctl run --rm -ti --runtime io.containerd.urunc.v2 harbor.nbfc.io/nubificus/urunc/nginx-cloud-hypervisor-linux-raw:latest
186186
```
187187

188188
### Solo5-hvt

pkg/unikontainers/hypervisors/cloud_hypervisor.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func (ch *CloudHypervisor) UsesKVM() bool {
4848
// SupportsSharedfs returns true as Cloud Hypervisor supports virtiofs
4949
func (ch *CloudHypervisor) SupportsSharedfs(fsType string) bool {
5050
switch fsType {
51-
case "virtio", "virtiofs":
51+
case "virtio":
5252
return true
5353
default:
5454
return false
@@ -66,7 +66,11 @@ func (ch *CloudHypervisor) Execve(args types.ExecArgs, ukernel types.Unikernel)
6666
exArgs := []string{ch.binaryPath}
6767

6868
// Memory configuration
69-
exArgs = append(exArgs, "--memory", fmt.Sprintf("size=%sM,shared=on", chMem))
69+
if args.Sharedfs.Type == "virtio" {
70+
exArgs = append(exArgs, "--memory", fmt.Sprintf("size=%sM,shared=on", chMem))
71+
} else {
72+
exArgs = append(exArgs, "--memory", fmt.Sprintf("size=%sM", chMem))
73+
}
7074

7175
// CPU configuration
7276
if args.VCPUs > 0 {
@@ -118,12 +122,12 @@ func (ch *CloudHypervisor) Execve(args types.ExecArgs, ukernel types.Unikernel)
118122

119123
// Check for extra initrd from unikernel monitor args
120124
extraMonArgs := ukernel.MonitorCli()
121-
if extraMonArgs.ExtraInitrd != "" && args.InitrdPath == "" {
125+
if extraMonArgs.ExtraInitrd != "" {
122126
exArgs = append(exArgs, "--initramfs", extraMonArgs.ExtraInitrd)
123127
}
124128

125129
switch args.Sharedfs.Type {
126-
case "virtiofs":
130+
case "virtio":
127131
exArgs = append(exArgs, "--fs", "tag=fs0,socket=/tmp/vhostqemu")
128132
default:
129133
// No shared filesystem

pkg/unikontainers/ipc.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ func AwaitMessage(listener *net.UnixListener, expectedMessage IPCMessage) error
165165
}
166166
msg := string(buf[0:n])
167167
if msg != string(expectedMessage) {
168-
return fmt.Errorf("received unexpected message: %s (expected %s)", msg, expectedMessage)
168+
return fmt.Errorf("received unexpected message: %s", msg)
169169
}
170170
return nil
171171
}

pkg/unikontainers/mount.go

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func createTmpfs(monRootfs string, path string, flags uint64, mode string, size
6464

6565
if mode == "1777" {
6666
// sonarcloud:go:S2612 -- This is a tmpfs mount point, sticky bit 1777 is required (like /tmp), controlled path, safe by design
67-
err := os.Chmod(dstPath, 01777) // NOSONAR
67+
err := os.Chmod(path, 01777) // NOSONAR
6868
if err != nil {
6969
return fmt.Errorf("failed to chmod %s: %w", path, err)
7070
}
@@ -116,23 +116,6 @@ func setupDev(monRootfs string, devPath string) error {
116116
// Create the new device node
117117
err = unix.Mknod(dstPath, devStat.Mode, int(newDev)) //nolint: gosec
118118
if err != nil {
119-
// If mknod fails because of permissions (e.g. in a container), try to bind mount it
120-
if errors.Is(err, unix.EPERM) || errors.Is(err, unix.EACCES) {
121-
uniklog.Warnf("failed to make device node %s: %v. Fallback to bind mount.", dstPath, err)
122-
// Create an empty file to be used as mount point
123-
f, err1 := os.Create(dstPath)
124-
if err1 != nil && !os.IsExist(err1) {
125-
return fmt.Errorf("failed to create mount point for device %s: %w", dstPath, err1)
126-
}
127-
if f != nil {
128-
f.Close()
129-
}
130-
err = unix.Mount(devPath, dstPath, "", unix.MS_BIND, "")
131-
if err != nil {
132-
return fmt.Errorf("failed to bind mount device %s: %w", devPath, err)
133-
}
134-
return nil
135-
}
136119
return fmt.Errorf("failed to make device node %s: %w", dstPath, err)
137120
}
138121

@@ -196,13 +179,7 @@ func fileFromHost(monRootfs string, hostPath string, target string, mFlags int,
196179
}
197180
}
198181
} else {
199-
if withCopy {
200-
return ErrCopyDir
201-
}
202-
err = bindMountFile(hostPath, dstPath, "", 0, mFlags, true)
203-
if err != nil {
204-
return fmt.Errorf("failed to bind mount file %s: %w", hostPath, err)
205-
}
182+
return ErrCopyDir
206183
}
207184

208185
if withCopy {

tests/e2e/common.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ func commonCmdExec(command string) (output string, err error) {
7878
var stderrBuf bytes.Buffer
7979

8080
params := strings.Fields(command)
81-
fmt.Printf("Executing command: %s\n", command)
8281
cmd := exec.Command(params[0], params[1:]...) //nolint:gosec
8382
cmd.Stderr = &stderrBuf
8483
outBytes, err := cmd.Output()
@@ -225,13 +224,9 @@ func findValOfKey(searchArea string, key string) (string, error) {
225224
return "", err
226225
}
227226
match := r.FindString(searchArea)
228-
if match == "" {
229-
return "", fmt.Errorf("key %s not found in search area", key)
230-
}
227+
231228
keyValMatch := strings.Split(match, ":")
232-
if len(keyValMatch) < 2 {
233-
return "", fmt.Errorf("invalid format for key %s: %s", key, match)
234-
}
229+
235230
val := strings.ReplaceAll(keyValMatch[1], "\"", "")
236231
return strings.TrimSpace(val), nil
237232
}

tests/e2e/nerdctl.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,7 @@ func (i *nerdctlInfo) inspectCAndGet(key string) (string, error) {
125125
return commonInspectCAndGet(nerdctlName, i.containerID, key)
126126
}
127127

128-
func (i *nerdctlInfo) inspectPAndGet(key string) (string, error) {
129-
if key == "pid" || key == "Pid" {
130-
return i.inspectCAndGet("Pid")
131-
}
128+
func (i *nerdctlInfo) inspectPAndGet(string) (string, error) {
132129
// Not supported by nerdctl
133130
return "", errToolDoesNotSupport
134131
}

tests/e2e/test_functions.go

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -169,28 +169,7 @@ func seccompTest(tool testTool) error {
169169
if len(wordsInLine) != 2 {
170170
return fmt.Errorf("Invalid format of line. Expecting 2 values, got %d", len(wordsInLine))
171171
}
172-
seccompStatus := strings.TrimSpace(wordsInLine[1])
173-
174-
// If main thread doesn't have seccomp, check other threads
175-
if seccompStatus == "0" && args.Seccomp {
176-
taskPath := "/proc/" + unikernelPID + "/task"
177-
tasks, err := os.ReadDir(taskPath)
178-
if err == nil {
179-
for _, task := range tasks {
180-
threadStatusPath := filepath.Join(taskPath, task.Name(), "status")
181-
line, err := findLineInFile(threadStatusPath, "Seccomp")
182-
if err == nil {
183-
words := strings.Split(line, ":")
184-
if len(words) == 2 && strings.TrimSpace(words[1]) == "2" {
185-
seccompStatus = "2"
186-
break
187-
}
188-
}
189-
}
190-
}
191-
}
192-
193-
if seccompStatus == "2" {
172+
if strings.TrimSpace(wordsInLine[1]) == "2" {
194173
if !args.Seccomp {
195174
return fmt.Errorf("Seccomp should not be enabled")
196175
}

tests/e2e/utils.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,8 @@ func pingUnikernel(ipAddress string) error {
3232
if err != nil {
3333
return fmt.Errorf("failed to create Pinger: %v", err)
3434
}
35-
pinger.SetPrivileged(true) // Enable privileged mode to use ICMP
36-
pinger.Count = 10
37-
pinger.Timeout = 15 * time.Second
35+
pinger.Count = 3
36+
pinger.Timeout = 5 * time.Second
3837
err = pinger.Run()
3938
if err != nil {
4039
return fmt.Errorf("failed to ping %s: %v", ipAddress, err)

0 commit comments

Comments
 (0)