AWS China region support #390

camilb · 2017-03-07T13:39:33Z

Hi @mumoshu, my colleague @mpucholblasco and I were working to adapt kube-aws for AWS China region and we need community feedback.

There were several problems we encountered and tried to fix:

there is no KMS support in cn-north-1region. We preferred to disable this entirely for this region only.We think later using https://github.com/mozilla/sops to encrypt the certificates fields.
China region has a different S3 endpoint. For now we are passing a flag "--s3-region=cn-north-1" when starting the cluster but we are working to get the region automatically from cluster.yaml
there are different ARNs. We automatically detect and change them for cn-north-1
most of the docker or rkt images are not pulled from dockerhub or quay and gcr.io is completely blocked in China. On large images, like hyperkube, you get many timeouts from quay.
We added an option to specify your own images, hosted on a registry in China.
We are using Alibaba Cloud registry, which is very fast. We also added an option for rkt to pull images from docker repos.
A special case is with amd64-pause image that kubernetes is using it and can not change its name or download it. In this case we added a simple service that pulls the image from our registry in China and tag it as gcr.io/google_containers/pause-amd64:3.0
CoreOS AMIs in cn-north-1 are outdated, so we're using a simple method to clone a AMI from Singapore region.
#ap-south-1
mkfs.ext4 /dev/xvdb
mkdir /ami
mount /dev/xvdb /ami
cd /ami
dd if=/dev/xvda of=root.img bs=1M
scp -i core.pem /ami/root.img core@cn-north-1_ip_address:/home/core
#cn-north-1
dd if=/home/core/root.img of=/dev/xvdb bs=1M oflag=direct
mkdir -p /ami
mount /dev/xvdb9 /ami
rm /ami/home/core/.ssh/authorized_keys
rm /ami/home/core/.ssh/authorized_keys.d/*
umount /ami
Create a snapshot of the volume.
Create an AMI from the snapshot.

After all of these changes we can successfully deploy kubernetes with kube-aws in cn-north-1 as fast as for other regions and without errors or additional configurations.

We are waiting for suggestions from the community before clean the commits and rebase.

* coreos/master: Emit a warning message when `t2.nano` or `t2.micro` is set for `*instanceType` closes #258 Drop deprecated `hostedZone`(not `hostedZoneId`) in cluster.yaml closes #287 fix(e2e): The second and the subsequent testinfra stacks fail to be created closes #367 fix(e2e): node pool "Fleet2" fails to be created closes #362 feat: Managed HA etcd cluster Add additional EBS volumes to worker nodes. Update ROADMAP.md Fix customSettings unavailability for node pools Make tests which depends on `clusterName` to pass on Travis Fix the inconsistent S3 object prefix issue Add a test to reproduce the inconsistent S3 object prefix issue Fix missing Name and ControllerHost in `status` and `up` commands Add support for ALBs Target Groups Fix typo in provisioner.go Fix typo in comment credentials: Add /O=systems:master for kube-admin Add experimental support for pod security policies README: remove the design decisions README: make the initial instructions self-contained README: remove the survey

Option for rkt to pull from Docker repos.

mumoshu · 2017-03-08T00:11:10Z

cfnstack/assets.go

+		url = fmt.Sprintf("https://s3.amazonaws.com/%s/%s", l.Bucket, l.Key)
+	}
+	log.Printf("Using S3 URL: %s on region: %s", url, l.S3Region)
+	return url


As we have the Region type in model/region.go today, perhaps we'd better implement func (r Region) S3EndpointName() string to return e.g. "s3.cn-north-1.amazonaws.com.cn" for cn-north-1 or "s3.amazonaws.com" otherwise, to be used here.

mumoshu · 2017-03-08T00:11:53Z

cfnstack/assets.go

+	Key      string
+	Bucket   string
+	Path     string
+	S3Region string


This should be Region Region rather than S3Region string once https://github.com/coreos/kube-aws/pull/390/files#r104814373 is implemented.

mumoshu · 2017-03-08T00:18:00Z

core/controlplane/config/templates/cloud-config-controller

+        RemainAfterExit=true
+        ExecStartPre=/usr/bin/docker pull {{.PauseImage.Repo}}
+        ExecStart=/usr/bin/docker tag  {{.PauseImage.Repo}} gcr.io/google_containers/pause-amd64:3.0
+{{end}}


mumoshu · 2017-03-08T00:26:03Z

core/nodepool/config/templates/stack-template.json

-  "   {{.AWSCliImageRepo}}:{{.AWSCliTag}} -- aws s3 --region $REGION  cp {{ .S3URI }}/{{ .StackName }}/$USERDATA_FILE /var/run/coreos/",
+  {{- if .AWSCliImage.RktPullDocker }}
+  "   --insecure-options=image \\",
+  {{- end }}


To reduce ifs in templates and make it more testable, how about implementing something like:

func (i Image) RktOptions() string { if i.RktPullDocker { return "--insecure-options=image" } return "" }

and refer to it from here?

mumoshu · 2017-03-08T00:30:19Z

core/nodepool/config/templates/stack-template.json

+  {{- if .AWSCliImage.RktPullDocker }}
+  "   --insecure-options=image \\",
+  {{- end }}
+  "   {{ if .AWSCliImage.RktPullDocker }}docker://{{ end }}{{.AWSCliImage.Repo}}:{{.AWSCliTag}} --exec=aws -- s3 --region $REGION  cp {{ .S3URI }}/{{ .StackName }}/$USERDATA_FILE /var/run/coreos/",


Almost the same as above at heart but how about implementing something like:

func (i Image) RktImage() string { if i.RktPullDocker { return fmt.Sprintf("docker://%s:%s", i.Repo, i.Tag) } return fmt.Sprintf("%s:%s", i.Repo, i.Tag) }

and refer to it from here?
I guess we'd better to drop *Tags e.g. AWSCliTag and integrate it to the Image struct to achieve this.

mumoshu · 2017-03-08T00:35:51Z

core/nodepool/config/config.go

-		KMSKeyARN:      c.KMSKeyARN,
-		EncryptService: c.providedEncryptService,
-	})
+	if stackConfig.ComputedConfig.IsChinaRegion {


If IsChinaRegion is almost meant for not TLSAssetsEncryptionEnabled in the context of this PR,
can we introduce new func like:

func (c *Cluster) TLSAssetsEncryptionEnabled() bool { return c.ManageCertificate && !c.IsChinaRegion }

according to https://github.com/coreos/kube-aws/pull/390/files#diff-954229fc24045500f68b18079524f6e1R222 and refer to it from here and various locations including templates?

mumoshu · 2017-03-08T00:41:07Z

core/controlplane/config/templates/stack-template.json

-          "   {{$.AWSCliImageRepo}}:{{$.AWSCliTag}} -- aws s3 --region $REGION  cp {{ $.S3URI }}/{{ $.StackName }}/$USERDATA_FILE /var/run/coreos/",
+          {{- if $.AWSCliImage.RktPullDocker }}
+          "   --insecure-options=image \\",
+          {{- end }}


Relates to https://github.com/coreos/kube-aws/pull/390/files#r104816364

mumoshu · 2017-03-08T00:41:18Z

core/controlplane/config/templates/cloud-config-worker

-        {{.AWSCliImageRepo}}:{{.AWSCliTag}} --exec=/bin/bash -- \
+        {{- if .AWSCliImage.RktPullDocker }}
+        --insecure-options=image \
+        {{- end }}


Relates to https://github.com/coreos/kube-aws/pull/390/files#r104816364

mumoshu · 2017-03-08T00:41:24Z

core/controlplane/config/templates/cloud-config-worker

-        {{.AWSCliImageRepo}}:{{.AWSCliTag}} --exec=/bin/bash -- \
+        {{- if .AWSCliImage.RktPullDocker }}
+        --insecure-options=image \
+        {{- end }}


Relates to https://github.com/coreos/kube-aws/pull/390/files#r104816364

mumoshu · 2017-03-08T00:41:45Z

core/controlplane/config/templates/cloud-config-worker

-        {{.AWSCliImageRepo}}:{{.AWSCliTag}} --exec=/bin/bash -- \
+        {{- if .AWSCliImage.RktPullDocker }}
+        --insecure-options=image \
+        {{- end }}


Relates to https://github.com/coreos/kube-aws/pull/390/files#r104816364

mumoshu · 2017-03-08T00:57:25Z

Thanks @camilb @mpucholblasco, I'm really impressed to see your great work!
I've left comments about cosmetics but the changes LGTM overall.

there is no KMS support in cn-north-1region. We preferred to disable this entirely for this region only.

Sounds good at this stage 👍

China region has a different S3 endpoint. For now we are passing a flag "--s3-region=cn-north-1" when starting the cluster but we are working to get the region automatically from cluster.yaml

👍

there are different ARNs. We automatically detect and change them for cn-north-1

Although I'd prefer adding ifs in golang code rather than templates, the functionality sounds great 👍

most of the docker or rkt images are not pulled from dockerhub or quay and gcr.io is completely blocked in China. On large images, like hyperkube, you get many timeouts from quay.
We added an option to specify your own images, hosted on a registry in China.

👍
// Btw I had imagined of hosting our own images even outside of the China region for faster deployments.

#cn-north-1
dd if=/home/core/root.img of=/dev/xvdb bs=1M oflag=direct
mkdir -p /ami
mount /dev/xvdb9 /ami
rm /ami/home/core/.ssh/authorized_keys
rm /ami/home/core/.ssh/authorized_keys.d/*
umount /ami

Sorry for being very nit picky but perhaps you're missing something like aws ec2 attach-volume --device xvdb9 ... in the beginning of this snippet? 😃

Anyway, this guidance of cloning AMIs among regions would help everyone trying kube-aws within the China region in the future.

camilb · 2017-03-09T00:07:58Z

@mumoshu thanks for your feedback. We agree with all of your suggestions and we'll make the changes these days.

…into kube-china

codecov-io · 2017-03-12T23:28:31Z

Codecov Report

Merging #390 into master will decrease coverage by 0.18%.
The diff coverage is 46.51%.

@@            Coverage Diff             @@
##           master     #390      +/-   ##
==========================================
- Coverage   38.71%   38.53%   -0.19%     
==========================================
  Files          29       30       +1     
  Lines        2278     2351      +73     
==========================================
+ Hits          882      906      +24     
- Misses       1277     1324      +47     
- Partials      119      121       +2

Impacted Files	Coverage Δ
model/region.go	`0% <0%> (ø)`	✅
model/derived/etcd_node.go	`0% <0%> (ø)`	✅
model/image.go	`0% <0%> (ø)`
model/derived/etcd_cluster.go	`80% <100%> (ø)`	✅
cfnstack/provisioner.go	`11.21% <100%> (+0.43%)`	✅
cfnstack/assets.go	`40.84% <66.66%> (+1.13%)`	✅
core/controlplane/cluster/cluster.go	`43.26% <66.66%> (+0.23%)`	✅
core/nodepool/cluster/cluster.go	`38.01% <66.66%> (ø)`	✅
core/controlplane/config/tls_config.go	`71.37% <71.42%> (-0.57%)`	❌
core/controlplane/config/config.go	`65.42% <79.16%> (+0.32%)`	✅
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c963bd...2655dcc. Read the comment docs.

mumoshu · 2017-03-12T23:43:21Z

core/nodepool/config/templates/stack-template.json

@@ -326,7 +326,7 @@
                  "Action": [
                    "s3:GetObject"
                  ],
-                  "Resource": "arn:aws:s3:::{{$.UserDataWorkerS3Path}}"
+                  "Resource": "arn:aws{{if .IsChinaRegion}}-cn{{end}}:s3:::{{$.UserDataWorkerS3Path}}"


nit but func (r Region) Partition() string to return aws-cn or aws according to http://docs.aws.amazon.com/general/latest/gr/aws-arns-and-namespaces.html would be slightly better.

mumoshu · 2017-03-12T23:43:42Z

core/nodepool/config/deployment.go

+	c.HyperkubeImage.Tag = c.K8sVer
+	c.AWSCliImage.MergeIfEmpty(main.AWSCliImage)
+	c.CalicoCtlImage.MergeIfEmpty(main.CalicoCtlImage)
+	c.PauseImage.MergeIfEmpty(main.PauseImage)


mumoshu · 2017-03-12T23:46:34Z

core/controlplane/config/config.go

+
+func (i *Image) RepoWithTag() string {
+	return fmt.Sprintf("%s:%s", i.Repo, i.Tag)
+}


nit but the type Image could be moved to the model package i.e. model/image.go

mumoshu · 2017-03-12T23:48:48Z

core/nodepool/config/templates/stack-template.json

@@ -348,7 +348,7 @@
                  "Effect": "Allow",
                  "Resource":
                    { "Fn::Join": [ "", [
-                      "arn:aws:cloudformation:",
+                      "arn:aws{{if .IsChinaRegion}}-cn{{end}}:cloudformation:",


Could be changed to arn:{{$.Region.Partition}}:cloudformation: if https://github.com/coreos/kube-aws/pull/390/files#r105572866 was addressed

mumoshu · 2017-03-12T23:53:36Z

Thanks for your effort @camilb 🙇
I've added 3 nits but LGTM overall.
Could you hopefully fix these nits and rebase?
As this is already LGTM overall, if you got short on time, I can merge this and fix it later 😃

mpucholblasco · 2017-03-13T12:49:34Z

Hello @mumoshu ,
we just pushed those changes you requested. We tried to rebase, but we have seen some last changes on master and merged with our code to ensure everything works.

After reviewing last changes, if you consider it, please, can you merge and rebase?

Thanks,
@camilb & me.

mumoshu · 2017-03-15T00:02:26Z

Thanks @mpucholblasco, sure 👍
I'm working on it now

mumoshu · 2017-03-15T00:22:02Z

LGTM. Great job, @camilb and @mpucholblasco!

Also align to work from kubernetes-retired#390 by allowing image repo/tag to be overridden.

* Introduce the rescheduler For #118. - Currently using raw Pod as per [salt setup](https://github.com/kubernetes/kubernetes/blob/master/cluster/salt base/salt/rescheduler/rescheduler.manifest0 - Logging routed via standard streams - gcr image used, not sure if hyperkube has the rescheduler * Rescheduler is off by default, can be enabled Also align to work from #390 by allowing image repo/tag to be overridden. * Comment on experimental nature of rescheduler * Remove resource todo * Correct default to v0.2.2 v0.3.0 is not ready until k8s 1.6 * Change rescheduler to Deployment Rescheduler currently crashes if there is more than one present * Add validation tests

* kubernetes-incubator/master: (29 commits) Emit errors when kube-aws sees unexpected keys in cluster.yaml Resolves kubernetes-retired#404 Tag controller nodes appropriately with `kubernetes.io/role`. Resolves kubernetes-retired#370 Make Container Linux AMI fetching a bit more reliable Stop locksmithd errors on etcd nodes Upgrade heapster to version 1.3.0 (kubernetes-retired#420) Auth token file support (kubernetes-retired#418) Update README.md Update README accordingly to the new git repo AWS China region support (kubernetes-retired#390) Conform as a Kubernetes Incubator Project Fixed typo in template upgrade aws-sdk to latest version Fix kubernetes-retired#388 Upgrade Kubernetes version to v1.5.4 Fix assumed public hostnames for EC2 instances in us-east-1 Fix assumed public hostnames for EC2 instances in us-east-1 typo fix: etcdDataVolumeEncrypted not creating encrypted volumes fixes kubernetes-retired#383 Allow disabling wait signals fixes kubernetes-retired#371 Update file paths in readme Fix an issue with glue security group documentation ...

AWS China region support + Parameterized docker/rkt images * Added S3 China * Parameterized images * Remove "decrypt" dependencies for China region. * Fix ca cert for China region. * Fix ARN names for China region. * Adding logging * Remove KMS from China region in nodepools. * Add "amd64-pause" image service for China. * Added testing and fixing * Removed parameter s3-region from all commands * Added image properties * Used TLSAssetsEncryptionEnabled instead of asking for China Region * Changed region string by region model

* Introduce the rescheduler For kubernetes-retired#118. - Currently using raw Pod as per [salt setup](https://github.com/kubernetes/kubernetes/blob/master/cluster/salt base/salt/rescheduler/rescheduler.manifest0 - Logging routed via standard streams - gcr image used, not sure if hyperkube has the rescheduler * Rescheduler is off by default, can be enabled Also align to work from kubernetes-retired#390 by allowing image repo/tag to be overridden. * Comment on experimental nature of rescheduler * Remove resource todo * Correct default to v0.2.2 v0.3.0 is not ready until k8s 1.6 * Change rescheduler to Deployment Rescheduler currently crashes if there is more than one present * Add validation tests

camilb and others added 14 commits March 2, 2017 10:56

Added S3 China

394c346

Parameterized images

292e928

Added China features

9ad9e31

Remove "decrypt" dependencies for China region.

940afe1

Fix ca cert for China region.

9c1b8c4

Fix ARN names for China region.

db9f3f3

Adding logging

9fc6714

Merge branch 'master' of https://github.com/mpucholblasco/kube-aws

57a9625

Remove KMS from China region in nodepools.

f269eb4

Add "amd64-pause" image service for China.

80c789b

Option for rkt to pull from Docker repos.

d02be1c

Merge pull request #2 from mpucholblasco/test

bea15ed

Option for rkt to pull from Docker repos.

mumoshu reviewed Mar 8, 2017

View reviewed changes

mpuchol-netquest added 4 commits March 10, 2017 14:32

Added testing and fixing

73531a5

Merge branch 'kube-china' of https://github.com/mpucholblasco/kube-aws …

c547388

…into kube-china

Merge remote-tracking branch 'upstream/master'

29b01f8

Integrating master into kube-china

564472c

mumoshu reviewed Mar 12, 2017

View reviewed changes

mpuchol-netquest added 4 commits March 13, 2017 12:55

Moved image to model

b3f165f

Changed region string by region model

e48f3f8

Formatted

8fd9d9c

Merge remote-tracking branch 'upstream/master' into kube-china

2655dcc

mumoshu changed the title ~~[WIP] AWS China region support~~ AWS China region support Mar 15, 2017

mumoshu merged commit cdf0ef7 into kubernetes-retired:master Mar 15, 2017

mumoshu added this to the v0.9.5-rc.4 milestone Mar 15, 2017

mumoshu mentioned this pull request Mar 17, 2017

Automatic recovery from permanent failures of etcd3 nodes #417

Merged

4 tasks

cknowles mentioned this pull request Mar 23, 2017

Introduce the rescheduler #441

Merged

cknowles pushed a commit to cknowles/kube-aws that referenced this pull request Mar 24, 2017

Rescheduler is off by default, can be enabled

87d9c52

Also align to work from kubernetes-retired#390 by allowing image repo/tag to be overridden.

qqshfox mentioned this pull request Jul 9, 2017

support china region kubernetes/kops#2889

Closed

This was referenced Mar 20, 2019

有没有办法把ami换成最新的debian ami kubernetes/kops#6642

Closed

amazon-vpc-routed-eni 节点会出现多个ip导致ssh 不到节点 nwcdheap/kops-cn#61

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AWS China region support #390

AWS China region support #390

camilb commented Mar 7, 2017 •

edited

mumoshu Mar 8, 2017

mumoshu Mar 8, 2017

mumoshu Mar 8, 2017

mumoshu Mar 8, 2017

mumoshu Mar 8, 2017

mumoshu Mar 8, 2017 •

edited

mumoshu Mar 8, 2017

mumoshu Mar 8, 2017

mumoshu Mar 8, 2017

mumoshu Mar 8, 2017

mumoshu commented Mar 8, 2017 •

edited

camilb commented Mar 9, 2017

codecov-io commented Mar 12, 2017 •

edited

mumoshu Mar 12, 2017

mumoshu Mar 12, 2017

mumoshu Mar 12, 2017

mumoshu Mar 12, 2017

mumoshu commented Mar 12, 2017

mpucholblasco commented Mar 13, 2017

mumoshu commented Mar 15, 2017

mumoshu commented Mar 15, 2017

AWS China region support #390

AWS China region support #390

Conversation

camilb commented Mar 7, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mumoshu Mar 8, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mumoshu commented Mar 8, 2017 • edited

camilb commented Mar 9, 2017

codecov-io commented Mar 12, 2017 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mumoshu commented Mar 12, 2017

mpucholblasco commented Mar 13, 2017

mumoshu commented Mar 15, 2017

mumoshu commented Mar 15, 2017

camilb commented Mar 7, 2017 •

edited

mumoshu Mar 8, 2017 •

edited

mumoshu commented Mar 8, 2017 •

edited

codecov-io commented Mar 12, 2017 •

edited