Skip to content

Commit 69cfdf4

Browse files
oliveromahonydhurley
authored andcommitted
replace duplicate isContainer call (#596)
* replace duplicate isContainer call and introduce IsContainer mocking for Virtualization test
1 parent 2a918fb commit 69cfdf4

File tree

17 files changed

+167
-29
lines changed

17 files changed

+167
-29
lines changed

sdk/config_helpers.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -856,18 +856,21 @@ func nginxPlusApiCallback(parent *crossplane.Directive, current *crossplane.Dire
856856
}
857857

858858
func pingStubStatusApiEndpoint(statusAPI string) bool {
859-
client := http.Client{Timeout: 50 * time.Millisecond}
859+
client := http.Client{Timeout: 1 * time.Second}
860860
resp, err := client.Get(statusAPI)
861861
if err != nil {
862+
log.Debugf("Unable to create client for Stub Status API request: %v", err)
862863
return false
863864
}
864865

865866
if resp.StatusCode != 200 {
867+
log.Debugf("Stub Status API responded with a %d status code", resp.StatusCode)
866868
return false
867869
}
868870

869871
bodyBytes, err := io.ReadAll(resp.Body)
870872
if err != nil {
873+
log.Debugf("Unable to read Stub Status API response body: %v", err)
871874
return false
872875
}
873876

@@ -882,18 +885,21 @@ func pingStubStatusApiEndpoint(statusAPI string) bool {
882885
}
883886

884887
func pingNginxPlusApiEndpoint(statusAPI string) bool {
885-
client := http.Client{Timeout: 50 * time.Millisecond}
888+
client := http.Client{Timeout: 1 * time.Second}
886889
resp, err := client.Get(statusAPI)
887890
if err != nil {
891+
log.Debugf("Unable to create client for NGINX Plus API request: %v", err)
888892
return false
889893
}
890894

891895
if resp.StatusCode != 200 {
896+
log.Debugf("NGINX Plus API responded with a %d status code", resp.StatusCode)
892897
return false
893898
}
894899

895900
bodyBytes, err := io.ReadAll(resp.Body)
896901
if err != nil {
902+
log.Debugf("Unable to read NGINX Plus API response body: %v", err)
897903
return false
898904
}
899905

src/core/environment.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ type Environment interface {
5959
GetContainerID() (string, error)
6060
GetNetOverflow() (float64, error)
6161
IsContainer() bool
62+
Virtualization() (string, string)
6263
}
6364

6465
type ConfigApplyMarker interface {
@@ -152,7 +153,7 @@ func (env *EnvironmentType) NewHostInfoWithContext(ctx context.Context, agentVer
152153
Uname: getUnixName(),
153154
Partitons: disks,
154155
Network: env.networks(),
155-
Processor: processors(hostInformation.KernelArch),
156+
Processor: env.processors(hostInformation.KernelArch),
156157
Release: releaseInfo("/etc/os-release"),
157158
Tags: *tags,
158159
AgentAccessibleDirs: configDirs,
@@ -729,15 +730,15 @@ func callSyscall(mib []int32) ([]byte, uint64, error) {
729730
return buf, length, nil
730731
}
731732

732-
func processors(architecture string) (res []*proto.CpuInfo) {
733+
func (env *EnvironmentType) processors(architecture string) (res []*proto.CpuInfo) {
733734
log.Debug("Reading CPU information for dataplane host")
734735
cpus, err := cpu.Info()
735736
if err != nil {
736737
log.Warnf("%v", err)
737738
return []*proto.CpuInfo{}
738739
}
739740

740-
hypervisor, virtual := virtualization()
741+
hypervisor, virtual := env.Virtualization()
741742

742743
for _, item := range cpus {
743744
processor := proto.CpuInfo{
@@ -858,7 +859,7 @@ func formatBytes(bytes int) string {
858859
}
859860
}
860861

861-
func virtualization() (string, string) {
862+
func (env *EnvironmentType) Virtualization() (string, string) {
862863
ctx := context.Background()
863864
defer ctx.Done()
864865
// doesn't check k8s
@@ -868,7 +869,7 @@ func virtualization() (string, string) {
868869
return "", "host"
869870
}
870871

871-
if virtualizationSystem == "docker" || isContainer() {
872+
if virtualizationSystem == "docker" || env.IsContainer() {
872873
log.Debugf("Virtualization detected as container with role %v", virtualizationRole)
873874
return "container", virtualizationRole
874875
}

src/core/environment_test.go

+27-6
Original file line numberDiff line numberDiff line change
@@ -440,33 +440,54 @@ func TestNetworks(t *testing.T) {
440440
assert.NotNil(t, networks.Default)
441441
}
442442

443+
// MockEnvironment is a mock implementation of the Environment interface for testing purposes.
444+
type MockEnvironment struct {
445+
isContainer bool
446+
}
447+
448+
// IsContainer is a mock function for IsContainer() method.
449+
func (m *MockEnvironment) IsContainer() bool {
450+
// Mock implementation for testing.
451+
return m.isContainer
452+
}
453+
454+
func (m *MockEnvironment) Virtualization() (string, string) {
455+
realEnv := &EnvironmentType{}
456+
return realEnv.Virtualization()
457+
}
458+
443459
func TestVirtualization(t *testing.T) {
460+
mockEnv := &MockEnvironment{isContainer: false}
461+
444462
// Test normal VM
445-
virtualizationFunc = func(ctx context.Context) (string, string, error) {
463+
virtualizationFunc = func(_ context.Context) (string, string, error) {
446464
return "LXC", "host", nil
447465
}
448466

449-
virtualizationSystem, virtualizationRole := virtualization()
467+
virtualizationSystem, virtualizationRole := mockEnv.Virtualization()
450468

451469
assert.Equal(t, "LXC", virtualizationSystem)
452470
assert.Equal(t, "host", virtualizationRole)
453471

454472
// Test container
455-
virtualizationFunc = func(ctx context.Context) (string, string, error) {
473+
virtualizationFunc = func(_ context.Context) (string, string, error) {
456474
return "docker", "host", nil
457475
}
458476

459-
virtualizationSystem, virtualizationRole = virtualization()
477+
mockEnv.isContainer = true
478+
479+
virtualizationSystem, virtualizationRole = mockEnv.Virtualization()
460480

461481
assert.Equal(t, "container", virtualizationSystem)
462482
assert.Equal(t, "host", virtualizationRole)
463483
}
464484

465485
func TestProcessors(t *testing.T) {
466-
processorInfo := processors("arm64")
486+
env := EnvironmentType{}
487+
processorInfo := env.processors("arm64")
467488
// at least one network interface
468489
assert.GreaterOrEqual(t, processorInfo[0].GetCpus(), int32(1))
469-
// non empty architecture
490+
// non-empty architecture
470491
assert.NotEmpty(t, processorInfo[0].GetArchitecture())
471492
assert.Equal(t, "arm64", processorInfo[0].GetArchitecture())
472493
}

src/core/fake_environment_test.go

+70
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/extensions/advanced-metrics/aggregator/mocks/aggregator_mock.go

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/extensions/advanced-metrics/ingester/mocks/ingester_mock.go

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/extensions/advanced-metrics/reader/mocks/net_mocks.go

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/extensions/advanced-metrics/reader/mocks/reader_mock.go

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/extensions/advanced-metrics/tables/mocks/staging_table_mocks.go

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/integration/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go

+8-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/integration/vendor/github.com/nginx/agent/v2/src/core/environment.go

+6-5
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/integration/vendor/github.com/nginx/agent/v2/test/utils/environment.go

+5
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)