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

feat: add UltraSSD support #2905

Merged
merged 2 commits into from
Mar 19, 2020
Merged

feat: add UltraSSD support #2905

merged 2 commits into from
Mar 19, 2020

Conversation

andyzhangx
Copy link
Contributor

@andyzhangx andyzhangx commented Mar 16, 2020

Reason for Change:

This PR adds UltraSSD support for both AgentPool and master node, details about UltraSSD

Issue Fixed:

Requirements:

Notes:

cc @kkmsft

@andyzhangx andyzhangx requested a review from jackfrancis March 16, 2020 02:17
@andyzhangx
Copy link
Contributor Author

/azp run pr-e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@codecov
Copy link

codecov bot commented Mar 16, 2020

Codecov Report

Merging #2905 into master will decrease coverage by 0.01%.
The diff coverage is 43.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2905      +/-   ##
==========================================
- Coverage   72.49%   72.48%   -0.02%     
==========================================
  Files         141      141              
  Lines       25847    25863      +16     
==========================================
+ Hits        18739    18746       +7     
- Misses       6021     6027       +6     
- Partials     1087     1090       +3     
Impacted Files Coverage Δ
pkg/api/types.go 95.63% <ø> (ø)
pkg/api/vlabs/types.go 73.93% <ø> (ø)
pkg/engine/virtualmachinescalesets.go 80.58% <0.00%> (-0.85%) ⬇️
pkg/engine/virtualmachines.go 80.66% <50.00%> (-0.45%) ⬇️
pkg/api/converterfromapi.go 93.78% <100.00%> (+0.02%) ⬆️
pkg/api/convertertoapi.go 93.59% <100.00%> (+0.02%) ⬆️

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 65f7530...674b343. Read the comment docs.

@jackfrancis
Copy link
Member

Thanks @andyzhangx! There appear to be missing implementations for both agentPoolProfile and masterProfile. Here's a recent PR for reference that added a property to both master and agent type definitions:

#2880

We want to make sure we similarly update both configuration objects for this changes as well, on these vectors:

  • api and vlabs type definitions
  • converter "to"
  • converter "from"
  • validation, if appropriate
  • defaults enforcement
  • ARM template composition
  • documentation

@andyzhangx andyzhangx changed the title feat: add UltraSSD support for AgentPool feat: add UltraSSD support Mar 17, 2020
@acs-bot acs-bot added the size/M label Mar 17, 2020
@andyzhangx
Copy link
Contributor Author

andyzhangx commented Mar 17, 2020

  • api and vlabs type definitions
    done
  • converter "to"
    done
  • converter "from"
    done
  • validation, if appropriate
    not necessary since it's a bool value
  • defaults enforcement
    default is false which is compatible
  • ARM template composition
    what is this? I have already run "make build" before commit, no new generated files in my branch.
  • documentation
    done

@jackfrancis PTAL

@andyzhangx
Copy link
Contributor Author

@jackfrancis PTAL

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

Thanks @andyzhangx

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

acs-bot commented Mar 19, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx, jackfrancis

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:

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

@jackfrancis
Copy link
Member

@yangl900 @xuto2 @palma21 FYI UltraSSD support has landed in AKS Engine w/ functional E2E test coverage

@ritazh @craiglpeters FYI

@andyzhangx
Copy link
Contributor Author

@yangl900 @xuto2 @palma21 FYI UltraSSD support has landed in AKS Engine w/ functional E2E test coverage

@ritazh @craiglpeters FYI

thanks for the code review, this feature is required by some customers, next we need to integrate into AKS.

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.

3 participants