Skip to content

Commit 734c09a

Browse files
committed
PR feedback
1 parent 849df18 commit 734c09a

File tree

2 files changed

+130
-4
lines changed

2 files changed

+130
-4
lines changed

internal/nginx/nginx_service.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func (n *NginxService) Instance(instanceID string) *mpi.Instance {
126126
}
127127

128128
func (n *NginxService) UpdateResource(ctx context.Context, resource *mpi.Resource) *mpi.Resource {
129-
slog.DebugContext(ctx, "Updating resource")
129+
slog.DebugContext(ctx, "Updating resource", "resource", resource)
130130
n.resourceMutex.Lock()
131131
defer n.resourceMutex.Unlock()
132132

internal/watcher/health/health_watcher_service_test.go

Lines changed: 129 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,13 @@
66
package health
77

88
import (
9+
"errors"
910
"fmt"
11+
"reflect"
1012
"testing"
1113

1214
mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
15+
"github.com/nginx/agent/v3/internal/watcher/health/healthfakes"
1316
"github.com/nginx/agent/v3/test/protos"
1417
"github.com/nginx/agent/v3/test/types"
1518
"github.com/stretchr/testify/assert"
@@ -59,8 +62,131 @@ func TestHealthWatcherService_UpdateHealthWatcher(t *testing.T) {
5962
assert.Equal(t, updatedInstance, healthWatcher.instances[instance.GetInstanceMeta().GetInstanceId()])
6063
}
6164

62-
// commented out to push branch
63-
// func TestHealthWatcherService_health(t *testing.T) {
65+
func TestHealthWatcherService_health(t *testing.T) {
66+
ossInstance := protos.NginxOssInstance([]string{})
67+
plusInstance := protos.NginxPlusInstance([]string{})
68+
unspecifiedInstance := protos.UnsupportedInstance()
69+
70+
tests := []struct {
71+
name string
72+
instances map[string]*mpi.Instance
73+
cache map[string]*mpi.InstanceHealth
74+
isHealthDiff bool
75+
updatedInstances []*mpi.InstanceHealth
76+
}{
77+
{
78+
name: "Test 1: NGINX Instance Status Changed",
79+
instances: map[string]*mpi.Instance{
80+
ossInstance.GetInstanceMeta().GetInstanceId(): ossInstance,
81+
plusInstance.GetInstanceMeta().GetInstanceId(): plusInstance,
82+
unspecifiedInstance.GetInstanceMeta().GetInstanceId(): unspecifiedInstance,
83+
},
84+
cache: map[string]*mpi.InstanceHealth{
85+
ossInstance.GetInstanceMeta().GetInstanceId(): protos.HealthyInstanceHealth(),
86+
plusInstance.GetInstanceMeta().GetInstanceId(): {
87+
InstanceId: plusInstance.GetInstanceMeta().GetInstanceId(),
88+
InstanceHealthStatus: mpi.InstanceHealth_INSTANCE_HEALTH_STATUS_HEALTHY,
89+
},
90+
unspecifiedInstance.GetInstanceMeta().GetInstanceId(): protos.UnspecifiedInstanceHealth(),
91+
},
92+
isHealthDiff: true,
93+
updatedInstances: []*mpi.InstanceHealth{
94+
protos.HealthyInstanceHealth(),
95+
{
96+
InstanceId: plusInstance.GetInstanceMeta().GetInstanceId(),
97+
InstanceHealthStatus: mpi.InstanceHealth_INSTANCE_HEALTH_STATUS_UNHEALTHY,
98+
},
99+
protos.UnspecifiedInstanceHealth(),
100+
},
101+
},
102+
{
103+
name: "Test 2: NGINX Instance No Status Changed",
104+
instances: map[string]*mpi.Instance{
105+
ossInstance.GetInstanceMeta().GetInstanceId(): ossInstance,
106+
plusInstance.GetInstanceMeta().GetInstanceId(): plusInstance,
107+
unspecifiedInstance.GetInstanceMeta().GetInstanceId(): unspecifiedInstance,
108+
},
109+
cache: map[string]*mpi.InstanceHealth{
110+
ossInstance.GetInstanceMeta().GetInstanceId(): protos.HealthyInstanceHealth(),
111+
plusInstance.GetInstanceMeta().GetInstanceId(): protos.UnhealthyInstanceHealth(),
112+
unspecifiedInstance.GetInstanceMeta().GetInstanceId(): protos.UnspecifiedInstanceHealth(),
113+
},
114+
isHealthDiff: false,
115+
updatedInstances: []*mpi.InstanceHealth{
116+
protos.HealthyInstanceHealth(),
117+
{
118+
InstanceId: plusInstance.GetInstanceMeta().GetInstanceId(),
119+
InstanceHealthStatus: mpi.InstanceHealth_INSTANCE_HEALTH_STATUS_UNHEALTHY,
120+
},
121+
protos.UnspecifiedInstanceHealth(),
122+
},
123+
},
124+
{
125+
name: "Test 3: Deleted NGINX Instances ",
126+
instances: map[string]*mpi.Instance{
127+
ossInstance.GetInstanceMeta().GetInstanceId(): ossInstance,
128+
unspecifiedInstance.GetInstanceMeta().GetInstanceId(): unspecifiedInstance,
129+
},
130+
cache: map[string]*mpi.InstanceHealth{
131+
ossInstance.GetInstanceMeta().GetInstanceId(): protos.HealthyInstanceHealth(),
132+
plusInstance.GetInstanceMeta().GetInstanceId(): protos.UnhealthyInstanceHealth(),
133+
unspecifiedInstance.GetInstanceMeta().GetInstanceId(): protos.UnspecifiedInstanceHealth(),
134+
},
135+
isHealthDiff: true,
136+
updatedInstances: []*mpi.InstanceHealth{
137+
protos.HealthyInstanceHealth(),
138+
{
139+
InstanceId: plusInstance.GetInstanceMeta().GetInstanceId(),
140+
InstanceHealthStatus: mpi.InstanceHealth_INSTANCE_HEALTH_STATUS_UNHEALTHY,
141+
},
142+
protos.UnspecifiedInstanceHealth(),
143+
},
144+
},
145+
{
146+
name: "Test 4: Added NGINX Instances ",
147+
instances: map[string]*mpi.Instance{
148+
ossInstance.GetInstanceMeta().GetInstanceId(): ossInstance,
149+
plusInstance.GetInstanceMeta().GetInstanceId(): plusInstance,
150+
unspecifiedInstance.GetInstanceMeta().GetInstanceId(): unspecifiedInstance,
151+
},
152+
cache: map[string]*mpi.InstanceHealth{
153+
ossInstance.GetInstanceMeta().GetInstanceId(): protos.HealthyInstanceHealth(),
154+
unspecifiedInstance.GetInstanceMeta().GetInstanceId(): protos.UnspecifiedInstanceHealth(),
155+
},
156+
isHealthDiff: true,
157+
updatedInstances: []*mpi.InstanceHealth{
158+
protos.HealthyInstanceHealth(),
159+
{
160+
InstanceId: plusInstance.GetInstanceMeta().GetInstanceId(),
161+
InstanceHealthStatus: mpi.InstanceHealth_INSTANCE_HEALTH_STATUS_UNHEALTHY,
162+
},
163+
protos.UnspecifiedInstanceHealth(),
164+
},
165+
},
166+
}
167+
168+
for _, test := range tests {
169+
t.Run(test.name, func(t *testing.T) {
170+
agentConfig := types.AgentConfig()
171+
healthWatcher := NewHealthWatcherService(agentConfig)
172+
fakeHealthWatcher := healthfakes.FakeHealthWatcherOperator{}
173+
174+
fakeHealthWatcher.HealthReturnsOnCall(0, protos.HealthyInstanceHealth(), nil)
175+
fakeHealthWatcher.HealthReturnsOnCall(1, protos.UnhealthyInstanceHealth(), nil)
176+
fakeHealthWatcher.HealthReturnsOnCall(2, nil, errors.New("unable to determine health"))
177+
178+
healthWatcher.instances = test.instances
179+
healthWatcher.updateCache(test.cache)
180+
healthWatcher.watcher = &fakeHealthWatcher
181+
updatedStatus, isHealthDiff := healthWatcher.health(t.Context())
182+
assert.Equal(t, test.isHealthDiff, isHealthDiff)
183+
184+
reflect.DeepEqual(test.updatedInstances, updatedStatus)
185+
})
186+
}
187+
}
188+
189+
//func TestHealthWatcherService_health2(t *testing.T) {
64190
// ctx := context.Background()
65191
// agentConfig := types.AgentConfig()
66192
// healthWatcher := NewHealthWatcherService(agentConfig)
@@ -139,7 +265,7 @@ func TestHealthWatcherService_UpdateHealthWatcher(t *testing.T) {
139265
// reflect.DeepEqual(instanceHealth, expected)
140266
// })
141267
// }
142-
// }
268+
//}
143269

144270
func TestHealthWatcherService_compareCache(t *testing.T) {
145271
ossInstance := protos.NginxOssInstance([]string{})

0 commit comments

Comments
 (0)