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 azure cloud provider #454

Merged
merged 7 commits into from
Nov 22, 2017
Merged

Add azure cloud provider #454

merged 7 commits into from
Nov 22, 2017

Conversation

feiskyer
Copy link
Member

@feiskyer feiskyer commented Nov 7, 2017

This is the first step of adding Azure cloud provider back (#449 and kubernetes/kubernetes#47511). Mainly changes includes:

  • bring old azure cloud provider code back
  • update azure sdk to latest stable v11.1.1
  • update azure sdk interfaces
  • add cloud provider interface initial implementations
  • unit tests

Note: Kubernetes Azure cloud provider VMSS support is not finished yet, so this work is tested togather with kubernetes/kubernetes#55833.

And there are more work to do to enable fully azure support, which is tracking at #449.

Closes: #133

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 7, 2017
@feiskyer
Copy link
Member Author

feiskyer commented Nov 7, 2017

@feiskyer
Copy link
Member Author

feiskyer commented Nov 7, 2017

@andyzhangx Any ideas of setting up the test on Azure before VMSS is ready?

return provider
}

func TestBuildAwsCloudProvider(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

Azure, not Aws

}

func TestIncreaseSize(t *testing.T) {

Copy link
Member

Choose a reason for hiding this comment

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

remove empty line

scaleSetCache: make(map[AzureRef]*ScaleSet),
}

//(resourceGroupName string, vmScaleSetName string,
Copy link
Member

Choose a reason for hiding this comment

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

remove comments


func (client *VirtualMachineScaleSetsClientMock) Get(resourceGroupName string,
vmScaleSetName string) (result compute.VirtualMachineScaleSet, err error) {
fmt.Printf("Called VirtualMachineScaleSetsClientMock.Get(%s,%s)\n", resourceGroupName, vmScaleSetName)
Copy link
Member

Choose a reason for hiding this comment

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

fmt.Printf ?

@mwielgus
Copy link
Contributor

mwielgus commented Nov 7, 2017

Cool, thank you for this contribution!
However, we will be able to review and merge it only after it is actually run and tested on Kubernetes.

@mwielgus mwielgus added area/provider/azure Issues or PRs related to azure provider area/cluster-autoscaler labels Nov 7, 2017
@mwielgus
Copy link
Contributor

mwielgus commented Nov 7, 2017

We will also need some documentation explaining how to setup CA on Azure, as a part of this PR.

@feiskyer
Copy link
Member Author

feiskyer commented Nov 8, 2017

@mwielgus yep, I'm validating this on Azure now.

@feiskyer feiskyer changed the title Add azure cloud provider back WIP: Add azure cloud provider Nov 8, 2017
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 8, 2017
@feiskyer feiskyer changed the title WIP: Add azure cloud provider Add azure cloud provider Nov 16, 2017
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 16, 2017
@feiskyer
Copy link
Member Author

Validated scaling up and down on Azure VMSS (with kubernetes/kubernetes#55833), both work as expected now.

@brendandburns @mwielgus @andyzhangx Ready for review now. PTAL.

@feiskyer
Copy link
Member Author

ping @MaciekPytel @mwielgus Could you take a look the PR?

@mwielgus
Copy link
Contributor

/lgtm

@feiskyer, @andyzhangx I assume that you will be the main contacts for all Azure-related issues and PRs.

Neither me nor @MaciekPytel, @aleksandra-malinowska or @bskiba have capacity/capabilities to do Azure-specific reviews, debugging or testing.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 22, 2017
@mwielgus mwielgus merged commit 8196877 into kubernetes:master Nov 22, 2017
@feiskyer
Copy link
Member Author

@mwielgus Thanks. Yep, we will be in charge of all Azure related changes.

@feiskyer feiskyer deleted the azure branch November 23, 2017 01:24
@feiskyer feiskyer mentioned this pull request Nov 23, 2017
8 tasks
yaroslava-serdiuk pushed a commit to yaroslava-serdiuk/autoscaler that referenced this pull request Feb 22, 2024
Refactor: split out flavor assigment logic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cluster-autoscaler area/provider/azure Issues or PRs related to azure provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants