Skip to content

Commit

Permalink
CI: Fix gometalinter errors
Browse files Browse the repository at this point in the history
Apply fixes to calm `gometalinter`.

Fixes kata-containers#105.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
  • Loading branch information
jodh-intel committed Jan 18, 2018
1 parent fb52144 commit 5ebcaec
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 64 deletions.
5 changes: 5 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ test: go-test
go-test:
bash hack/go-test.sh

check: check-go-static

check-go-static:
bash .ci/static-checks.sh

define INSTALL_FILE
install -D -m 644 $1 $(DESTDIR)$2/$1 || exit 1;
endef
Expand Down
44 changes: 31 additions & 13 deletions config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (
)

func TestNewConfig(t *testing.T) {
assert := assert.New(t)

testLogLevel := logrus.DebugLevel

expectedConfig := agentConfig{
Expand All @@ -25,37 +27,45 @@ func TestNewConfig(t *testing.T) {

config := newConfig(testLogLevel)

assert.True(t, reflect.DeepEqual(config, expectedConfig),
assert.True(reflect.DeepEqual(config, expectedConfig),
"Config structures should be identical: got %+v, expecting %+v",
config, expectedConfig)
}

func TestParseCmdlineOptionEmptyOption(t *testing.T) {
assert := assert.New(t)

a := &agentConfig{}

err := a.parseCmdlineOption("")
assert.Nil(t, err, "%v", err)
assert.NoError(err, "%v", err)
}

func TestParseCmdlineOptionWrongOptionValue(t *testing.T) {
assert := assert.New(t)

a := &agentConfig{}

wrongOption := logLevelFlag + "=debgu"

err := a.parseCmdlineOption(wrongOption)
assert.Error(t, err, "Parsing should fail because wrong option %q", wrongOption)
assert.Errorf(err, "Parsing should fail because wrong option %q", wrongOption)
}

func TestParseCmdlineOptionWrongOptionParam(t *testing.T) {
assert := assert.New(t)

a := &agentConfig{}

wrongOption := "agent.lgo=debug"

err := a.parseCmdlineOption(wrongOption)
assert.Error(t, err, "Parsing should fail because wrong option %q", wrongOption)
assert.Errorf(err, "Parsing should fail because wrong option %q", wrongOption)
}

func TestParseCmdlineOptionCorrectOptions(t *testing.T) {
assert := assert.New(t)

a := &agentConfig{}

logFlagList := []string{"debug", "info", "warn", "error", "fatal", "panic"}
Expand All @@ -64,11 +74,13 @@ func TestParseCmdlineOptionCorrectOptions(t *testing.T) {
option := logLevelFlag + "=" + logFlag

err := a.parseCmdlineOption(option)
assert.Nil(t, err, "%v", err)
assert.NoError(err, "%v", err)
}
}

func TestParseCmdlineOptionIncorrectOptions(t *testing.T) {
assert := assert.New(t)

a := &agentConfig{}

logFlagList := []string{"debg", "ifo", "wan", "eror", "ftal", "pnic"}
Expand All @@ -77,37 +89,43 @@ func TestParseCmdlineOptionIncorrectOptions(t *testing.T) {
option := logLevelFlag + "=" + logFlag

err := a.parseCmdlineOption(option)
assert.Error(t, err, "Should fail because of incorrect option %q", logFlag)
assert.Errorf(err, "Should fail because of incorrect option %q", logFlag)
}
}

func TestGetConfigEmptyFileName(t *testing.T) {
assert := assert.New(t)

a := &agentConfig{}

err := a.getConfig("")
assert.Error(t, err, "Should fail because command line path is empty")
assert.Error(err, "Should fail because command line path is empty")
}

func TestGetConfigFilePathNotExist(t *testing.T) {
assert := assert.New(t)

a := &agentConfig{}

tmpFile, err := ioutil.TempFile("", "test")
assert.Nil(t, err, "%v", err)
assert.NoError(err, "%v", err)

fileName := tmpFile.Name()
tmpFile.Close()
err = os.Remove(fileName)
assert.Nil(t, err, "%v", err)
assert.NoError(err, "%v", err)

err = a.getConfig(fileName)
assert.Error(t, err, "Should fail because command line path does not exist")
assert.Error(err, "Should fail because command line path does not exist")
}

func TestGetConfig(t *testing.T) {
assert := assert.New(t)

a := &agentConfig{}

tmpFile, err := ioutil.TempFile("", "test")
assert.Nil(t, err, "%v", err)
assert.NoError(err, "%v", err)
fileName := tmpFile.Name()

tmpFile.Write([]byte(logLevelFlag + "=info"))
Expand All @@ -116,9 +134,9 @@ func TestGetConfig(t *testing.T) {
defer os.Remove(fileName)

err = a.getConfig(fileName)
assert.Nil(t, err, "%v", err)
assert.NoError(err, "%v", err)

assert.True(t, a.logLevel == logrus.InfoLevel,
assert.True(a.logLevel == logrus.InfoLevel,
"Log levels should be identical: got %+v, expecting %+v",
a.logLevel, logrus.InfoLevel)
}
60 changes: 33 additions & 27 deletions protocols/grpc/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,38 +26,38 @@ func copyValue(to, from reflect.Value) error {
to.Set(reflect.New(to.Type().Elem()))
if fromKind == reflect.Ptr {
return copyValue(to.Elem(), from.Elem())
} else {
return copyValue(to.Elem(), from)
}
} else {
// Here the destination is not a pointer.
// Let's check what's the origin.
if fromKind == reflect.Ptr {
return copyValue(to, from.Elem())
}

switch toKind {
case reflect.Struct:
return copyStructValue(to, from)
case reflect.Slice:
return copySliceValue(to, from)
case reflect.Map:
return copyMapValue(to, from)
default:
// We now are copying non pointers scalar.
// This is the leaf of the recursion.
if from.Type() != to.Type() {
if from.Type().ConvertibleTo(to.Type()) {
to.Set(from.Convert(to.Type()))
return nil
} else {
return fmt.Errorf("Can not convert %v to %v", from.Type(), to.Type())
}
} else {
to.Set(from)
return copyValue(to.Elem(), from)
}

// Here the destination is not a pointer.
// Let's check what's the origin.
if fromKind == reflect.Ptr {
return copyValue(to, from.Elem())
}

switch toKind {
case reflect.Struct:
return copyStructValue(to, from)
case reflect.Slice:
return copySliceValue(to, from)
case reflect.Map:
return copyMapValue(to, from)
default:
// We now are copying non pointers scalar.
// This is the leaf of the recursion.
if from.Type() != to.Type() {
if from.Type().ConvertibleTo(to.Type()) {
to.Set(from.Convert(to.Type()))
return nil
}

return fmt.Errorf("Can not convert %v to %v", from.Type(), to.Type())
}

to.Set(from)
return nil
}
}

Expand Down Expand Up @@ -221,6 +221,7 @@ func copyStruct(to interface{}, from interface{}) (err error) {
return copyStructValue(toVal, fromVal)
}

// OCItoGRPC converts an OCI specification to its gRPC representation
func OCItoGRPC(ociSpec *specs.Spec) (*Spec, error) {
s := &Spec{}

Expand All @@ -229,6 +230,7 @@ func OCItoGRPC(ociSpec *specs.Spec) (*Spec, error) {
return s, err
}

// GRPCtoOCI converts a gRPC specification back into an OCI representation
func GRPCtoOCI(grpcSpec *Spec) (*specs.Spec, error) {
s := &specs.Spec{}

Expand All @@ -237,6 +239,8 @@ func GRPCtoOCI(grpcSpec *Spec) (*specs.Spec, error) {
return s, err
}

// ProcessOCItoGRPC converts an OCI process specification into its gRPC
// representation
func ProcessOCItoGRPC(ociProcess *specs.Process) (*Process, error) {
s := &Process{}

Expand All @@ -245,6 +249,8 @@ func ProcessOCItoGRPC(ociProcess *specs.Process) (*Process, error) {
return s, err
}

// ProcessGRPCtoOCI converts a gRPC specification back into an OCI
// representation
func ProcessGRPCtoOCI(grpcProcess *Process) (*specs.Process, error) {
s := &specs.Process{}

Expand Down
16 changes: 8 additions & 8 deletions protocols/grpc/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,10 @@ func TestOCItoGRPC(t *testing.T) {
assert := assert.New(t)
var ociSpec specs.Spec

configJsonBytes, err := ioutil.ReadFile(ociConfigFile)
configJSONBytes, err := ioutil.ReadFile(ociConfigFile)
assert.NoError(err, "Could not open OCI config file")

err = json.Unmarshal(configJsonBytes, &ociSpec)
err = json.Unmarshal(configJSONBytes, &ociSpec)
assert.NoError(err, "Could not unmarshall OCI config file")

spec, err := OCItoGRPC(&ociSpec)
Expand All @@ -98,10 +98,10 @@ func TestGRPCtoOCI(t *testing.T) {

var ociSpec specs.Spec

configJsonBytes, err := ioutil.ReadFile(ociConfigFile)
configJSONBytes, err := ioutil.ReadFile(ociConfigFile)
assert.NoError(err, "Could not open OCI config file")

err = json.Unmarshal(configJsonBytes, &ociSpec)
err = json.Unmarshal(configJSONBytes, &ociSpec)
assert.NoError(err, "Could not unmarshall OCI config file")

grpcSpec, err := OCItoGRPC(&ociSpec)
Expand All @@ -117,10 +117,10 @@ func TestProcessOCItoGRPC(t *testing.T) {
assert := assert.New(t)
var ociSpec specs.Spec

configJsonBytes, err := ioutil.ReadFile(ociConfigFile)
configJSONBytes, err := ioutil.ReadFile(ociConfigFile)
assert.NoError(err, "Could not open OCI config file")

err = json.Unmarshal(configJsonBytes, &ociSpec)
err = json.Unmarshal(configJSONBytes, &ociSpec)
assert.NoError(err, "Could not unmarshall OCI config file")

process, err := ProcessOCItoGRPC(ociSpec.Process)
Expand All @@ -133,10 +133,10 @@ func TestProcessGRPCtoOCI(t *testing.T) {

var ociSpec specs.Spec

configJsonBytes, err := ioutil.ReadFile(ociConfigFile)
configJSONBytes, err := ioutil.ReadFile(ociConfigFile)
assert.NoError(err, "Could not open OCI config file")

err = json.Unmarshal(configJsonBytes, &ociSpec)
err = json.Unmarshal(configJSONBytes, &ociSpec)
assert.NoError(err, "Could not unmarshall OCI config file")

grpcProcess, err := ProcessOCItoGRPC(ociSpec.Process)
Expand Down
2 changes: 2 additions & 0 deletions protocols/grpc/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,6 @@

package grpc

// APIVersion specifies the version of the gRPC communications protocol used
// by Kata Containers.
const APIVersion = "0.0.1"
34 changes: 18 additions & 16 deletions protocols/mockserver/mockserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
)

const (
// MockServerVersion specifies the version of the fake server
MockServerVersion = "mock.0.1"

podStartingPid = 100
Expand Down Expand Up @@ -47,6 +48,7 @@ type mockServer struct {
pod *pod
}

// NewMockServer creates a new gRPC server
func NewMockServer() *grpc.Server {
mock := &mockServer{}
serv := grpc.NewServer()
Expand All @@ -63,48 +65,48 @@ func validateOCISpec(spec *pb.Spec) error {
return nil
}

func (m *mockServer) checkExist(containerId, execId string, createContainer, checkProcess bool) error {
func (m *mockServer) checkExist(containerID, execID string, createContainer, checkProcess bool) error {
if m.pod == nil {
return status.Error(codes.NotFound, "pod not created")
}
if containerId == "" {
if containerID == "" {
return status.Error(codes.InvalidArgument, "container ID must be set")
}
if checkProcess && execId == "0" {
if checkProcess && execID == "0" {
return status.Error(codes.InvalidArgument, "process ID must be set")
}

// Check container existence
if createContainer {
if m.pod.containers[containerId] != nil {
return status.Errorf(codes.AlreadyExists, "container ID %s already taken", containerId)
if m.pod.containers[containerID] != nil {
return status.Errorf(codes.AlreadyExists, "container ID %s already taken", containerID)
}
return nil
} else if m.pod.containers[containerId] == nil {
return status.Errorf(codes.NotFound, "container %s does not exist", containerId)
} else if m.pod.containers[containerID] == nil {
return status.Errorf(codes.NotFound, "container %s does not exist", containerID)
}

// Check process existence
if checkProcess {
c := m.pod.containers[containerId]
if c.proc[execId] == nil {
return status.Errorf(codes.NotFound, "process %s does not exist", execId)
c := m.pod.containers[containerID]
if c.proc[execID] == nil {
return status.Errorf(codes.NotFound, "process %s does not exist", execID)
}
}

return nil
}

func (m *mockServer) processExist(containerId string, execId string) error {
return m.checkExist(containerId, execId, false, true)
func (m *mockServer) processExist(containerID string, execID string) error {
return m.checkExist(containerID, execID, false, true)
}

func (m *mockServer) containerExist(containerId string) error {
return m.checkExist(containerId, "0", false, false)
func (m *mockServer) containerExist(containerID string) error {
return m.checkExist(containerID, "0", false, false)
}

func (m *mockServer) containerNonExist(containerId string) error {
return m.checkExist(containerId, "0", true, false)
func (m *mockServer) containerNonExist(containerID string) error {
return m.checkExist(containerID, "0", true, false)
}

func (m *mockServer) podExist() error {
Expand Down

0 comments on commit 5ebcaec

Please sign in to comment.