Skip to content

Commit

Permalink
format fix.
Browse files Browse the repository at this point in the history
  • Loading branch information
@hdwhdw
hdwhdw committed Feb 21, 2025
1 parent fd7a2a3 commit 4908b28
Show file tree
Hide file tree
Showing 2 changed files with 173 additions and 92 deletions.
66 changes: 37 additions & 29 deletions gnmi_server/gnoi.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,42 +371,50 @@ func (srv *SystemServer) SetPackage(rs gnoi_system_pb.System_SetPackageServer) e
return status.Errorf(codes.InvalidArgument, errMsg)
}

// Validate filename
// A filename and a version must be provided
if pkg.Package.Filename == "" {
log.Errorf("Filename is missing in package request")
return status.Errorf(codes.InvalidArgument, "filename is missing in package request")
}
if pkg.Package.Version == "" {
log.Errorf("Version is missing in package request")
return status.Errorf(codes.InvalidArgument, "version is missing in package request")
}
// Log the package filename and version
log.V(1).Infof("Package filename: %s, version: %s", pkg.Package.Filename, pkg.Package.Version)

// Download the package if RemoteDownload is provided
if pkg.Package.RemoteDownload != nil {
// Validate RemoteDownload
log.V(1).Infof("RemoteDownload provided")
// Check if the path is provided
if pkg.Package.RemoteDownload.Path == "" {
log.Errorf("RemoteDownload path is missing")
return status.Errorf(codes.InvalidArgument, "remote download path is missing")
}
log.V(1).Infof("RemoteDownload path: %s", pkg.Package.RemoteDownload.Path)


// Validate remote download information
if pkg.Package.RemoteDownload == nil || pkg.Package.RemoteDownload.Path == "" {
log.Errorf("Missing or invalid RemoteDownload information in package request")
return status.Errorf(codes.InvalidArgument, "RemoteDownload information is missing or invalid")
}

// Does not support not activating the image for now.
// TODO: Support not activating the image.
if pkg.Package.Activate != nil && !pkg.Package.Activate.Value {
log.Errorf("Not activating the image is not supported")
return status.Errorf(codes.InvalidArgument, "not activating the image is not supported")
}

// Log received package details
log.V(1).Infof("Received package information: Filename=%s, RemoteDownload=%v", pkg.Package.Filename, pkg.Package.RemoteDownload)

// Download the package
err = dbus.DownloadImage(pkg.Package.RemoteDownload.Path, pkg.Package.Filename)
if err != nil {
log.Errorf("Failed to download image from %s: %v", pkg.Package.RemoteDownload.Path, err)
return status.Errorf(codes.Internal, "failed to download image: %v", err)
// Download the package
err = dbus.DownloadImage(pkg.Package.RemoteDownload.Path, pkg.Package.Filename)
if err != nil {
log.Errorf("Failed to download image: %v", err)
return status.Errorf(codes.Internal, "failed to download image: %v", err)
}
log.V(1).Infof("Package %s downloaded successfully to %s", pkg.Package.Version, pkg.Package.Filename)
}

// Install the package
// TODO: clarify install, activate or simply putting the package on the host.
err = dbus.InstallImage(pkg.Package.Filename)
if err != nil {
log.Errorf("Failed to install image: %v", err)
return status.Errorf(codes.Internal, "failed to install image: %v", err)
// If activate is requested, install the package and set it to be the next boot image
if pkg.Package.Activate {
log.V(1).Infof("Activate is requested")
// Install the package
err = dbus.InstallImage(pkg.Package.Filename)
if err != nil {
log.Errorf("Failed to install image: %v", err)
return status.Errorf(codes.Internal, "failed to install image: %v", err)
}
log.V(1).Infof("Package %s installed successfully", pkg.Package.Filename)
// Currently, Installing the image will automatically set it as the next boot image
log.V(1).Infof("Package %s set as next boot image", pkg.Package.Filename)
}

// Send response to client
Expand Down
199 changes: 136 additions & 63 deletions gnmi_server/gnoi_system_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,25 +35,115 @@ func TestSetPackage(t *testing.T) {

mockServer.EXPECT().Context().Return(ctx).AnyTimes()

t.Run("SetPackageSuccess", func(t *testing.T) {
t.Run("SetPackageSuccessDownloadOnly", func(t *testing.T) {
mockServer.EXPECT().Recv().Return(&gnoi_system_pb.SetPackageRequest{
Request: &gnoi_system_pb.SetPackageRequest_Package{
Package: &gnoi_system_pb.Package{
RemoteDownload: &gnoi_common_pb.RemoteDownload{
Path: "http://example.com/package",
},
Filename: "package.bin",
Version: "1.0",
Activate: false,
},
},
}, nil).Times(1)

patches := gomonkey.NewPatches()
defer patches.Reset()

downloadCalled := false
installCalled := false

patches.ApplyMethod(reflect.TypeOf(&ssc.DbusClient{}), "DownloadImage", func(_ *ssc.DbusClient, path, filename string) error {
downloadCalled = true
return nil
})
patches.ApplyMethod(reflect.TypeOf(&ssc.DbusClient{}), "InstallImage", func(_ *ssc.DbusClient, filename string) error {
installCalled = true
return nil
})

mockServer.EXPECT().SendAndClose(gomock.Any()).Return(nil).Times(1)

err := srv.SetPackage(mockServer)
if err != nil {
t.Fatalf("SetPackage failed unexpectedly: %v", err)
}
if !downloadCalled {
t.Errorf("Expected DownloadImage to be called, but it was not")
}
if installCalled {
t.Errorf("Expected InstallImage not to be called, but it was")
}
})

t.Run("SetPackageSuccessInstallOnly", func(t *testing.T) {
mockServer.EXPECT().Recv().Return(&gnoi_system_pb.SetPackageRequest{
Request: &gnoi_system_pb.SetPackageRequest_Package{
Package: &gnoi_system_pb.Package{
Filename: "package.bin",
Version: "1.0",
Activate: true,
},
},
}, nil).Times(1)

patches := gomonkey.NewPatches()
defer patches.Reset()

downloadCalled := false
installCalled := false

patches.ApplyMethod(reflect.TypeOf(&ssc.DbusClient{}), "DownloadImage", func(_ *ssc.DbusClient, path, filename string) error {
downloadCalled = true
return nil
})
patches.ApplyMethod(reflect.TypeOf(&ssc.DbusClient{}), "InstallImage", func(_ *ssc.DbusClient, filename string) error {
installCalled = true
return nil
})

mockServer.EXPECT().SendAndClose(gomock.Any()).Return(nil).Times(1)

err := srv.SetPackage(mockServer)
if err != nil {
t.Fatalf("SetPackage failed unexpectedly: %v", err)
}
if downloadCalled {
t.Errorf("Expected DownloadImage not to be called, but it was")
}
if !installCalled {
t.Errorf("Expected InstallImage to be called, but it was not")
}
})

t.Run("SetPackageSuccessDownloadAndInstall", func(t *testing.T) {
mockServer.EXPECT().Recv().Return(&gnoi_system_pb.SetPackageRequest{
Request: &gnoi_system_pb.SetPackageRequest_Package{
Package: &gnoi_system_pb.Package{
RemoteDownload: &gnoi_common_pb.RemoteDownload{
Path: "http://example.com/package",
},
Filename: "package.bin",
Version: "1.0",
Activate: true,
},
},
}, nil).Times(1)

patches := gomonkey.NewPatches()
defer patches.Reset()

downloadCalled := false
installCalled := false

patches.ApplyMethod(reflect.TypeOf(&ssc.DbusClient{}), "DownloadImage", func(_ *ssc.DbusClient, path, filename string) error {
downloadCalled = true
return nil
})
patches.ApplyMethod(reflect.TypeOf(&ssc.DbusClient{}), "InstallImage", func(_ *ssc.DbusClient, filename string) error {
installCalled = true
return nil
})

Expand All @@ -63,6 +153,12 @@ func TestSetPackage(t *testing.T) {
if err != nil {
t.Fatalf("SetPackage failed unexpectedly: %v", err)
}
if !downloadCalled {
t.Errorf("Expected DownloadImage to be called, but it was not")
}
if !installCalled {
t.Errorf("Expected InstallImage to be called, but it was not")
}
})

t.Run("SetPackageDownloadFailure", func(t *testing.T) {
Expand All @@ -75,6 +171,8 @@ func TestSetPackage(t *testing.T) {
Path: "http://example.com/package",
},
Filename: "package.bin",
Version: "1.0",
Activate: false,
},
},
}, nil).Times(1)
Expand Down Expand Up @@ -105,10 +203,9 @@ func TestSetPackage(t *testing.T) {
mockServer.EXPECT().Recv().Return(&gnoi_system_pb.SetPackageRequest{
Request: &gnoi_system_pb.SetPackageRequest_Package{
Package: &gnoi_system_pb.Package{
RemoteDownload: &gnoi_common_pb.RemoteDownload{
Path: "http://example.com/package",
},
Filename: "package.bin",
Version: "1.0",
Activate: true,
},
},
}, nil).Times(1)
Expand Down Expand Up @@ -143,6 +240,8 @@ func TestSetPackage(t *testing.T) {
RemoteDownload: &gnoi_common_pb.RemoteDownload{
Path: "http://example.com/package",
},
Version: "1.0",
Activate: true,
},
},
}, nil).Times(1)
Expand All @@ -160,11 +259,40 @@ func TestSetPackage(t *testing.T) {
}
})

t.Run("SetPackageRemoteDownloadInfoMissing", func(t *testing.T) {
t.Run("SetPackageMissingVersion", func(t *testing.T) {
mockServer.EXPECT().Recv().Return(&gnoi_system_pb.SetPackageRequest{
Request: &gnoi_system_pb.SetPackageRequest_Package{
Package: &gnoi_system_pb.Package{
RemoteDownload: &gnoi_common_pb.RemoteDownload{
Path: "http://example.com/package",
},
Filename: "package.bin",
Activate: true,
},
},
}, nil).Times(1)
err := srv.SetPackage(mockServer)
if err == nil {
t.Fatalf("Expected error but got none")
}
expectedErrorCode := codes.InvalidArgument
st, ok := status.FromError(err)
if !ok {
t.Fatalf("Expected gRPC status error, but got %v", err)
}
if st.Code() != expectedErrorCode {
t.Errorf("Expected error code '%v', but got '%v'", expectedErrorCode, st.Code())
}
})

t.Run("SetPackageRemoteDownloadPathMissing", func(t *testing.T) {
mockServer.EXPECT().Recv().Return(&gnoi_system_pb.SetPackageRequest{
Request: &gnoi_system_pb.SetPackageRequest_Package{
Package: &gnoi_system_pb.Package{
RemoteDownload: &gnoi_common_pb.RemoteDownload{},
Filename: "package.bin",
Version: "1.0",
Activate: false,
},
},
}, nil).Times(1)
Expand Down Expand Up @@ -211,6 +339,8 @@ func TestSetPackage(t *testing.T) {
Path: "http://example.com/package",
},
Filename: "package.bin",
Version: "1.0",
Activate: false,
},
},
}, nil).Times(1)
Expand Down Expand Up @@ -240,25 +370,7 @@ func TestSetPackage(t *testing.T) {
}
})

t.Run("RecvFails", func(t *testing.T) {
expectedError := status.Errorf(codes.Internal, "Recv() failed")

mockServer.EXPECT().Recv().Return(nil, expectedError).Times(1)

err := srv.SetPackage(mockServer)
if err == nil {
t.Fatalf("Expected error but got none")
}
st, ok := status.FromError(err)
if !ok {
t.Fatalf("Expected gRPC status error, but got %v", err)
}
if st.Code() != status.Code(expectedError) {
t.Errorf("Expected error code '%v', but got '%v'", status.Code(expectedError), st.Code())
}
})

t.Run("InvalidRequestType", func(t *testing.T) {
t.Run("SetPackageInvalidRequestType", func(t *testing.T) {
mockServer.EXPECT().Recv().Return(&gnoi_system_pb.SetPackageRequest{
Request: &gnoi_system_pb.SetPackageRequest_Hash{ // Wrong type
Hash: &gnoi_types_pb.HashType{},
Expand All @@ -278,43 +390,4 @@ func TestSetPackage(t *testing.T) {
t.Errorf("Expected error code '%v', but got '%v'", expectedErrorCode, st.Code())
}
})

t.Run("SendAndCloseFails", func(t *testing.T) {
expectedError := status.Errorf(codes.Internal, "SendAndClose() failed")

mockServer.EXPECT().Recv().Return(&gnoi_system_pb.SetPackageRequest{
Request: &gnoi_system_pb.SetPackageRequest_Package{
Package: &gnoi_system_pb.Package{
RemoteDownload: &gnoi_common_pb.RemoteDownload{
Path: "http://example.com/package",
},
Filename: "package.bin",
},
},
}, nil).Times(1)

patches := gomonkey.NewPatches()
defer patches.Reset()

patches.ApplyMethod(reflect.TypeOf(&ssc.DbusClient{}), "DownloadImage", func(_ *ssc.DbusClient, path, filename string) error {
return nil
})
patches.ApplyMethod(reflect.TypeOf(&ssc.DbusClient{}), "InstallImage", func(_ *ssc.DbusClient, filename string) error {
return nil
})

mockServer.EXPECT().SendAndClose(gomock.Any()).Return(expectedError).Times(1)

err := srv.SetPackage(mockServer)
if err == nil {
t.Fatalf("Expected error but got none")
}
st, ok := status.FromError(err)
if !ok {
t.Fatalf("Expected gRPC status error, but got %v", err)
}
if st.Code() != status.Code(expectedError) {
t.Errorf("Expected error code '%v', but got '%v'", status.Code(expectedError), st.Code())
}
})
}

0 comments on commit 4908b28

Please sign in to comment.