Do not deploy AKS Billing extension for national clouds #3417
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: gsacavdm Assign the PR to them by writing 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 |
Codecov Report
@@ Coverage Diff @@
## master #3417 +/- ##
==========================================
+ Coverage 55.83% 55.84% +0.01%
==========================================
Files 105 105
Lines 15884 15888 +4
==========================================
+ Hits 8869 8873 +4
Misses 6270 6270
Partials 745 745 |
|
Instead of making the extension optional (which might allow users to disable it and have an effect on our billing numbers, would it be possible to disable the extension for non-AzurePublicCloud deployments? We would then be able to re-enable it when the extension is added there. |
|
National clouds aside, is the AKS extension supposed be running on non AKS Kubernetes deployments (aka those deployed directly through acs-engine)? Sounds off... That aside, I can certainly remove the parameter and make it such that it automatically not deploys the extension for sovereign clouds, just need confirmation that's the preferred approach. |
|
@gsacavdm, yes, the extension is deployed on acs-engine deployments as well, see #3360 (comment). @jackfrancis can weigh in when he gets back from vacation next week but I believe the objective is to be able to know how many VMs are deployed through acs-engine, no matter if the customer is using AKS or not. https://github.com/Azure/acs-engine/pull/3417/files#diff-fe31772161e814fe2b4f68df9e62be8cL342 shows two cases, one for acs-engine, one for AKS. |
gsacavdm
changed the title
|
Ok, I rebased my PR and change the code so that UseAksExtension isn't a config rather something that's automatically set based on whether deploying to public Azure or a national cloud. I also updated the PR title & description to align with the update. |
* Add UseAksExtension condition * Add UseAksExtension config * Remove UseAksExtension from autoscaler * Auto disable AKS extension for national clouds
|
Is this merged into acs-engine 19.4, i've tested with this version and still same issue ? if we deploy new cluster and this resource fails is this affecting the k8s cluster installation ? |
|
How did you deploy the new cluster? Can you share the command, api-model and steps you followed? |
|
i've builded a new acs-engine on my own from master branch and its working..., but on acs engine 1.9.4 sources the commit was not merged, i've looked into template_generator.go , and the changes that you made are not included. maybe on the next acs-engine build |
|
This will be included in v0.20.0, currently in testing |
What this PR does / why we need it: This PR prevents acs-engine from including the AKS billing extension when deploying to national clouds given that the extension isn't available in those yet.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #3416Special notes for your reviewer: This is a work-in-progress but want to start getting feedback as early as possible.
If applicable:
Release note: