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

fix: cleanup VMSSName + addpool #4067

Merged
merged 3 commits into from
Dec 1, 2020

Conversation

jackfrancis
Copy link
Member

Reason for Change:

This PR cleans up some things from #4051, specifically:

  • Use the proper "next array index" value when calculating the VMSSName property during aks-engine addpool
  • Clarify that the above is not for back-compat: this property computation will always occur during aks-engine addpool
  • improve the clarity of the "generate a VMSS prefix string based on AgentPoolProfile data within the API model" foo
    • and deprecate the unused "Windows name v2" thing

Issue Fixed:

Credit Where Due:

Does this change contain code from or inspired by another project?

  • No
  • Yes

If "Yes," did you notify that project's maintainers and provide attribution?

  • No
  • Yes

Requirements:

Notes:

numExistingPools := len(apc.containerService.Properties.AgentPoolProfiles)
// we can reuse the value of numExistingPools due to array index beginning at "0"
apc.nodePool.VMSSName = apc.containerService.Properties.GetAgentVMPrefix(apc.nodePool, numExistingPools)
if apc.nodePool.VMSSName == "" {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't know what to do if we aren't able to generate a VMSSName, so we'll error out

@codecov
Copy link

codecov bot commented Nov 30, 2020

Codecov Report

Merging #4067 (631d5df) into master (7fdbef0) will increase coverage by 0.00%.
The diff coverage is 54.54%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4067   +/-   ##
=======================================
  Coverage   73.25%   73.26%           
=======================================
  Files         135      135           
  Lines       20542    20539    -3     
=======================================
- Hits        15048    15047    -1     
  Misses       4521     4521           
+ Partials      973      971    -2     
Impacted Files Coverage Δ
cmd/addpool.go 17.97% <0.00%> (-0.11%) ⬇️
pkg/api/types.go 92.56% <100.00%> (+0.41%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7fdbef0...631d5df. Read the comment docs.

Copy link
Member

@mboersma mboersma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@jackfrancis jackfrancis merged commit c3d524d into Azure:master Dec 1, 2020
@jackfrancis jackfrancis deleted the addpool-vmssname branch December 1, 2020 00:38
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.

None yet

2 participants