From 79edf03ae168373c63feb7189f663351674eebda Mon Sep 17 00:00:00 2001 From: oliveromahony Date: Fri, 15 Mar 2024 15:29:12 +0000 Subject: [PATCH] replace duplicate isContainer call (#596) * replace duplicate isContainer call and introduce IsContainer mocking for Virtualization test --- src/core/environment.go | 11 +-- src/core/environment_test.go | 33 +++++++-- src/core/fake_environment_test.go | 70 +++++++++++++++++++ .../aggregator/mocks/aggregator_mock.go | 1 + .../ingester/mocks/ingester_mock.go | 1 + .../reader/mocks/net_mocks.go | 1 + .../reader/mocks/reader_mock.go | 1 + .../tables/mocks/staging_table_mocks.go | 1 + .../nginx/agent/v2/src/core/environment.go | 11 +-- .../nginx/agent/v2/test/utils/environment.go | 5 ++ .../nginx/agent/v2/src/core/environment.go | 11 +-- .../nginx/agent/v2/test/utils/environment.go | 5 ++ test/utils/environment.go | 5 ++ 13 files changed, 135 insertions(+), 21 deletions(-) diff --git a/src/core/environment.go b/src/core/environment.go index 59b2d2f1c..033684d0c 100644 --- a/src/core/environment.go +++ b/src/core/environment.go @@ -59,6 +59,7 @@ type Environment interface { GetContainerID() (string, error) GetNetOverflow() (float64, error) IsContainer() bool + Virtualization() (string, string) } type ConfigApplyMarker interface { @@ -152,7 +153,7 @@ func (env *EnvironmentType) NewHostInfoWithContext(ctx context.Context, agentVer Uname: getUnixName(), Partitons: disks, Network: env.networks(), - Processor: processors(hostInformation.KernelArch), + Processor: env.processors(hostInformation.KernelArch), Release: releaseInfo("/etc/os-release"), Tags: *tags, AgentAccessibleDirs: configDirs, @@ -729,7 +730,7 @@ func callSyscall(mib []int32) ([]byte, uint64, error) { return buf, length, nil } -func processors(architecture string) (res []*proto.CpuInfo) { +func (env *EnvironmentType) processors(architecture string) (res []*proto.CpuInfo) { log.Debug("Reading CPU information for dataplane host") cpus, err := cpu.Info() if err != nil { @@ -737,7 +738,7 @@ func processors(architecture string) (res []*proto.CpuInfo) { return []*proto.CpuInfo{} } - hypervisor, virtual := virtualization() + hypervisor, virtual := env.Virtualization() for _, item := range cpus { processor := proto.CpuInfo{ @@ -858,7 +859,7 @@ func formatBytes(bytes int) string { } } -func virtualization() (string, string) { +func (env *EnvironmentType) Virtualization() (string, string) { ctx := context.Background() defer ctx.Done() // doesn't check k8s @@ -868,7 +869,7 @@ func virtualization() (string, string) { return "", "host" } - if virtualizationSystem == "docker" || isContainer() { + if virtualizationSystem == "docker" || env.IsContainer() { log.Debugf("Virtualization detected as container with role %v", virtualizationRole) return "container", virtualizationRole } diff --git a/src/core/environment_test.go b/src/core/environment_test.go index 1fe40e84f..41a62d0e4 100644 --- a/src/core/environment_test.go +++ b/src/core/environment_test.go @@ -440,33 +440,54 @@ func TestNetworks(t *testing.T) { assert.NotNil(t, networks.Default) } +// MockEnvironment is a mock implementation of the Environment interface for testing purposes. +type MockEnvironment struct { + isContainer bool +} + +// IsContainer is a mock function for IsContainer() method. +func (m *MockEnvironment) IsContainer() bool { + // Mock implementation for testing. + return m.isContainer +} + +func (m *MockEnvironment) Virtualization() (string, string) { + realEnv := &EnvironmentType{} + return realEnv.Virtualization() +} + func TestVirtualization(t *testing.T) { + mockEnv := &MockEnvironment{isContainer: false} + // Test normal VM - virtualizationFunc = func(ctx context.Context) (string, string, error) { + virtualizationFunc = func(_ context.Context) (string, string, error) { return "LXC", "host", nil } - virtualizationSystem, virtualizationRole := virtualization() + virtualizationSystem, virtualizationRole := mockEnv.Virtualization() assert.Equal(t, "LXC", virtualizationSystem) assert.Equal(t, "host", virtualizationRole) // Test container - virtualizationFunc = func(ctx context.Context) (string, string, error) { + virtualizationFunc = func(_ context.Context) (string, string, error) { return "docker", "host", nil } - virtualizationSystem, virtualizationRole = virtualization() + mockEnv.isContainer = true + + virtualizationSystem, virtualizationRole = mockEnv.Virtualization() assert.Equal(t, "container", virtualizationSystem) assert.Equal(t, "host", virtualizationRole) } func TestProcessors(t *testing.T) { - processorInfo := processors("arm64") + env := EnvironmentType{} + processorInfo := env.processors("arm64") // at least one network interface assert.GreaterOrEqual(t, processorInfo[0].GetCpus(), int32(1)) - // non empty architecture + // non-empty architecture assert.NotEmpty(t, processorInfo[0].GetArchitecture()) assert.Equal(t, "arm64", processorInfo[0].GetArchitecture()) } diff --git a/src/core/fake_environment_test.go b/src/core/fake_environment_test.go index 4de09b7a2..793cbc229 100644 --- a/src/core/fake_environment_test.go +++ b/src/core/fake_environment_test.go @@ -163,6 +163,18 @@ type FakeEnvironment struct { result1 []string result2 error } + VirtualizationStub func() (string, string) + virtualizationMutex sync.RWMutex + virtualizationArgsForCall []struct { + } + virtualizationReturns struct { + result1 string + result2 string + } + virtualizationReturnsOnCall map[int]struct { + result1 string + result2 string + } WriteFileStub func(ConfigApplyMarker, *proto.File, string) error writeFileMutex sync.RWMutex writeFileArgsForCall []struct { @@ -949,6 +961,62 @@ func (fake *FakeEnvironment) ReadDirectoryReturnsOnCall(i int, result1 []string, }{result1, result2} } +func (fake *FakeEnvironment) Virtualization() (string, string) { + fake.virtualizationMutex.Lock() + ret, specificReturn := fake.virtualizationReturnsOnCall[len(fake.virtualizationArgsForCall)] + fake.virtualizationArgsForCall = append(fake.virtualizationArgsForCall, struct { + }{}) + stub := fake.VirtualizationStub + fakeReturns := fake.virtualizationReturns + fake.recordInvocation("Virtualization", []interface{}{}) + fake.virtualizationMutex.Unlock() + if stub != nil { + return stub() + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeEnvironment) VirtualizationCallCount() int { + fake.virtualizationMutex.RLock() + defer fake.virtualizationMutex.RUnlock() + return len(fake.virtualizationArgsForCall) +} + +func (fake *FakeEnvironment) VirtualizationCalls(stub func() (string, string)) { + fake.virtualizationMutex.Lock() + defer fake.virtualizationMutex.Unlock() + fake.VirtualizationStub = stub +} + +func (fake *FakeEnvironment) VirtualizationReturns(result1 string, result2 string) { + fake.virtualizationMutex.Lock() + defer fake.virtualizationMutex.Unlock() + fake.VirtualizationStub = nil + fake.virtualizationReturns = struct { + result1 string + result2 string + }{result1, result2} +} + +func (fake *FakeEnvironment) VirtualizationReturnsOnCall(i int, result1 string, result2 string) { + fake.virtualizationMutex.Lock() + defer fake.virtualizationMutex.Unlock() + fake.VirtualizationStub = nil + if fake.virtualizationReturnsOnCall == nil { + fake.virtualizationReturnsOnCall = make(map[int]struct { + result1 string + result2 string + }) + } + fake.virtualizationReturnsOnCall[i] = struct { + result1 string + result2 string + }{result1, result2} +} + func (fake *FakeEnvironment) WriteFile(arg1 ConfigApplyMarker, arg2 *proto.File, arg3 string) error { fake.writeFileMutex.Lock() ret, specificReturn := fake.writeFileReturnsOnCall[len(fake.writeFileArgsForCall)] @@ -1110,6 +1178,8 @@ func (fake *FakeEnvironment) Invocations() map[string][][]interface{} { defer fake.processesMutex.RUnlock() fake.readDirectoryMutex.RLock() defer fake.readDirectoryMutex.RUnlock() + fake.virtualizationMutex.RLock() + defer fake.virtualizationMutex.RUnlock() fake.writeFileMutex.RLock() defer fake.writeFileMutex.RUnlock() fake.writeFilesMutex.RLock() diff --git a/src/extensions/advanced-metrics/aggregator/mocks/aggregator_mock.go b/src/extensions/advanced-metrics/aggregator/mocks/aggregator_mock.go index 1a308788d..71f847985 100644 --- a/src/extensions/advanced-metrics/aggregator/mocks/aggregator_mock.go +++ b/src/extensions/advanced-metrics/aggregator/mocks/aggregator_mock.go @@ -11,6 +11,7 @@ // // mockgen -source aggregator.go -destination mocks/aggregator_mock.go -package mocks -copyright_file=../../../../COPYRIGHT // + // Package mocks is a generated GoMock package. package mocks diff --git a/src/extensions/advanced-metrics/ingester/mocks/ingester_mock.go b/src/extensions/advanced-metrics/ingester/mocks/ingester_mock.go index 936e6dabd..1f187736b 100644 --- a/src/extensions/advanced-metrics/ingester/mocks/ingester_mock.go +++ b/src/extensions/advanced-metrics/ingester/mocks/ingester_mock.go @@ -11,6 +11,7 @@ // // mockgen -source ingester.go -destination mocks/ingester_mock.go -package mocks -copyright_file=../../../../COPYRIGHT // + // Package mocks is a generated GoMock package. package mocks diff --git a/src/extensions/advanced-metrics/reader/mocks/net_mocks.go b/src/extensions/advanced-metrics/reader/mocks/net_mocks.go index 107f129f4..01cfeff47 100644 --- a/src/extensions/advanced-metrics/reader/mocks/net_mocks.go +++ b/src/extensions/advanced-metrics/reader/mocks/net_mocks.go @@ -5,6 +5,7 @@ // // mockgen -destination mocks/net_mocks.go -build_flags=--mod=mod -package mocks net Listener,Conn // + // Package mocks is a generated GoMock package. package mocks diff --git a/src/extensions/advanced-metrics/reader/mocks/reader_mock.go b/src/extensions/advanced-metrics/reader/mocks/reader_mock.go index 771344132..8715808b7 100644 --- a/src/extensions/advanced-metrics/reader/mocks/reader_mock.go +++ b/src/extensions/advanced-metrics/reader/mocks/reader_mock.go @@ -11,6 +11,7 @@ // // mockgen -source reader.go -destination mocks/reader_mock.go -package mocks -copyright_file=../../../../COPYRIGHT // + // Package mocks is a generated GoMock package. package mocks diff --git a/src/extensions/advanced-metrics/tables/mocks/staging_table_mocks.go b/src/extensions/advanced-metrics/tables/mocks/staging_table_mocks.go index 99e34d115..b6794cd0e 100644 --- a/src/extensions/advanced-metrics/tables/mocks/staging_table_mocks.go +++ b/src/extensions/advanced-metrics/tables/mocks/staging_table_mocks.go @@ -11,6 +11,7 @@ // // mockgen -source staging_table.go -destination mocks/staging_table_mocks.go -package mocks -copyright_file=../../../../COPYRIGHT // + // Package mocks is a generated GoMock package. package mocks diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/environment.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/environment.go index 59b2d2f1c..033684d0c 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/environment.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/environment.go @@ -59,6 +59,7 @@ type Environment interface { GetContainerID() (string, error) GetNetOverflow() (float64, error) IsContainer() bool + Virtualization() (string, string) } type ConfigApplyMarker interface { @@ -152,7 +153,7 @@ func (env *EnvironmentType) NewHostInfoWithContext(ctx context.Context, agentVer Uname: getUnixName(), Partitons: disks, Network: env.networks(), - Processor: processors(hostInformation.KernelArch), + Processor: env.processors(hostInformation.KernelArch), Release: releaseInfo("/etc/os-release"), Tags: *tags, AgentAccessibleDirs: configDirs, @@ -729,7 +730,7 @@ func callSyscall(mib []int32) ([]byte, uint64, error) { return buf, length, nil } -func processors(architecture string) (res []*proto.CpuInfo) { +func (env *EnvironmentType) processors(architecture string) (res []*proto.CpuInfo) { log.Debug("Reading CPU information for dataplane host") cpus, err := cpu.Info() if err != nil { @@ -737,7 +738,7 @@ func processors(architecture string) (res []*proto.CpuInfo) { return []*proto.CpuInfo{} } - hypervisor, virtual := virtualization() + hypervisor, virtual := env.Virtualization() for _, item := range cpus { processor := proto.CpuInfo{ @@ -858,7 +859,7 @@ func formatBytes(bytes int) string { } } -func virtualization() (string, string) { +func (env *EnvironmentType) Virtualization() (string, string) { ctx := context.Background() defer ctx.Done() // doesn't check k8s @@ -868,7 +869,7 @@ func virtualization() (string, string) { return "", "host" } - if virtualizationSystem == "docker" || isContainer() { + if virtualizationSystem == "docker" || env.IsContainer() { log.Debugf("Virtualization detected as container with role %v", virtualizationRole) return "container", virtualizationRole } diff --git a/test/integration/vendor/github.com/nginx/agent/v2/test/utils/environment.go b/test/integration/vendor/github.com/nginx/agent/v2/test/utils/environment.go index 2039a2ce2..2615dfff2 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/test/utils/environment.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/test/utils/environment.go @@ -154,3 +154,8 @@ func (m *MockEnvironment) IsContainer() bool { ret := m.Called() return ret.Get(0).(bool) } + +func (m *MockEnvironment) Virtualization() (string, string) { + ret := m.Called() + return ret.Get(0).(string), ret.Get(0).(string) +} diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/environment.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/environment.go index 59b2d2f1c..033684d0c 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/environment.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/environment.go @@ -59,6 +59,7 @@ type Environment interface { GetContainerID() (string, error) GetNetOverflow() (float64, error) IsContainer() bool + Virtualization() (string, string) } type ConfigApplyMarker interface { @@ -152,7 +153,7 @@ func (env *EnvironmentType) NewHostInfoWithContext(ctx context.Context, agentVer Uname: getUnixName(), Partitons: disks, Network: env.networks(), - Processor: processors(hostInformation.KernelArch), + Processor: env.processors(hostInformation.KernelArch), Release: releaseInfo("/etc/os-release"), Tags: *tags, AgentAccessibleDirs: configDirs, @@ -729,7 +730,7 @@ func callSyscall(mib []int32) ([]byte, uint64, error) { return buf, length, nil } -func processors(architecture string) (res []*proto.CpuInfo) { +func (env *EnvironmentType) processors(architecture string) (res []*proto.CpuInfo) { log.Debug("Reading CPU information for dataplane host") cpus, err := cpu.Info() if err != nil { @@ -737,7 +738,7 @@ func processors(architecture string) (res []*proto.CpuInfo) { return []*proto.CpuInfo{} } - hypervisor, virtual := virtualization() + hypervisor, virtual := env.Virtualization() for _, item := range cpus { processor := proto.CpuInfo{ @@ -858,7 +859,7 @@ func formatBytes(bytes int) string { } } -func virtualization() (string, string) { +func (env *EnvironmentType) Virtualization() (string, string) { ctx := context.Background() defer ctx.Done() // doesn't check k8s @@ -868,7 +869,7 @@ func virtualization() (string, string) { return "", "host" } - if virtualizationSystem == "docker" || isContainer() { + if virtualizationSystem == "docker" || env.IsContainer() { log.Debugf("Virtualization detected as container with role %v", virtualizationRole) return "container", virtualizationRole } diff --git a/test/performance/vendor/github.com/nginx/agent/v2/test/utils/environment.go b/test/performance/vendor/github.com/nginx/agent/v2/test/utils/environment.go index 2039a2ce2..2615dfff2 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/test/utils/environment.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/test/utils/environment.go @@ -154,3 +154,8 @@ func (m *MockEnvironment) IsContainer() bool { ret := m.Called() return ret.Get(0).(bool) } + +func (m *MockEnvironment) Virtualization() (string, string) { + ret := m.Called() + return ret.Get(0).(string), ret.Get(0).(string) +} diff --git a/test/utils/environment.go b/test/utils/environment.go index 2039a2ce2..2615dfff2 100644 --- a/test/utils/environment.go +++ b/test/utils/environment.go @@ -154,3 +154,8 @@ func (m *MockEnvironment) IsContainer() bool { ret := m.Called() return ret.Get(0).(bool) } + +func (m *MockEnvironment) Virtualization() (string, string) { + ret := m.Called() + return ret.Get(0).(string), ret.Get(0).(string) +}