Skip to content

Commit b97d7a9

Browse files
unsumanunsuman
committed
driver(external): server logs to a file and fixed json marshal error in SetConfig()
Signed-off-by: Ansuman Sahoo <[email protected]>
1 parent 1451395 commit b97d7a9

File tree

5 files changed

+105
-100
lines changed

5 files changed

+105
-100
lines changed

‎pkg/driver/external/client/methods.go

Lines changed: 18 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
func (d *DriverClient) Validate() error {
2121
d.logger.Debug("Validating driver for the given config")
2222

23-
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
23+
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
2424
defer cancel()
2525

2626
_, err := d.DriverSvc.Validate(ctx, &emptypb.Empty{})
@@ -36,9 +36,7 @@ func (d *DriverClient) Validate() error {
3636
func (d *DriverClient) Initialize(ctx context.Context) error {
3737
d.logger.Debug("Initializing driver instance")
3838

39-
connCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
40-
defer cancel()
41-
_, err := d.DriverSvc.Initialize(connCtx, &emptypb.Empty{})
39+
_, err := d.DriverSvc.Initialize(ctx, &emptypb.Empty{})
4240
if err != nil {
4341
d.logger.Errorf("Initialization failed: %v", err)
4442
return err
@@ -64,9 +62,7 @@ func (d *DriverClient) CreateDisk(ctx context.Context) error {
6462
func (d *DriverClient) Start(ctx context.Context) (chan error, error) {
6563
d.logger.Debug("Starting driver instance")
6664

67-
connCtx, cancel := context.WithTimeout(ctx, time.Minute)
68-
defer cancel()
69-
stream, err := d.DriverSvc.Start(connCtx, &emptypb.Empty{})
65+
stream, err := d.DriverSvc.Start(ctx, &emptypb.Empty{})
7066
if err != nil {
7167
d.logger.Errorf("Failed to start driver instance: %v", err)
7268
return nil, err
@@ -97,7 +93,7 @@ func (d *DriverClient) Start(ctx context.Context) (chan error, error) {
9793
func (d *DriverClient) Stop(ctx context.Context) error {
9894
d.logger.Debug("Stopping driver instance")
9995

100-
connCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
96+
connCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
10197
defer cancel()
10298
_, err := d.DriverSvc.Stop(connCtx, &emptypb.Empty{})
10399
if err != nil {
@@ -112,7 +108,7 @@ func (d *DriverClient) Stop(ctx context.Context) error {
112108
func (d *DriverClient) RunGUI() error {
113109
d.logger.Debug("Running GUI for the driver instance")
114110

115-
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
111+
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
116112
defer cancel()
117113

118114
_, err := d.DriverSvc.RunGUI(ctx, &emptypb.Empty{})
@@ -128,10 +124,7 @@ func (d *DriverClient) RunGUI() error {
128124
func (d *DriverClient) ChangeDisplayPassword(ctx context.Context, password string) error {
129125
d.logger.Debug("Changing display password for the driver instance")
130126

131-
connCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
132-
defer cancel()
133-
134-
_, err := d.DriverSvc.ChangeDisplayPassword(connCtx, &pb.ChangeDisplayPasswordRequest{
127+
_, err := d.DriverSvc.ChangeDisplayPassword(ctx, &pb.ChangeDisplayPasswordRequest{
135128
Password: password,
136129
})
137130
if err != nil {
@@ -146,10 +139,7 @@ func (d *DriverClient) ChangeDisplayPassword(ctx context.Context, password strin
146139
func (d *DriverClient) GetDisplayConnection(ctx context.Context) (string, error) {
147140
d.logger.Debug("Getting display connection for the driver instance")
148141

149-
connCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
150-
defer cancel()
151-
152-
resp, err := d.DriverSvc.GetDisplayConnection(connCtx, &emptypb.Empty{})
142+
resp, err := d.DriverSvc.GetDisplayConnection(ctx, &emptypb.Empty{})
153143
if err != nil {
154144
d.logger.Errorf("Failed to get display connection: %v", err)
155145
return "", err
@@ -162,10 +152,7 @@ func (d *DriverClient) GetDisplayConnection(ctx context.Context) (string, error)
162152
func (d *DriverClient) CreateSnapshot(ctx context.Context, tag string) error {
163153
d.logger.Debugf("Creating snapshot with tag: %s", tag)
164154

165-
connCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
166-
defer cancel()
167-
168-
_, err := d.DriverSvc.CreateSnapshot(connCtx, &pb.CreateSnapshotRequest{
155+
_, err := d.DriverSvc.CreateSnapshot(ctx, &pb.CreateSnapshotRequest{
169156
Tag: tag,
170157
})
171158
if err != nil {
@@ -180,10 +167,7 @@ func (d *DriverClient) CreateSnapshot(ctx context.Context, tag string) error {
180167
func (d *DriverClient) ApplySnapshot(ctx context.Context, tag string) error {
181168
d.logger.Debugf("Applying snapshot with tag: %s", tag)
182169

183-
connCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
184-
defer cancel()
185-
186-
_, err := d.DriverSvc.ApplySnapshot(connCtx, &pb.ApplySnapshotRequest{
170+
_, err := d.DriverSvc.ApplySnapshot(ctx, &pb.ApplySnapshotRequest{
187171
Tag: tag,
188172
})
189173
if err != nil {
@@ -198,10 +182,7 @@ func (d *DriverClient) ApplySnapshot(ctx context.Context, tag string) error {
198182
func (d *DriverClient) DeleteSnapshot(ctx context.Context, tag string) error {
199183
d.logger.Debugf("Deleting snapshot with tag: %s", tag)
200184

201-
connCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
202-
defer cancel()
203-
204-
_, err := d.DriverSvc.DeleteSnapshot(connCtx, &pb.DeleteSnapshotRequest{
185+
_, err := d.DriverSvc.DeleteSnapshot(ctx, &pb.DeleteSnapshotRequest{
205186
Tag: tag,
206187
})
207188
if err != nil {
@@ -216,10 +197,7 @@ func (d *DriverClient) DeleteSnapshot(ctx context.Context, tag string) error {
216197
func (d *DriverClient) ListSnapshots(ctx context.Context) (string, error) {
217198
d.logger.Debug("Listing snapshots")
218199

219-
connCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
220-
defer cancel()
221-
222-
resp, err := d.DriverSvc.ListSnapshots(connCtx, &emptypb.Empty{})
200+
resp, err := d.DriverSvc.ListSnapshots(ctx, &emptypb.Empty{})
223201
if err != nil {
224202
d.logger.Errorf("Failed to list snapshots: %v", err)
225203
return "", err
@@ -232,9 +210,7 @@ func (d *DriverClient) ListSnapshots(ctx context.Context) (string, error) {
232210
func (d *DriverClient) Register(ctx context.Context) error {
233211
d.logger.Debug("Registering driver instance")
234212

235-
connCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
236-
defer cancel()
237-
_, err := d.DriverSvc.Register(connCtx, &emptypb.Empty{})
213+
_, err := d.DriverSvc.Register(ctx, &emptypb.Empty{})
238214
if err != nil {
239215
d.logger.Errorf("Failed to register driver instance: %v", err)
240216
return err
@@ -247,10 +223,7 @@ func (d *DriverClient) Register(ctx context.Context) error {
247223
func (d *DriverClient) Unregister(ctx context.Context) error {
248224
d.logger.Debug("Unregistering driver instance")
249225

250-
connCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
251-
defer cancel()
252-
253-
_, err := d.DriverSvc.Unregister(connCtx, &emptypb.Empty{})
226+
_, err := d.DriverSvc.Unregister(ctx, &emptypb.Empty{})
254227
if err != nil {
255228
d.logger.Errorf("Failed to unregister driver instance: %v", err)
256229
return err
@@ -263,7 +236,7 @@ func (d *DriverClient) Unregister(ctx context.Context) error {
263236
func (d *DriverClient) ForwardGuestAgent() bool {
264237
d.logger.Debug("Checking if guest agent needs to be forwarded")
265238

266-
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
239+
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
267240
defer cancel()
268241

269242
resp, err := d.DriverSvc.ForwardGuestAgent(ctx, &emptypb.Empty{})
@@ -280,10 +253,7 @@ func (d *DriverClient) ForwardGuestAgent() bool {
280253
func (d *DriverClient) GuestAgentConn(ctx context.Context) (net.Conn, error) {
281254
d.logger.Debug("Getting guest agent connection")
282255

283-
connCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
284-
defer cancel()
285-
286-
connStream, err := d.DriverSvc.GuestAgentConn(connCtx, &emptypb.Empty{})
256+
connStream, err := d.DriverSvc.GuestAgentConn(ctx, &emptypb.Empty{})
287257
if err != nil {
288258
d.logger.Errorf("Failed to get guest agent connection: %v", err)
289259
return nil, err
@@ -295,7 +265,7 @@ func (d *DriverClient) GuestAgentConn(ctx context.Context) (net.Conn, error) {
295265
func (d *DriverClient) GetInfo() driver.Info {
296266
d.logger.Debug("Getting driver info")
297267

298-
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
268+
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
299269
defer cancel()
300270

301271
resp, err := d.DriverSvc.GetInfo(ctx, &emptypb.Empty{})
@@ -317,13 +287,13 @@ func (d *DriverClient) GetInfo() driver.Info {
317287
func (d *DriverClient) SetConfig(inst *store.Instance, sshLocalPort int) {
318288
d.logger.Debugf("Setting config for instance %s with SSH local port %d", inst.Name, sshLocalPort)
319289

320-
instJson, err := json.Marshal(inst)
290+
instJson, err := inst.MarshalJSON()
321291
if err != nil {
322292
d.logger.Errorf("Failed to marshal instance config: %v", err)
323293
return
324294
}
325295

326-
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
296+
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
327297
defer cancel()
328298

329299
_, err = d.DriverSvc.SetConfig(ctx, &pb.SetConfigRequest{

‎pkg/driver/external/server/methods.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package server
66
import (
77
"context"
88
"encoding/json"
9+
"errors"
910
"io"
1011

1112
"google.golang.org/grpc/codes"
@@ -53,7 +54,7 @@ func (s *DriverServer) SetConfig(ctx context.Context, req *pb.SetConfigRequest)
5354
s.logger.Debugf("Received SetConfig request")
5455
var inst store.Instance
5556

56-
if err := json.Unmarshal(req.InstanceConfigJson, &inst); err != nil {
57+
if err := inst.UnmarshalJSON(req.InstanceConfigJson); err != nil {
5758
s.logger.Errorf("Failed to unmarshal InstanceConfigJson: %v", err)
5859
return &emptypb.Empty{}, err
5960
}
@@ -79,7 +80,7 @@ func (s *DriverServer) GuestAgentConn(empty *emptypb.Empty, stream pb.Driver_Gue
7980
for {
8081
n, err := conn.Read(buf)
8182
if err != nil {
82-
if err == io.EOF {
83+
if errors.Is(err, io.EOF) {
8384
return nil
8485
}
8586
return status.Errorf(codes.Internal, "error reading: %v", err)

‎pkg/registry/registry.go

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
package registry
55

66
import (
7-
"bufio"
87
"context"
98
"fmt"
109
"io"
@@ -17,6 +16,7 @@ import (
1716

1817
"github.com/lima-vm/lima/pkg/driver"
1918
"github.com/lima-vm/lima/pkg/driver/external/client"
19+
"github.com/lima-vm/lima/pkg/store/filenames"
2020
"github.com/lima-vm/lima/pkg/usrlocalsharelima"
2121
"github.com/sirupsen/logrus"
2222
)
@@ -64,27 +64,29 @@ func (e *ExternalDriver) Start() error {
6464
return fmt.Errorf("failed to create stdout pipe: %w", err)
6565
}
6666

67-
stderr, err := cmd.StderrPipe()
67+
sharedDir, err := usrlocalsharelima.Dir()
6868
if err != nil {
6969
cancel()
70-
return fmt.Errorf("failed to create stderr pipe: %w", err)
70+
return fmt.Errorf("failed to determine Lima share directory: %w", err)
71+
}
72+
logPath := filepath.Join(sharedDir, filenames.ExternalDriverStderrLog)
73+
logFile, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644)
74+
if err != nil {
75+
cancel()
76+
return fmt.Errorf("failed to open external driver log file: %w", err)
7177
}
7278

79+
// Redirect stderr to the log file
80+
cmd.Stderr = logFile
81+
7382
if err := cmd.Start(); err != nil {
7483
cancel()
7584
return fmt.Errorf("failed to start external driver: %w", err)
7685
}
7786

7887
driverLogger := e.logger.WithField("driver", e.Name)
7988

80-
go func() {
81-
scanner := bufio.NewScanner(stderr)
82-
for scanner.Scan() {
83-
driverLogger.Info(scanner.Text())
84-
}
85-
}()
86-
87-
time.Sleep(4 * time.Second)
89+
time.Sleep(time.Millisecond * 100)
8890

8991
driverClient, err := client.NewDriverClient(stdin, stdout, e.logger)
9092
if err != nil {
@@ -148,15 +150,10 @@ func (r *Registry) RegisterDriver(name, path string) {
148150
return
149151
}
150152

151-
log := logrus.New()
152-
log.SetFormatter(&logrus.TextFormatter{
153-
FullTimestamp: true,
154-
})
155-
156153
DefaultRegistry.externalDrivers[name] = &ExternalDriver{
157154
Name: name,
158155
Path: path,
159-
logger: log,
156+
logger: logrus.New(),
160157
}
161158
}
162159

‎pkg/store/filenames/filenames.go

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -31,39 +31,40 @@ const (
3131
// Filenames that may appear under an instance directory
3232

3333
const (
34-
LimaYAML = "lima.yaml"
35-
LimaVersion = "lima-version" // Lima version used to create instance
36-
CIDataISO = "cidata.iso"
37-
CIDataISODir = "cidata"
38-
CloudConfig = "cloud-config.yaml"
39-
BaseDisk = "basedisk"
40-
DiffDisk = "diffdisk"
41-
Kernel = "kernel"
42-
KernelCmdline = "kernel.cmdline"
43-
Initrd = "initrd"
44-
QMPSock = "qmp.sock"
45-
SerialLog = "serial.log" // default serial (ttyS0, but ttyAMA0 on qemu-system-{arm,aarch64})
46-
SerialSock = "serial.sock"
47-
SerialPCILog = "serialp.log" // pci serial (ttyS0 on qemu-system-{arm,aarch64})
48-
SerialPCISock = "serialp.sock"
49-
SerialVirtioLog = "serialv.log" // virtio serial
50-
SerialVirtioSock = "serialv.sock"
51-
SSHSock = "ssh.sock"
52-
SSHConfig = "ssh.config"
53-
VhostSock = "virtiofsd-%d.sock"
54-
VNCDisplayFile = "vncdisplay"
55-
VNCPasswordFile = "vncpassword"
56-
GuestAgentSock = "ga.sock"
57-
VirtioPort = "io.lima-vm.guest_agent.0"
58-
HostAgentPID = "ha.pid"
59-
HostAgentSock = "ha.sock"
60-
HostAgentStdoutLog = "ha.stdout.log"
61-
HostAgentStderrLog = "ha.stderr.log"
62-
VzIdentifier = "vz-identifier"
63-
VzEfi = "vz-efi" // efi variable store
64-
QemuEfiCodeFD = "qemu-efi-code.fd" // efi code; not always created
65-
QemuEfiFullFD = "qemu-efi-full.fd" // concatenated efi vars and code; not always created
66-
AnsibleInventoryYAML = "ansible-inventory.yaml"
34+
LimaYAML = "lima.yaml"
35+
LimaVersion = "lima-version" // Lima version used to create instance
36+
CIDataISO = "cidata.iso"
37+
CIDataISODir = "cidata"
38+
CloudConfig = "cloud-config.yaml"
39+
BaseDisk = "basedisk"
40+
DiffDisk = "diffdisk"
41+
Kernel = "kernel"
42+
KernelCmdline = "kernel.cmdline"
43+
Initrd = "initrd"
44+
QMPSock = "qmp.sock"
45+
SerialLog = "serial.log" // default serial (ttyS0, but ttyAMA0 on qemu-system-{arm,aarch64})
46+
SerialSock = "serial.sock"
47+
SerialPCILog = "serialp.log" // pci serial (ttyS0 on qemu-system-{arm,aarch64})
48+
SerialPCISock = "serialp.sock"
49+
SerialVirtioLog = "serialv.log" // virtio serial
50+
SerialVirtioSock = "serialv.sock"
51+
SSHSock = "ssh.sock"
52+
SSHConfig = "ssh.config"
53+
VhostSock = "virtiofsd-%d.sock"
54+
VNCDisplayFile = "vncdisplay"
55+
VNCPasswordFile = "vncpassword"
56+
GuestAgentSock = "ga.sock"
57+
VirtioPort = "io.lima-vm.guest_agent.0"
58+
HostAgentPID = "ha.pid"
59+
HostAgentSock = "ha.sock"
60+
HostAgentStdoutLog = "ha.stdout.log"
61+
HostAgentStderrLog = "ha.stderr.log"
62+
ExternalDriverStderrLog = "ed.stderr.log"
63+
VzIdentifier = "vz-identifier"
64+
VzEfi = "vz-efi" // efi variable store
65+
QemuEfiCodeFD = "qemu-efi-code.fd" // efi code; not always created
66+
QemuEfiFullFD = "qemu-efi-full.fd" // concatenated efi vars and code; not always created
67+
AnsibleInventoryYAML = "ansible-inventory.yaml"
6768

6869
// SocketDir is the default location for forwarded sockets with a relative paths in HostSocket.
6970
SocketDir = "sock"

0 commit comments

Comments
 (0)