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

Updates from aks-engine spike #4302

Merged
merged 18 commits into from
Nov 29, 2018
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ version: 2
defaults: &defaults
working_directory: /go/src/github.com/Azure/acs-engine
docker:
- image: quay.io/deis/go-dev:v1.17.2
- image: quay.io/deis/go-dev:v1.17.3
environment:
GOPATH: /go

Expand All @@ -24,9 +24,6 @@ jobs:
- run:
name: Install dependencies
command: make bootstrap
- run:
Copy link
Member

Choose a reason for hiding this comment

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

This was removed because OpenShift support was taken out of aks-engine, but I think it should stay in acs-engine. (For now--I'm going to remove OpenShift support separately in a later PR.)

Copy link
Member Author

Choose a reason for hiding this comment

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

@CecileRobertMichon thoughts? Because we don't run OpenShift tests anymore this is fine to take out preemptively.

Copy link
Member

Choose a reason for hiding this comment

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

It's ok with me, I'm just complaining that the PR would be simpler without this removal, and that the OpenShift code removal contained herein is incomplete, so if we merge this I'll follow up with another PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe it was removed in aks-engine. I remember doing it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with it either way, I would have preferred removing it with the PR that removes the relevant code but unit tests passed already so this is fine too.

name: Run validation rules
command: make validate-generated
- run:
name: Run linting rules
command: make test-style
Expand Down
10 changes: 5 additions & 5 deletions .prowci/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ prow: prow-config prow-secrets prow-services
.PHONY: prow

prow-config:
kubectl create cm config --from-file=config=config.yaml
kubectl create cm plugins --from-file=plugins=plugins.yaml
kubectl create cm config --from-file=config.yaml=config.yaml
kubectl create cm plugins --from-file=plugins.yaml=plugins.yaml
.PHONY: prow-config

prow-config-update:
kubectl create cm config --from-file=config=config.yaml -o yaml --dry-run | kubectl replace -f -
kubectl create cm plugins --from-file=plugins=plugins.yaml -o yaml --dry-run | kubectl replace -f -
kubectl create cm config --from-file=config.yaml=config.yaml -o yaml --dry-run | kubectl replace -f -
kubectl create cm plugins --from-file=plugins.yaml=plugins.yaml -o yaml --dry-run | kubectl replace -f -
.PHONY: prow-config-update

prow-secrets:
Expand All @@ -23,4 +23,4 @@ prow-services:
kubectl create -f hook.yaml
kubectl create -f tide.yaml
kubectl create -f ingress.yaml
.PHONY: prow-services
.PHONY: prow-services
9 changes: 4 additions & 5 deletions .prowci/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ Prow in [upstream docs][0].

## acs-engine setup

Prow is optimized to run as a Kubernetes application. There are some pre-installation
steps that need to happen in a new Kubernetes cluster before deploying Prow. These
involve setting up an Ingress controller and a mechanism to do TLS. The [Azure docs][1]
explain how to setup Ingress with TLS on top of a Kubernetes cluster in Azure.
Deploy a new Kubernetes cluster (eg. `az aks create -g acse-test-prow-ci -n prow)

Set up an Ingress controller and a mechanism to do TLS. The [Azure docs][1]
explain how to setup Ingress with TLS on top of a Kubernetes cluster in Azure. (make sure you specify `--set rbac.create=true` when creating the ingress controller)

A Github webhook also needs to be setup in the repo that points to `dns-name/hook`.
`dns-name` is the DNS name setup during the DNS configuration of the Ingress controller.
Expand All @@ -35,6 +35,5 @@ appropriately on Github. `deck` is installed as the Prow frontend. Last, `tide`
is also installed that takes care of merging pull requests that pass all tests
and satisfy a set of label requirements.


[0]: https://github.com/kubernetes/test-infra/tree/master/prow#prow
[1]: https://docs.microsoft.com/en-us/azure/aks/ingress
2 changes: 1 addition & 1 deletion .prowci/config.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
log_level: debug

tide:
# target_url: http://ci-bot-aks-ingress.eastus.cloudapp.azure.com/tide.html
# target_url: http://prow-ci-bot-ingress.eastus.cloudapp.azure.com/tide.html
Copy link
Member

Choose a reason for hiding this comment

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

Is the new prow CI bot ready for acs-engine (not just aks-engine)?

Copy link
Contributor

Choose a reason for hiding this comment

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

no this bot runs for aks-engine, not acs-engine

Copy link
Contributor

Choose a reason for hiding this comment

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

I fixed the setup to make it work automatically so we do want these changes though

merge_method:
Azure/acs-engine: squash
queries:
Expand Down
2 changes: 1 addition & 1 deletion .prowci/hook.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ items:
spec:
containers:
- name: hook
image: quay.io/kargakis/hook:workaround
image: registry.svc.ci.openshift.org/ci/hook:latest
imagePullPolicy: IfNotPresent
args:
- --dry-run=false
Expand Down
4 changes: 2 additions & 2 deletions .prowci/ingress.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ spec:
tls:
- secretName: prow-tls
hosts:
- ci-bot-aks-ingress.eastus.cloudapp.azure.com
- prow-ci-bot-ingress.eastus.cloudapp.azure.com
rules:
- host: ci-bot-aks-ingress.eastus.cloudapp.azure.com
- host: prow-ci-bot-ingress.eastus.cloudapp.azure.com
http:
paths:
- path: /*
Expand Down
2 changes: 1 addition & 1 deletion .prowci/tide.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ items:
serviceAccountName: tide
containers:
- name: tide
image: quay.io/kargakis/tide:workaround
image: registry.svc.ci.openshift.org/ci/tide:latest
imagePullPolicy: IfNotPresent
args:
- --dry-run=false
Expand Down
2 changes: 1 addition & 1 deletion .vsts-ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ trigger: none
phases:
- phase: build_vhd
queue:
name: Hosted Linux Preview
name: Hosted Ubuntu 1604
timeoutInMinutes: 120
steps:
- script: |
Expand Down
8 changes: 2 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ DIST_DIRS = find * -type d -exec

.NOTPARALLEL:

.PHONY: bootstrap build test test_fmt validate-generated fmt lint ci devenv
.PHONY: bootstrap build test test_fmt validate-headers fmt lint ci devenv
jackfrancis marked this conversation as resolved.
Show resolved Hide resolved

ifdef DEBUG
GOFLAGS := -gcflags="-N -l"
Expand All @@ -25,7 +25,7 @@ GITTAG := $(VERSION_SHORT)
endif

REPO_PATH := github.com/Azure/acs-engine
DEV_ENV_IMAGE := quay.io/deis/go-dev:v1.17.2
DEV_ENV_IMAGE := quay.io/deis/go-dev:v1.17.3
DEV_ENV_WORK_DIR := /go/src/${REPO_PATH}
DEV_ENV_OPTS := --rm -v ${CURDIR}:${DEV_ENV_WORK_DIR} -w ${DEV_ENV_WORK_DIR} ${DEV_ENV_VARS}
DEV_ENV_CMD := docker run ${DEV_ENV_OPTS} ${DEV_ENV_IMAGE}
Expand All @@ -44,10 +44,6 @@ all: build
dev:
$(DEV_ENV_CMD_IT) bash

.PHONY: validate-generated
Copy link
Member

Choose a reason for hiding this comment

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

I think this should stay in acs-engine until OpenShift support is removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Again, because OpenShift isn't active, this build task is already obsolete.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok to remove

validate-generated: bootstrap
./scripts/validate-generated.sh

.PHONY: validate-dependencies
validate-dependencies: bootstrap
./scripts/validate-dependencies.sh
Expand Down
12 changes: 9 additions & 3 deletions cmd/scale.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const (
scaleName = "scale"
scaleShortDescription = "Scale an existing Kubernetes or OpenShift cluster"
scaleLongDescription = "Scale an existing Kubernetes or OpenShift cluster by specifying increasing or decreasing the node count of an agentpool"
apiModelFilename = "apimodel.json"
)

// NewScaleCmd run a command to upgrade a Kubernetes cluster
Expand Down Expand Up @@ -137,7 +138,7 @@ func (sc *scaleCmd) load(cmd *cobra.Command) error {
}

// load apimodel from the deployment directory
sc.apiModelPath = path.Join(sc.deploymentDirectory, "apimodel.json")
sc.apiModelPath = path.Join(sc.deploymentDirectory, apiModelFilename)

if _, err = os.Stat(sc.apiModelPath); os.IsNotExist(err) {
return errors.Errorf("specified api model does not exist (%s)", sc.apiModelPath)
Expand Down Expand Up @@ -308,7 +309,7 @@ func (sc *scaleCmd) run(cmd *cobra.Command, args []string) error {
return err
}

return nil
return sc.saveAPIModel()
}
} else {
for vmssListPage, err := sc.client.ListVirtualMachineScaleSets(ctx, sc.resourceGroupName); vmssListPage.NotDone(); vmssListPage.Next() {
Expand Down Expand Up @@ -423,6 +424,11 @@ func (sc *scaleCmd) run(cmd *cobra.Command, args []string) error {
return err
}

return sc.saveAPIModel()
}

func (sc *scaleCmd) saveAPIModel() error {
var err error
apiloader := &api.Apiloader{
Translator: &i18n.Translator{
Locale: sc.locale,
Expand All @@ -447,7 +453,7 @@ func (sc *scaleCmd) run(cmd *cobra.Command, args []string) error {
},
}

return f.SaveFile(sc.deploymentDirectory, "apimodel.json", b)
return f.SaveFile(sc.deploymentDirectory, apiModelFilename, b)
}

func (sc *scaleCmd) vmInAgentPool(vmName string, tags map[string]*string) bool {
Expand Down
2 changes: 1 addition & 1 deletion makedev.ps1
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
$REPO_PATH = "github.com/Azure/acs-engine"
$DEV_ENV_IMAGE = "quay.io/deis/go-dev:v1.17.2"
$DEV_ENV_IMAGE = "quay.io/deis/go-dev:v1.17.3"
$DEV_ENV_WORK_DIR = "/go/src/$REPO_PATH"

docker.exe run -it --rm -w $DEV_ENV_WORK_DIR -v `"$($PWD)`":$DEV_ENV_WORK_DIR $DEV_ENV_IMAGE bash
5 changes: 3 additions & 2 deletions packer/install-dependencies.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ installEtcd
installDeps

if [[ ${FEATURE_FLAGS} == *"docker-engine"* ]]; then
DOCKER_ENGINE_REPO="https://apt.dockerproject.org/repo"
installDockerEngine
installGPUDrivers
else
Expand Down Expand Up @@ -90,7 +91,7 @@ for TILLER_VERSION in ${TILLER_VERSIONS}; do
pullContainerImage "docker" "gcr.io/kubernetes-helm/tiller:v${TILLER_VERSION}"
done

CLUSTER_AUTOSCALER_VERSIONS="1.3.3 1.3.1 1.3.0 1.2.2 1.1.2"
CLUSTER_AUTOSCALER_VERSIONS="1.3.4 1.3.3 1.3.1 1.3.0 1.2.2 1.1.2"
for CLUSTER_AUTOSCALER_VERSION in ${CLUSTER_AUTOSCALER_VERSIONS}; do
pullContainerImage "docker" "k8s.gcr.io/cluster-autoscaler:v${CLUSTER_AUTOSCALER_VERSION}"
done
Expand Down Expand Up @@ -153,7 +154,7 @@ done
pullContainerImage "docker" "busybox"

# TODO: fetch supported k8s versions from an acs-engine command instead of hardcoding them here
K8S_VERSIONS="1.7.15 1.7.16 1.8.14 1.8.15 1.9.10 1.9.11 1.10.8 1.10.9 1.11.3 1.11.4 1.12.1 1.12.2"
K8S_VERSIONS="1.7.15 1.7.16 1.8.14 1.8.15 1.9.10 1.9.11 1.10.8 1.10.9 1.11.4 1.11.5 1.12.1 1.12.2"

for KUBERNETES_VERSION in ${K8S_VERSIONS}; do
HYPERKUBE_URL="k8s.gcr.io/hyperkube-amd64:v${KUBERNETES_VERSION}"
Expand Down
10 changes: 7 additions & 3 deletions parts/k8s/containeraddons/ip-masq-agent.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ metadata:
data:
ip-masq-agent: |-
nonMasqueradeCIDRs:
- <nonmasqCIDR>
- <nonmasqCNIIP>
masqLinkLocal: <masqLink>
- {{ContainerConfig "non-masquerade-cidr"}}
{{- if ContainerConfig "non-masq-cni-cidr"}}
- {{ContainerConfig "non-masq-cni-cidr"}}
masqLinkLocal: true
{{else -}}
masqLinkLocal: false
{{end -}}
resyncInterval: 60s
8 changes: 0 additions & 8 deletions parts/k8s/kubernetesmastercustomdata.yml
Original file line number Diff line number Diff line change
Expand Up @@ -350,14 +350,6 @@ MASTER_ARTIFACTS_CONFIG_PLACEHOLDER
{{if HasCustomSearchDomain}}
sed -i "s|<searchDomainName>|{{WrapAsParameter "searchDomainName"}}|g; s|<searchDomainRealmUser>|{{WrapAsParameter "searchDomainRealmUser"}}|g; s|<searchDomainRealmPassword>|{{WrapAsParameter "searchDomainRealmPassword"}}|g" /opt/azure/containers/setup-custom-search-domains.sh
{{end}}
a=/etc/kubernetes/addons/ip-masq-agent.yaml
sed -i "s|<nonmasqCIDR>|{{WrapAsParameter "kubernetesNonMasqueradeCidr"}}|g" $a
{{if IsAzureCNI}}
sed -i "s|<nonmasqCNIIP>|168.63.129.16/32|g; s|<masqLink>|true|g" $a
{{else}}
sed -i "\|<nonmasqCNIIP>|d" $a
sed -i "s|<masqLink>|false|g" $a
{{end}}

- path: /opt/azure/containers/mountetcd.sh
permissions: "0744"
Expand Down
9 changes: 0 additions & 9 deletions parts/k8s/kubernetesparams.t
Original file line number Diff line number Diff line change
Expand Up @@ -195,15 +195,6 @@
},
"type": "string"
},
{{if not IsHostedMaster}}
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this section need to stay here to support ACS use cases?

Copy link
Member Author

Choose a reason for hiding this comment

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

This removal is part of the addons refactor. @tariq1890 can verify

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. I can confirm

"kubernetesNonMasqueradeCidr": {
"metadata": {
"description": "kubernetesNonMasqueradeCidr cluster subnet"
},
"defaultValue": "{{GetDefaultVNETCIDR}}",
"type": "string"
},
{{end}}
"kubernetesKubeletClusterDomain": {
"metadata": {
"description": "--cluster-domain Kubelet config"
Expand Down
11 changes: 0 additions & 11 deletions pkg/acsengine/params_k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,17 +96,6 @@ func assignKubernetesParameters(properties *api.Properties, parametersMap params
CloudProviderRateLimitBucket: kubernetesConfig.CloudProviderRateLimitBucket,
})
addValue(parametersMap, "kubeClusterCidr", kubernetesConfig.ClusterSubnet)
if !properties.IsHostedMasterProfile() {
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above.

Copy link
Member Author

Choose a reason for hiding this comment

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

also same as above w/ respect to addons refactor

Copy link
Contributor

Choose a reason for hiding this comment

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

Confirmed

if properties.OrchestratorProfile.IsAzureCNI() {
if properties.MasterProfile != nil && properties.MasterProfile.IsCustomVNET() {
addValue(parametersMap, "kubernetesNonMasqueradeCidr", properties.MasterProfile.VnetCidr)
} else {
addValue(parametersMap, "kubernetesNonMasqueradeCidr", DefaultVNETCIDR)
}
} else {
addValue(parametersMap, "kubernetesNonMasqueradeCidr", properties.OrchestratorProfile.KubernetesConfig.ClusterSubnet)
}
}
addValue(parametersMap, "kubernetesKubeletClusterDomain", kubernetesConfig.KubeletConfig["--cluster-domain"])
addValue(parametersMap, "dockerBridgeCidr", kubernetesConfig.DockerBridgeSubnet)
addValue(parametersMap, "networkPolicy", kubernetesConfig.NetworkPolicy)
Expand Down
7 changes: 2 additions & 5 deletions pkg/acsengine/template_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -534,16 +534,13 @@ func (t *TemplateGenerator) getTemplateFuncMap(cs *api.ContainerService) templat
if cs.Properties.OrchestratorProfile.OrchestratorType == api.DCOS {
return helpers.GetDCOSMasterAllowedSizes()
}
return helpers.GetMasterAgentAllowedSizes()
return helpers.GetKubernetesAllowedSizes()
},
"GetDefaultVNETCIDR": func() string {
return DefaultVNETCIDR
},
"GetAgentAllowedSizes": func() string {
if cs.Properties.OrchestratorProfile.IsKubernetes() || cs.Properties.OrchestratorProfile.IsOpenShift() {
Copy link
Member

Choose a reason for hiding this comment

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

I think this change is safe to merge, but ideally it would be part of a separate "remove OpenShift" PR.

return helpers.GetKubernetesAgentAllowedSizes()
}
return helpers.GetMasterAgentAllowedSizes()
return helpers.GetKubernetesAllowedSizes()
},
"getSwarmVersions": func() string {
return getSwarmVersions(api.SwarmVersion, api.SwarmDockerComposeVersion)
Expand Down
4 changes: 4 additions & 0 deletions pkg/api/addons.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,10 @@ func (cs *ContainerService) setAddonsConfig(isUpdate bool) {
Image: specConfig.KubernetesImageBase + "ip-masq-agent-amd64:v2.0.0",
},
},
Config: map[string]string{
"non-masquerade-cidr": cs.Properties.GetNonMasqueradeCIDR(),
"non-masq-cni-cidr": cs.Properties.GetAzureCNICidr(),
},
}

defaultAzureCNINetworkMonitorAddonsConfig := KubernetesAddon{
Expand Down
3 changes: 2 additions & 1 deletion pkg/api/common/versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,9 @@ var AllKubernetesSupportedVersions = map[string]bool{
"1.11.0": false,
"1.11.1": false,
"1.11.2": false,
"1.11.3": true,
"1.11.3": false,
"1.11.4": true,
"1.11.5": true,
"1.12.0-alpha.1": false,
"1.12.0-beta.0": false,
"1.12.0-beta.1": false,
Expand Down
4 changes: 4 additions & 0 deletions pkg/api/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ const (
DefaultFirstConsecutiveKubernetesStaticIP = "10.240.255.5"
// DefaultFirstConsecutiveKubernetesStaticIPVMSS specifies the static IP address on Kubernetes master 0 of VMSS
DefaultFirstConsecutiveKubernetesStaticIPVMSS = "10.240.0.4"
//DefaultCNICIDR specifies the default value for
DefaultCNICIDR = "168.63.129.16/32"
// DefaultKubernetesFirstConsecutiveStaticIPOffset specifies the IP address offset of master 0
// when VNET integration is enabled.
DefaultKubernetesFirstConsecutiveStaticIPOffset = 5
Expand Down Expand Up @@ -194,6 +196,8 @@ const (
ARMVirtualNetworksResourceType = "virtualNetworks"
// DefaultAcceleratedNetworkingWindowsEnabled determines the acs-engine provided default for enabling accelerated networking on Windows nodes
DefaultAcceleratedNetworkingWindowsEnabled = false
// DefaultAcceleratedNetworking determines the acs-engine provided default for enabling accelerated networking on Linux nodes
DefaultAcceleratedNetworking = true
// DefaultDNSAutoscalerAddonName is the name of the dns-autoscaler addon
DefaultDNSAutoscalerAddonName = "dns-autoscaler"
)
Expand Down
4 changes: 2 additions & 2 deletions pkg/api/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,11 +431,11 @@ func (p *Properties) setAgentProfileDefaults(isUpgrade, isScale bool) {
// On instances that support hyperthreading, Accelerated Networking is supported on VM instances with 4 or more vCPUs.
// Supported series are: D/DSv3, E/ESv3, Fsv2, and Ms/Mms.
if profile.AcceleratedNetworkingEnabled == nil {
profile.AcceleratedNetworkingEnabled = helpers.PointerToBool(!isUpgrade && !isScale && helpers.AcceleratedNetworkingSupported(profile.VMSize))
profile.AcceleratedNetworkingEnabled = helpers.PointerToBool(DefaultAcceleratedNetworking && !isUpgrade && !isScale && helpers.AcceleratedNetworkingSupported(profile.VMSize))
}

if profile.AcceleratedNetworkingEnabledWindows == nil {
profile.AcceleratedNetworkingEnabledWindows = helpers.PointerToBool(DefaultAcceleratedNetworkingWindowsEnabled)
profile.AcceleratedNetworkingEnabledWindows = helpers.PointerToBool(DefaultAcceleratedNetworkingWindowsEnabled && !isUpgrade && !isScale && helpers.AcceleratedNetworkingSupported(profile.VMSize))
}

if profile.OSType != Windows {
Expand Down
Loading