Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Include computer name in AgentRegistration & decode Instance ID from it #2557

Merged
merged 12 commits into from
Oct 26, 2022

Conversation

Porges
Copy link
Member

@Porges Porges commented Oct 24, 2022

Addressing #2550 but in a slightly different way:

  1. Always include the machine name in the agent registration message. From the machine name we can extract the instance ID.
  2. Use the stored value in preference to fetching it every time.
  3. To back-fill existing nodes that do not have the value stored, update it in AcquireScaleInProtection if it is not already present.

@mgreisen
Copy link
Contributor

@Porges I just remembered the work that @chkeita is doing with unmanaged nodes. Unmanaged nodes are likely to have some other naming scheme. I'm not sure how we will distinguish when registering.

@codecov-commenter
Copy link

codecov-commenter commented Oct 24, 2022

Codecov Report

Merging #2557 (c628f87) into main (0e8876b) will decrease coverage by 5.69%.
The diff coverage is 13.04%.

@@            Coverage Diff             @@
##             main    #2557      +/-   ##
==========================================
- Coverage   35.52%   29.83%   -5.70%     
==========================================
  Files         134      289     +155     
  Lines       17123    35747   +18624     
==========================================
+ Hits         6083    10664    +4581     
- Misses      11040    25083   +14043     
Impacted Files Coverage Δ
...piService/ApiService/Functions/AgentCanSchedule.cs 0.00% <0.00%> (ø)
...c/ApiService/ApiService/TestHooks/VmssTestHooks.cs 0.00% <0.00%> (ø)
...ApiService/ApiService/onefuzzlib/NodeOperations.cs 23.65% <0.00%> (ø)
...ApiService/ApiService/onefuzzlib/VmssOperations.cs 0.00% <0.00%> (ø)
src/agent/onefuzz-agent/src/config.rs 0.00% <0.00%> (ø)
...iService/ApiService/Functions/AgentRegistration.cs 89.71% <66.66%> (ø)
src/ApiService/ApiService/OneFuzzTypes/Model.cs 71.97% <100.00%> (ø)
...rc/ApiService/ApiService/onefuzzlib/InstanceIds.cs 100.00% <100.00%> (ø)
src/agent/input-tester/src/lib.rs 2.94% <0.00%> (-97.06%) ⬇️
src/agent/win-util/src/lib.rs 18.51% <0.00%> (-81.49%) ⬇️
... and 174 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Porges
Copy link
Member Author

Porges commented Oct 24, 2022

@Porges I just remembered the work that @chkeita is doing with unmanaged nodes. Unmanaged nodes are likely to have some other naming scheme. I'm not sure how we will distinguish when registering.

Yes, we will have to have different handling for them. If they send null for this then we would fall back to the existing code path. We are going to have to have different handling for unmanaged nodes in any case, so I don’t think this change will adversely affect that work.

@Porges Porges force-pushed the include-instance-id branch from 9f17cad to e77734a Compare October 25, 2022 00:37
@Porges Porges changed the title Include computer name in AgentRegistration Include computer name in AgentRegistration & decode Instance ID from it Oct 25, 2022
@Porges Porges force-pushed the include-instance-id branch from dc66f18 to e6f0f40 Compare October 25, 2022 01:01
@Porges Porges marked this pull request as ready for review October 25, 2022 20:07
@Porges Porges requested review from stishkin and chkeita October 25, 2022 20:49
@Porges Porges force-pushed the include-instance-id branch from c9d67ae to 487907e Compare October 26, 2022 00:11
@Porges Porges merged commit 81e4b15 into main Oct 26, 2022
@Porges Porges deleted the include-instance-id branch October 26, 2022 21:01
@ghost ghost locked as resolved and limited conversation to collaborators Nov 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants