Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add some VM image code #688

Merged
merged 3 commits into from
May 9, 2016
Merged

Add some VM image code #688

merged 3 commits into from
May 9, 2016

Conversation

jianghaolu
Copy link
Contributor

No description provided.

this.version = version;
}

VirtualMachineImageImpl(String publisher, String offer, String sku, String version, VirtualMachineImageInner innerObject) {
Copy link
Member

@anuchandy anuchandy May 9, 2016

Choose a reason for hiding this comment

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

In general, the name of the inner parameter in fluent models is "innerModel".

@jianghaolu
Copy link
Contributor Author

@milismsft Maybe I should remove the version from KnownVirtualMachineImage and fetch the latest every time. But I also don't like doing thing implicitly.

WINDOWS_SERVER_2012_R2_DATACENTER("MicrosoftWindowsServer", "WindowsServer", "2012-R2-Datacenter", "4.0.20160430"),
WINDOWS_SERVER_2016_TECHNICAL_PREVIEW_WITH_CONTAINIERS("MicrosoftWindowsServer", "WindowsServer", "2016-Technical-Preview-with-Containers", "2016.0.20151118"),
WINDOWS_SERVER_TECHNICAL_PREVIEW("MicrosoftWindowsServer", "WindowsServer", "Windows-Server-Technical-Preview", "5.0.20160420");

Copy link
Member

Choose a reason for hiding this comment

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

if we set version to 'latest', then server picks the latest image from the specific sku.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh that's cool. But still I wonder if we should do that... If latest changes in the middle of a user deploying 100 VMs they will actually get different images.

Copy link
Contributor

Choose a reason for hiding this comment

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

Setting the version to "latest" by default is ok as long as the user has an option to overwrite that/specify something different later on.
The scenario I'm thinking is when the user has a web app that he verified/tested against a particular version; he might not want to default "upgrade" to the latest version of the image because that is something he did not tested for and he does not want the risk associated with that.

Copy link
Member

@anuchandy anuchandy May 9, 2016

Choose a reason for hiding this comment

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

Few caveats with KnownVirtualMachineImage enum that we all aware of:

  1. This enum entries are likely to grow as Azure rollout support for new skus ( UBUNTU_16_04_LTS, UBUNTU_16_06_LTS, UBUNTU_17_01_LTS)
  2. If Azure drop support for a sku then we will have to remove it from the enum, that's a breaking change. (WINDOWS_SERVER_TECHNICAL_PREVIEW seems a candidate for this)

But anyway we have an enum for "regions" which also has the same issues, but rollout/removal of new regions is less often compared to skus.

@anuchandy anuchandy merged commit e06fb56 into Azure:master May 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants