Add unit tests for computeagent #1182
Conversation
cmd/ncproxy/ncproxy.go
Outdated
| @@ -84,7 +84,6 @@ func (s *grpcService) AddNIC(ctx context.Context, req *ncproxygrpc.AddNICRequest | |||
| } | |||
| if agent, ok := s.containerIDToComputeAgent.get(req.ContainerID); ok { | |||
| caReq := &computeagent.AddNICInternalRequest{ | |||
| ContainerID: req.ContainerID, | |||
There was a problem hiding this comment.
My ncproxy cache is getting purged, we don't need to pass this along anymore (if we even needed to at all)? I don't believe it was needed but we don't log it just for tracing purposes or anything in the shim?
There was a problem hiding this comment.
I don't know of a specific reason to. If it was for logging, we can just log the compute agent's utility VM ID in compute agent.
There was a problem hiding this comment.
We added an annotation so they could specify a custom "container id" to use to pass during requests https://github.com/microsoft/hcsshim/blob/master/cmd/containerd-shim-runhcs-v1/pod.go#L178-L181, so the UVM ID might not be the same
There was a problem hiding this comment.
Ah, good point. I can add it back just for future tracing needs.
There was a problem hiding this comment.
I still see them getting deleted here, although I see that you added back where they get logged (but they'll just be empty strings as they're not getting passed here)
There was a problem hiding this comment.
Whoops, added back.
katiewasnothere
force-pushed
the
add_compute_agent_unit_tests
branch
from
4a4e5f9 to
4f423e2
Compare
Signed-off-by: Kathryn Baldauf <[email protected]>
katiewasnothere
force-pushed
the
add_compute_agent_unit_tests
branch
from
4f423e2 to
d524163
Compare
|
Besides the ContainerID removal this LGTM, I'll relook on the next push |
|
Should push new commits for these btw so it's easier to see what changed, we can always just squash on check-in |
Signed-off-by: Kathryn Baldauf <[email protected]>
Related work items: microsoft#1067, microsoft#1097, microsoft#1119, microsoft#1170, microsoft#1176, microsoft#1180, microsoft#1181, microsoft#1182, microsoft#1183, microsoft#1184, microsoft#1185, microsoft#1186, microsoft#1187, microsoft#1188, microsoft#1189, microsoft#1191, microsoft#1193, microsoft#1194, microsoft#1195, microsoft#1196, microsoft#1197, microsoft#1200, microsoft#1201, microsoft#1202, microsoft#1203, microsoft#1204, microsoft#1205, microsoft#1206, microsoft#1207, microsoft#1209, microsoft#1210, microsoft#1211, microsoft#1218, microsoft#1219, microsoft#1220, microsoft#1223
…ent_unit_tests Add unit tests for computeagent
This PR adds unit tests for the compute agent portion of the ncproxy flow.
Signed-off-by: Kathryn Baldauf [email protected]