diff --git a/Makefile b/Makefile index 789e7e14..9a90e028 100644 --- a/Makefile +++ b/Makefile @@ -70,7 +70,7 @@ $(GO_DEPS): go.mod $(PATCHES) swsscommon_wrap $(GNOI_YANG) $(GO) mod download github.com/google/gnxi@v0.0.0-20181220173256-89f51f0ce1e2 cp -r $(GOPATH)/pkg/mod/github.com/google/gnxi@v0.0.0-20181220173256-89f51f0ce1e2/* vendor/github.com/google/gnxi/ -# Apply patch from sonic-mgmt-common, ignore glog.patch because glog version changed +# Apply patch from sonic-mgmt-common, ignore glog.patch because glog version changed sed -i 's/patch -d $${DEST_DIR}\/github.com\/golang\/glog/\#patch -d $${DEST_DIR}\/github.com\/golang\/glog/g' $(MGMT_COMMON_DIR)/patches/apply.sh $(MGMT_COMMON_DIR)/patches/apply.sh vendor sed -i 's/#patch -d $${DEST_DIR}\/github.com\/golang\/glog/patch -d $${DEST_DIR}\/github.com\/golang\/glog/g' $(MGMT_COMMON_DIR)/patches/apply.sh @@ -106,7 +106,7 @@ endif $(GO) install -mod=vendor github.com/sonic-net/sonic-gnmi/gnmi_dump $(GO) install -mod=vendor github.com/sonic-net/sonic-gnmi/build/gnoi_yang/client/gnoi_openconfig_client $(GO) install -mod=vendor github.com/sonic-net/sonic-gnmi/build/gnoi_yang/client/gnoi_sonic_client - + endif # download and apply patch for gnmi client, which will break advancetls @@ -216,11 +216,12 @@ endif sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race -coverprofile=coverage-data.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/sonic_data_client sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race -coverprofile=coverage-dbus.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/sonic_service_client sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(TESTENV) $(GO) test -race -coverprofile=coverage-translutils.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/transl_utils + sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(TESTENV) $(GO) test -race -coverprofile=coverage-gnoi-client-system.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/gnoi_client/system $(GO) install github.com/axw/gocov/gocov@v1.1.0 $(GO) install github.com/AlekSi/gocov-xml@latest $(GO) mod vendor gocov convert coverage-*.txt | gocov-xml -source $(shell pwd) > coverage.xml - rm -rf coverage-*.txt + rm -rf coverage-*.txt check_memleak: $(DBCONFG) $(ENVFILE) sudo CGO_LDFLAGS="$(MEMCHECK_CGO_LDFLAGS)" CGO_CXXFLAGS="$(MEMCHECK_CGO_CXXFLAGS)" $(GO) test -coverprofile=coverage-telemetry.txt -covermode=atomic -mod=vendor $(MEMCHECK_FLAGS) -v github.com/sonic-net/sonic-gnmi/telemetry diff --git a/gnoi_client/gnoi_client.go b/gnoi_client/gnoi_client.go index 1f9ea53f..ffa2e153 100644 --- a/gnoi_client/gnoi_client.go +++ b/gnoi_client/gnoi_client.go @@ -43,6 +43,8 @@ func main() { system.RebootStatus(conn, ctx) case "KillProcess": system.KillProcess(conn, ctx) + case "SetPackage": + system.SetPackage(conn, ctx) default: panic("Invalid RPC Name") } diff --git a/gnoi_client/system/set_package.go b/gnoi_client/system/set_package.go new file mode 100644 index 00000000..8f1ee08b --- /dev/null +++ b/gnoi_client/system/set_package.go @@ -0,0 +1,120 @@ +package system + +import ( + "context" + "flag" + "fmt" + + "github.com/openconfig/gnoi/common" + "github.com/openconfig/gnoi/system" + "github.com/sonic-net/sonic-gnmi/gnoi_client/utils" + "google.golang.org/grpc" +) + +var ( + // Flags for System.SetPackage + filename = flag.String("package_filename", "", "Destination path and filename of the package") + version = flag.String("package_version", "", "Version of the package, i.e. vendor internal name") + url = flag.String("package_url", "", "URL to download the package from") + activate = flag.Bool("package_activate", true, "Whether to activate the package after setting it") +) + +// newSystemClient is a package-level variable that returns a new system.SystemClient. +// We define it here so that unit tests can replace it with a mock constructor if needed. +var newSystemClient = func(conn *grpc.ClientConn) system.SystemClient { + return system.NewSystemClient(conn) +} + +// SetPackage is the main entry point. It validates flags, creates the SystemClient, +// and then calls setPackageClient to perform the actual SetPackage gRPC flow. +func SetPackage(conn *grpc.ClientConn, ctx context.Context) { + fmt.Println("System SetPackage") + + // Attach user credentials if needed. + ctx = utils.SetUserCreds(ctx) + + // Validate the flags before proceeding. + err := validateFlags() + if err != nil { + fmt.Println("Error validating flags:", err) + return + } + + // Create a new gNOI SystemClient using the function defined above. + sc := newSystemClient(conn) + + // Call the helper that implements the SetPackage logic (sending requests, closing, etc.). + if err := setPackageClient(sc, ctx); err != nil { + fmt.Println("Error during SetPackage:", err) + } +} + +// setPackageClient contains the core gRPC calls to send the package request and +// receive the response. We separate it out for easier testing and mocking. +func setPackageClient(sc system.SystemClient, ctx context.Context) error { + // Prepare the remote download info. + download := &common.RemoteDownload{ + Path: *url, + } + + // Build the package with flags. + pkg := &system.Package{ + Filename: *filename, + Version: *version, + Activate: *activate, + RemoteDownload: download, + } + + // The gNOI SetPackageRequest can contain different request types, but we only + // use the "Package" request type here. + req := &system.SetPackageRequest{ + Request: &system.SetPackageRequest_Package{ + Package: pkg, + }, + } + + // Open a streaming RPC. + stream, err := sc.SetPackage(ctx) + if err != nil { + return fmt.Errorf("error creating stream: %v", err) + } + + // Send the package information. + if err := stream.Send(req); err != nil { + return fmt.Errorf("error sending package request: %v", err) + } + + // Close the send direction of the stream. The device should download the + // package itself, so we are not sending direct data or checksums here. + if err := stream.CloseSend(); err != nil { + return fmt.Errorf("error closing send direction: %v", err) + } + + // Receive the final response from the device. + resp, err := stream.CloseAndRecv() + if err != nil { + return fmt.Errorf("error receiving response: %v", err) + } + + // For demonstration purposes, we simply print the response. + fmt.Println(resp) + return nil +} + +// validateFlags ensures all required flags are set correctly before we proceed. +func validateFlags() error { + if *filename == "" { + return fmt.Errorf("missing -package_filename: Destination path and filename of the package is required for the SetPackage operation") + } + if *version == "" { + return fmt.Errorf("missing -package_version: Version of the package is required for the SetPackage operation") + } + if *url == "" { + return fmt.Errorf("missing -package_url: URL to download the package from is required for the SetPackage operation. Direct transfer is not supported yet") + } + if !*activate { + // TODO: Currently, installing the image will always set it to default. + return fmt.Errorf("-package_activate=false is not yet supported: The package will always be activated after setting it") + } + return nil +} diff --git a/gnoi_client/system/system_mock_test.go b/gnoi_client/system/system_mock_test.go new file mode 100644 index 00000000..9e6e2a4d --- /dev/null +++ b/gnoi_client/system/system_mock_test.go @@ -0,0 +1,320 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/openconfig/gnoi/system (interfaces: SystemClient,System_SetPackageClient) + +// Package system is a generated GoMock package. +package system + +import ( + context "context" + gomock "github.com/golang/mock/gomock" + system "github.com/openconfig/gnoi/system" + grpc "google.golang.org/grpc" + metadata "google.golang.org/grpc/metadata" + reflect "reflect" +) + +// MockSystemClient is a mock of SystemClient interface +type MockSystemClient struct { + ctrl *gomock.Controller + recorder *MockSystemClientMockRecorder +} + +// MockSystemClientMockRecorder is the mock recorder for MockSystemClient +type MockSystemClientMockRecorder struct { + mock *MockSystemClient +} + +// NewMockSystemClient creates a new mock instance +func NewMockSystemClient(ctrl *gomock.Controller) *MockSystemClient { + mock := &MockSystemClient{ctrl: ctrl} + mock.recorder = &MockSystemClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockSystemClient) EXPECT() *MockSystemClientMockRecorder { + return m.recorder +} + +// CancelReboot mocks base method +func (m *MockSystemClient) CancelReboot(arg0 context.Context, arg1 *system.CancelRebootRequest, arg2 ...grpc.CallOption) (*system.CancelRebootResponse, error) { + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "CancelReboot", varargs...) + ret0, _ := ret[0].(*system.CancelRebootResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CancelReboot indicates an expected call of CancelReboot +func (mr *MockSystemClientMockRecorder) CancelReboot(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CancelReboot", reflect.TypeOf((*MockSystemClient)(nil).CancelReboot), varargs...) +} + +// KillProcess mocks base method +func (m *MockSystemClient) KillProcess(arg0 context.Context, arg1 *system.KillProcessRequest, arg2 ...grpc.CallOption) (*system.KillProcessResponse, error) { + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "KillProcess", varargs...) + ret0, _ := ret[0].(*system.KillProcessResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// KillProcess indicates an expected call of KillProcess +func (mr *MockSystemClientMockRecorder) KillProcess(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "KillProcess", reflect.TypeOf((*MockSystemClient)(nil).KillProcess), varargs...) +} + +// Ping mocks base method +func (m *MockSystemClient) Ping(arg0 context.Context, arg1 *system.PingRequest, arg2 ...grpc.CallOption) (system.System_PingClient, error) { + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "Ping", varargs...) + ret0, _ := ret[0].(system.System_PingClient) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Ping indicates an expected call of Ping +func (mr *MockSystemClientMockRecorder) Ping(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Ping", reflect.TypeOf((*MockSystemClient)(nil).Ping), varargs...) +} + +// Reboot mocks base method +func (m *MockSystemClient) Reboot(arg0 context.Context, arg1 *system.RebootRequest, arg2 ...grpc.CallOption) (*system.RebootResponse, error) { + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "Reboot", varargs...) + ret0, _ := ret[0].(*system.RebootResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Reboot indicates an expected call of Reboot +func (mr *MockSystemClientMockRecorder) Reboot(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Reboot", reflect.TypeOf((*MockSystemClient)(nil).Reboot), varargs...) +} + +// RebootStatus mocks base method +func (m *MockSystemClient) RebootStatus(arg0 context.Context, arg1 *system.RebootStatusRequest, arg2 ...grpc.CallOption) (*system.RebootStatusResponse, error) { + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "RebootStatus", varargs...) + ret0, _ := ret[0].(*system.RebootStatusResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// RebootStatus indicates an expected call of RebootStatus +func (mr *MockSystemClientMockRecorder) RebootStatus(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RebootStatus", reflect.TypeOf((*MockSystemClient)(nil).RebootStatus), varargs...) +} + +// SetPackage mocks base method +func (m *MockSystemClient) SetPackage(arg0 context.Context, arg1 ...grpc.CallOption) (system.System_SetPackageClient, error) { + varargs := []interface{}{arg0} + for _, a := range arg1 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "SetPackage", varargs...) + ret0, _ := ret[0].(system.System_SetPackageClient) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// SetPackage indicates an expected call of SetPackage +func (mr *MockSystemClientMockRecorder) SetPackage(arg0 interface{}, arg1 ...interface{}) *gomock.Call { + varargs := append([]interface{}{arg0}, arg1...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetPackage", reflect.TypeOf((*MockSystemClient)(nil).SetPackage), varargs...) +} + +// SwitchControlProcessor mocks base method +func (m *MockSystemClient) SwitchControlProcessor(arg0 context.Context, arg1 *system.SwitchControlProcessorRequest, arg2 ...grpc.CallOption) (*system.SwitchControlProcessorResponse, error) { + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "SwitchControlProcessor", varargs...) + ret0, _ := ret[0].(*system.SwitchControlProcessorResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// SwitchControlProcessor indicates an expected call of SwitchControlProcessor +func (mr *MockSystemClientMockRecorder) SwitchControlProcessor(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SwitchControlProcessor", reflect.TypeOf((*MockSystemClient)(nil).SwitchControlProcessor), varargs...) +} + +// Time mocks base method +func (m *MockSystemClient) Time(arg0 context.Context, arg1 *system.TimeRequest, arg2 ...grpc.CallOption) (*system.TimeResponse, error) { + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "Time", varargs...) + ret0, _ := ret[0].(*system.TimeResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Time indicates an expected call of Time +func (mr *MockSystemClientMockRecorder) Time(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Time", reflect.TypeOf((*MockSystemClient)(nil).Time), varargs...) +} + +// Traceroute mocks base method +func (m *MockSystemClient) Traceroute(arg0 context.Context, arg1 *system.TracerouteRequest, arg2 ...grpc.CallOption) (system.System_TracerouteClient, error) { + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "Traceroute", varargs...) + ret0, _ := ret[0].(system.System_TracerouteClient) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Traceroute indicates an expected call of Traceroute +func (mr *MockSystemClientMockRecorder) Traceroute(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Traceroute", reflect.TypeOf((*MockSystemClient)(nil).Traceroute), varargs...) +} + +// MockSystem_SetPackageClient is a mock of System_SetPackageClient interface +type MockSystem_SetPackageClient struct { + ctrl *gomock.Controller + recorder *MockSystem_SetPackageClientMockRecorder +} + +// MockSystem_SetPackageClientMockRecorder is the mock recorder for MockSystem_SetPackageClient +type MockSystem_SetPackageClientMockRecorder struct { + mock *MockSystem_SetPackageClient +} + +// NewMockSystem_SetPackageClient creates a new mock instance +func NewMockSystem_SetPackageClient(ctrl *gomock.Controller) *MockSystem_SetPackageClient { + mock := &MockSystem_SetPackageClient{ctrl: ctrl} + mock.recorder = &MockSystem_SetPackageClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockSystem_SetPackageClient) EXPECT() *MockSystem_SetPackageClientMockRecorder { + return m.recorder +} + +// CloseAndRecv mocks base method +func (m *MockSystem_SetPackageClient) CloseAndRecv() (*system.SetPackageResponse, error) { + ret := m.ctrl.Call(m, "CloseAndRecv") + ret0, _ := ret[0].(*system.SetPackageResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CloseAndRecv indicates an expected call of CloseAndRecv +func (mr *MockSystem_SetPackageClientMockRecorder) CloseAndRecv() *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CloseAndRecv", reflect.TypeOf((*MockSystem_SetPackageClient)(nil).CloseAndRecv)) +} + +// CloseSend mocks base method +func (m *MockSystem_SetPackageClient) CloseSend() error { + ret := m.ctrl.Call(m, "CloseSend") + ret0, _ := ret[0].(error) + return ret0 +} + +// CloseSend indicates an expected call of CloseSend +func (mr *MockSystem_SetPackageClientMockRecorder) CloseSend() *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CloseSend", reflect.TypeOf((*MockSystem_SetPackageClient)(nil).CloseSend)) +} + +// Context mocks base method +func (m *MockSystem_SetPackageClient) Context() context.Context { + ret := m.ctrl.Call(m, "Context") + ret0, _ := ret[0].(context.Context) + return ret0 +} + +// Context indicates an expected call of Context +func (mr *MockSystem_SetPackageClientMockRecorder) Context() *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Context", reflect.TypeOf((*MockSystem_SetPackageClient)(nil).Context)) +} + +// Header mocks base method +func (m *MockSystem_SetPackageClient) Header() (metadata.MD, error) { + ret := m.ctrl.Call(m, "Header") + ret0, _ := ret[0].(metadata.MD) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Header indicates an expected call of Header +func (mr *MockSystem_SetPackageClientMockRecorder) Header() *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Header", reflect.TypeOf((*MockSystem_SetPackageClient)(nil).Header)) +} + +// RecvMsg mocks base method +func (m *MockSystem_SetPackageClient) RecvMsg(arg0 interface{}) error { + ret := m.ctrl.Call(m, "RecvMsg", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// RecvMsg indicates an expected call of RecvMsg +func (mr *MockSystem_SetPackageClientMockRecorder) RecvMsg(arg0 interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RecvMsg", reflect.TypeOf((*MockSystem_SetPackageClient)(nil).RecvMsg), arg0) +} + +// Send mocks base method +func (m *MockSystem_SetPackageClient) Send(arg0 *system.SetPackageRequest) error { + ret := m.ctrl.Call(m, "Send", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// Send indicates an expected call of Send +func (mr *MockSystem_SetPackageClientMockRecorder) Send(arg0 interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Send", reflect.TypeOf((*MockSystem_SetPackageClient)(nil).Send), arg0) +} + +// SendMsg mocks base method +func (m *MockSystem_SetPackageClient) SendMsg(arg0 interface{}) error { + ret := m.ctrl.Call(m, "SendMsg", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// SendMsg indicates an expected call of SendMsg +func (mr *MockSystem_SetPackageClientMockRecorder) SendMsg(arg0 interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendMsg", reflect.TypeOf((*MockSystem_SetPackageClient)(nil).SendMsg), arg0) +} + +// Trailer mocks base method +func (m *MockSystem_SetPackageClient) Trailer() metadata.MD { + ret := m.ctrl.Call(m, "Trailer") + ret0, _ := ret[0].(metadata.MD) + return ret0 +} + +// Trailer indicates an expected call of Trailer +func (mr *MockSystem_SetPackageClientMockRecorder) Trailer() *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Trailer", reflect.TypeOf((*MockSystem_SetPackageClient)(nil).Trailer)) +} diff --git a/gnoi_client/system/system_test.go b/gnoi_client/system/system_test.go new file mode 100644 index 00000000..4a472091 --- /dev/null +++ b/gnoi_client/system/system_test.go @@ -0,0 +1,196 @@ +package system + +import ( + "bytes" + "context" + "io" + "os" + "strings" + "testing" + + "github.com/golang/mock/gomock" + "github.com/openconfig/gnoi/system" + "google.golang.org/grpc" +) + +// TestValidateFlags verifies flag validation logic using a table-driven approach. +func TestValidateFlags(t *testing.T) { + // Save original flag values + origFilename := *filename + origVersion := *version + origURL := *url + origActivate := *activate + + // Restore them when test finishes + defer func() { + *filename = origFilename + *version = origVersion + *url = origURL + *activate = origActivate + }() + + tests := []struct { + name string + fn string + ver string + u string + act bool + wantErr bool + errSub string // substring we expect in the error + }{ + { + name: "Missing filename", + fn: "", + ver: "1.0", + u: "http://example.com/pkg", + act: true, + wantErr: true, + errSub: "missing -package_filename", + }, + { + name: "Missing version", + fn: "sonic.pkg", + ver: "", + u: "http://example.com/pkg", + act: true, + wantErr: true, + errSub: "missing -package_version", + }, + { + name: "Missing url", + fn: "sonic.pkg", + ver: "1.0", + u: "", + act: true, + wantErr: true, + errSub: "missing -package_url", + }, + { + name: "Activate false not supported", + fn: "sonic.pkg", + ver: "1.0", + u: "http://example.com/pkg", + act: false, + wantErr: true, + errSub: "-package_activate=false is not yet supported", + }, + { + name: "Valid flags", + fn: "sonic.pkg", + ver: "1.0", + u: "http://example.com/pkg", + act: true, + wantErr: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + *filename = tc.fn + *version = tc.ver + *url = tc.u + *activate = tc.act + + err := validateFlags() + if tc.wantErr && err == nil { + t.Fatalf("Expected an error, got none") + } + if !tc.wantErr && err != nil { + t.Fatalf("Did not expect error, got %v", err) + } + if tc.wantErr && err != nil && !strings.Contains(err.Error(), tc.errSub) { + t.Fatalf("Expected error containing %q, got %v", tc.errSub, err) + } + }) + } +} + +// TestSetPackage_FlagValidationError ensures SetPackage prints the flag error and returns +// if validateFlags() fails, without calling setPackageClient logic. +func TestSetPackage_FlagValidationError(t *testing.T) { + // Force an error by leaving -package_filename blank + *filename = "" + *version = "1.0" + *url = "http://example.com/package" + *activate = true + + // Capture stdout to see the printed error + oldStdout := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + // We pass a nil *grpc.ClientConn here because we expect an early return. + SetPackage(nil, context.Background()) + + // Restore stdout + w.Close() + os.Stdout = oldStdout + + var buf bytes.Buffer + io.Copy(&buf, r) + + output := buf.String() + if !strings.Contains(output, "Error validating flags") { + t.Errorf("Expected validation error in output, got:\n%s", output) + } + if !strings.Contains(output, "missing -package_filename") { + t.Errorf("Expected 'missing -package_filename' in error, got:\n%s", output) + } +} + +// TestSetPackage_Success tests that SetPackage calls newSystemClient, then setPackageClient, +// and the gRPC flow with our mock client is correct. +func TestSetPackage_Success(t *testing.T) { + // We'll mock out the entire gRPC flow. + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockClient := NewMockSystemClient(ctrl) + mockStream := NewMockSystem_SetPackageClient(ctrl) + + // Override the global newSystemClient to return our mock client. + original := newSystemClient + newSystemClient = func(conn *grpc.ClientConn) system.SystemClient { + return mockClient + } + defer func() { newSystemClient = original }() + + // Provide valid flags so validateFlags() passes. + *filename = "sonic.pkg" + *version = "1.0" + *url = "http://example.com/pkg" + *activate = true + + // We'll capture stdout to verify what's printed at the end. + oldStdout := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + // We expect the mockClient.SetPackage(...) to be called and return mockStream. + mockClient.EXPECT().SetPackage(gomock.Any()).Return(mockStream, nil) + + // Next, we expect the stream.Send(...) call. + // If you want to inspect the request, you can use gomock.Any() or a custom matcher. + mockStream.EXPECT().Send(gomock.Any()).Return(nil) + + // Then CloseSend(). + mockStream.EXPECT().CloseSend().Return(nil) + + // Finally CloseAndRecv(), returning a dummy SetPackageResponse. + mockStream.EXPECT().CloseAndRecv().Return(&system.SetPackageResponse{}, nil) + + // Actually call SetPackage; pass a nil conn since it won't be used (we override newSystemClient). + SetPackage(nil, context.Background()) + + // Restore stdout + w.Close() + os.Stdout = oldStdout + + var buf bytes.Buffer + io.Copy(&buf, r) + output := buf.String() + + if !strings.Contains(output, "System SetPackage") { + t.Errorf("Expected 'System SetPackage' in output, got:\n%s", output) + } +} diff --git a/go.mod b/go.mod index 6282b059..88dc2f3c 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/godbus/dbus/v5 v5.1.0 github.com/gogo/protobuf v1.3.2 github.com/golang/glog v1.2.0 + github.com/golang/mock v1.1.1 github.com/golang/protobuf v1.5.4 github.com/google/gnxi v0.0.0-20181220173256-89f51f0ce1e2 github.com/kylelemons/godebug v1.1.0 diff --git a/go.sum b/go.sum index 8802e8ee..3fa70ac5 100644 --- a/go.sum +++ b/go.sum @@ -43,6 +43,7 @@ github.com/golang/glog v1.2.0 h1:uCdmnmatrKCgMBlM4rMuJZWOkPDqdbZPnrMXDY4gI68= github.com/golang/glog v1.2.0/go.mod h1:6AhwSGph0fcJtXVM/PEHPqZlFeoLxhs7/t5UDAwmO+w= github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e h1:1r7pUrabqp18hOBcwBwiTsbnFeTZHV9eER/QT5JVZxY= github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= +github.com/golang/mock v1.1.1 h1:G5FRp8JnTd7RQH5kemVNlMeyXQAztQ3mOWV95KxsXH8= github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= @@ -58,6 +59,8 @@ github.com/golang/protobuf v1.4.1/go.mod h1:U8fpvMrcmy5pZrNK1lt4xCsGvpyWQ/VVv6QD github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= +github.com/google/gnxi v0.0.0-20181220173256-89f51f0ce1e2 h1:Cb0tImy52kinqtUXYR59HSNXYz83Y69yD8/zoKScE3s= +github.com/google/gnxi v0.0.0-20181220173256-89f51f0ce1e2/go.mod h1:6kkMbKS62iZMyk1q0zukcqkEJwnIhcbgg/hmoFmRDZk= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= @@ -71,8 +74,6 @@ github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+ github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= -github.com/google/gnxi v0.0.0-20181220173256-89f51f0ce1e2 h1:Cb0tImy52kinqtUXYR59HSNXYz83Y69yD8/zoKScE3s= -github.com/google/gnxi v0.0.0-20181220173256-89f51f0ce1e2/go.mod h1:6kkMbKS62iZMyk1q0zukcqkEJwnIhcbgg/hmoFmRDZk= github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=