-
Notifications
You must be signed in to change notification settings - Fork 560
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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 |
Codecov Report
@@ Coverage Diff @@
## master #3604 +/- ##
==========================================
+ Coverage 55.32% 55.69% +0.37%
==========================================
Files 107 107
Lines 16173 16209 +36
==========================================
+ Hits 8947 9028 +81
+ Misses 6451 6429 -22
+ Partials 775 752 -23 |
"aci-connector": "virtual-kubelet:latest", | ||
ContainerMonitoringAddonName: "oms:ciprod05082018", | ||
AzureCNINetworkMonitoringAddonName: "networkmonitor:v0.0.4", | ||
"cluster-autoscaler": "cluster-autoscaler:v1.3.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cluster-autoscaler updated to 1.3.1 as per release notes
@feiskyer @andyzhangx FYI Any specific componentry config recommendations we should know about for 1.12 specifically? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a few comments but overall /lgtm
@@ -98,6 +98,13 @@ func setKubeletConfig(cs *api.ContainerService) { | |||
} | |||
} | |||
|
|||
// Get rid of values not supported in v1.12 and up | |||
if common.IsKubernetesVersionGe(o.OrchestratorVersion, "1.12.0-alpha.1") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be 1.12.0-alpha.0
if we want to be rigorous (but otherwise doesn't matter)
echo 'export CLUSTER_DEFINITION=examples/e2e-tests/kubernetes/release/default/definition.json' >> $BASH_ENV | ||
echo 'export CREATE_VNET=true' >> $BASH_ENV | ||
echo 'export ENABLE_KMS_ENCRYPTION=true' >> $BASH_ENV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be enabled for 1.12 as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this diff is hard to read, but yeah, I am moving forward 1.11's test config to 1.12 (which includes KMS encryption)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, I think I got mixed up in the diff
echo 'export CLEANUP_ON_EXIT=${CLEANUP_ON_EXIT}' >> $BASH_ENV | ||
echo 'export CLEANUP_IF_FAIL=false' >> $BASH_ENV | ||
echo 'export RETAIN_SSH=false' >> $BASH_ENV | ||
echo 'export SUBSCRIPTION_ID=${SUBSCRIPTION_ID_E2E_KUBERNETES}' >> $BASH_ENV | ||
echo 'export CLIENT_ID=${SERVICE_PRINCIPAL_CLIENT_ID_E2E_KUBERNETES}' >> $BASH_ENV | ||
echo 'export CLIENT_SECRET=${SERVICE_PRINCIPAL_CLIENT_SECRET_E2E_KUBERNETES}' >> $BASH_ENV | ||
echo 'export CLIENT_OBJECTID=${SERVICE_PRINCIPAL_OBJECT_ID_E2E_KUBERNETES}' >> $BASH_ENV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this needed for encryption?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
# Conflicts: # .circleci/config.yml
No new changes AFAI |
@feiskyer thanks for weighing in! |
What this PR does / why we need it: See:
https://github.com/kubernetes/kubernetes/releases/tag/v1.12.0-alpha.1
Housekeeping: with the introduction of 1.12, we'll deprecate 1.7 E2E test req's from the PR pipeline
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
If applicable:
Release note: