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

Expand valid scaleset names #3045

Merged
merged 13 commits into from
May 16, 2023
Merged

Expand valid scaleset names #3045

merged 13 commits into from
May 16, 2023

Conversation

Porges
Copy link
Member

@Porges Porges commented Apr 20, 2023

Closes #2189.

Scaleset names are now permitted to be any (valid) strings, instead of only GUIDs. When we generate a scaleset name it is now based upon the pool name; for example the pool pool might get a scaleset named pool-3b24ba211cad4b078655914754485838.

This should be backwards-compatible since GUIDs are already serialized to table storage as strings, so this simply loosens the restrictions placed upon them.

Scaleset IDs now have a strong type in the same way as other IDs; this helps to avoid mixing them up with other strings. Because of this I found one bug in the scaleset search query logic due to Pool ID/VMSS ID confusion. As part of fixing this I've changed the scaleset search query to only return nodes from the table rather than querying Azure to find a list; this seems to be sufficient for the CLI.

@codecov-commenter
Copy link

codecov-commenter commented Apr 20, 2023

Codecov Report

Merging #3045 (e5577d2) into main (44afdb3) will increase coverage by 0.07%.
The diff coverage is 37.81%.

@@            Coverage Diff             @@
##             main    #3045      +/-   ##
==========================================
+ Coverage   31.42%   31.50%   +0.07%     
==========================================
  Files         306      306              
  Lines       36622    36638      +16     
==========================================
+ Hits        11508    11542      +34     
+ Misses      25114    25096      -18     
Impacted Files Coverage Δ
src/ApiService/ApiService/Functions/Proxy.cs 0.00% <ø> (ø)
...ce/ApiService/TestHooks/NodeOperationsTestHooks.cs 0.00% <0.00%> (ø)
...vice/ApiService/TestHooks/ProxyForwardTestHooks.cs 0.00% <0.00%> (ø)
...c/ApiService/ApiService/TestHooks/VmssTestHooks.cs 0.00% <0.00%> (ø)
src/ApiService/ApiService/onefuzzlib/AutoScale.cs 0.00% <0.00%> (ø)
...c/ApiService/ApiService/onefuzzlib/IpOperations.cs 0.00% <0.00%> (ø)
...ce/ApiService/onefuzzlib/ProxyForwardOperations.cs 0.00% <0.00%> (ø)
...rc/ApiService/ApiService/onefuzzlib/ShrinkQueue.cs 0.00% <0.00%> (ø)
...ApiService/ApiService/onefuzzlib/VmssOperations.cs 0.00% <0.00%> (ø)
...ApiService/ApiService/onefuzzlib/NodeOperations.cs 23.78% <23.07%> (ø)
... and 8 more

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

@Porges Porges marked this pull request as ready for review April 20, 2023 21:58
@Porges
Copy link
Member Author

Porges commented Apr 21, 2023

Passed check-pr.

@Porges Porges merged commit 2f478d6 into main May 16, 2023
@Porges Porges deleted the scaleset-names branch May 16, 2023 21:59
Porges added a commit that referenced this pull request May 21, 2023
Scaleset names are now permitted to be any (valid) strings, instead of only GUIDs. When we generate a scaleset name it is now based upon the pool name; for example the pool `pool` might get a scaleset named `pool-3b24ba211cad4b078655914754485838`.

This should be backwards-compatible since GUIDs are [already serialized to table storage as strings](https://github.com/microsoft/onefuzz/blob/dddcfa49490e44834afc9a6910c350698022792c/src/ApiService/ApiService/onefuzzlib/orm/EntityConverter.cs#L190-L191), so this simply loosens the restrictions placed upon them.

Scaleset IDs now have a strong type in the same way as other IDs; this helps to avoid mixing them up with other strings. Because of this I found one bug in the scaleset search query logic due to Pool ID/VMSS ID confusion. As part of fixing this I've changed the scaleset search query to only return nodes from the table rather than querying Azure to find a list; this seems to be sufficient for the CLI.
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.

Changing the way scalesets are named
4 participants