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

fix: scale cannot find VM index for the VMAS+VHD+Linux case #2906

Merged
merged 4 commits into from
Mar 18, 2020

Conversation

jadarsie
Copy link
Member

@jadarsie jadarsie commented Mar 16, 2020

Reason for Change:
Scaling fails if the target pool is a VMAS pool of VHD-based linux nodes.
VM osPublisher is not enought to distinguish between Linux and Windows.

Notes:

@jadarsie jadarsie requested a review from marosset March 16, 2020 16:02
@marosset
Copy link
Contributor

/lgtm
@mboersma or @jackfrancis can you take a look too? - this logic has been fragile

@codecov
Copy link

codecov bot commented Mar 16, 2020

Codecov Report

Merging #2906 into master will increase coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master   #2906      +/-   ##
=========================================
+ Coverage    72.5%   72.5%   +<.01%     
=========================================
  Files         141     141              
  Lines       25744   25743       -1     
=========================================
  Hits        18666   18666              
+ Misses       5998    5997       -1     
  Partials     1080    1080

@jackfrancis
Copy link
Member

@jadarsie Let's ensure we protect any fixes to VMAS + scale by adding "SCALE_CLUSTER": true to this test config:

https://github.com/Azure/aks-engine/blob/master/test/e2e/test_cluster_configs/availabilityset.json#L3

(under the env config object)

@marosset
Copy link
Contributor

Lets add a windows node pool to the ^^^ config

@jadarsie jadarsie requested a review from mboersma March 16, 2020 22:31
Copy link
Member

@jackfrancis jackfrancis left a comment

Choose a reason for hiding this comment

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

/lgtm

@acs-bot acs-bot added the lgtm label Mar 18, 2020
@jackfrancis jackfrancis merged commit 1ae9cca into Azure:master Mar 18, 2020
@acs-bot
Copy link

acs-bot commented Mar 18, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis, jadarsie, marosset

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [jackfrancis,marosset]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jadarsie jadarsie deleted the fix-scale-vmas-vhd-linux branch May 4, 2020 18:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants