Skip to content

Conversation

Sn0rt
Copy link
Contributor

@Sn0rt Sn0rt commented Sep 30, 2019

this PR for Amazone vpc cni support wtih v1alpha2 , v1alpha1 here is #1062

how to use

this is different from HEAD of repo, the user should set a bash env variable as follow

$ export AWS_REGION=us-east-2
$ export AWS_CREDENTIALS=$(cat ~/aws_cluster_deploy_env.sh)
$ export AWS_ACCESS_KEY_ID=$(echo $AWS_CREDENTIALS | jq .AccessKey.AccessKeyId -r)
$ export AWS_SECRET_ACCESS_KEY=$(echo $AWS_CREDENTIALS | jq .AccessKey.SecretAccessKey -r)
$ export CLUSTER_NAME=sn0rttest
$ export NETWORK=amazon-vpc-cni-k8s
$ rm -rf examples/_out
$ ./examples/generate.sh
Generated /Volumes/home/guohao/workspace/go/src/sigs.k8s.io/cluster-api-provider-aws/examples/_out/addons.yaml
Generated /Volumes/home/guohao/workspace/go/src/sigs.k8s.io/cluster-api-provider-aws/examples/_out/cluster.yaml
Generated /Volumes/home/guohao/workspace/go/src/sigs.k8s.io/cluster-api-provider-aws/examples/_out/controlplane.yaml
Generated /Volumes/home/guohao/workspace/go/src/sigs.k8s.io/cluster-api-provider-aws/examples/_out/machinedeployment.yaml

cluster and machine spec

the cluster file example

apiVersion: cluster.x-k8s.io/v1alpha2
kind: Cluster
metadata:
  annotations:
    cluster.k8s.io/network-cni: amazon-vpc-cni-k8s
  name: test1
spec:
  clusterNetwork:
    pods:
      cidrBlocks:
      - 10.0.0.0/16
    services:
      cidrBlocks:
      - 192.168.0.0/16
  infrastructureRef:
    apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
    kind: AWSCluster
    name: test1
    namespace: default
---
apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
kind: AWSCluster
metadata:
  name: test1
  namespace: default
spec:
  region: us-east-2
  sshKeyName: default

the KubeadmConfig of test1-controlplane-0

apiVersion: bootstrap.cluster.x-k8s.io/v1alpha2
kind: KubeadmConfig
metadata:
  name: test1-controlplane-0
  namespace: default
spec:
  clusterConfiguration:
    apiServer:
      extraArgs:
        cloud-provider: aws
    controllerManager:
      extraArgs:
        cloud-provider: aws
  initConfiguration:
    nodeRegistration:
      kubeletExtraArgs:
        cloud-provider: aws
        max-pods: 19
        node-ip: '{{ ds.meta_data.hostname }}'
      name: '{{ ds.meta_data.hostname }}'

the AWSMachine of test1-controlplane-0

apiVersion: cluster.x-k8s.io/v1alpha2
kind: Machine
metadata:
  labels:
    cluster.x-k8s.io/cluster-name: test1
    cluster.x-k8s.io/control-plane: "true"
  name: test1-controlplane-0
spec:
  bootstrap:
    configRef:
      apiVersion: bootstrap.cluster.x-k8s.io/v1alpha2
      kind: KubeadmConfig
      name: test1-controlplane-0
  infrastructureRef:
    apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
    kind: AWSMachine
    name: test1-controlplane-0
    namespace: default
  version: v1.15.3

the AWSMachine of test1-controlplane-0

apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
kind: AWSMachine
metadata:
  name: test1-controlplane-0
  namespace: default
spec:
  iamInstanceProfile: controllers.cluster-api-provider-aws.sigs.k8s.io
  instanceType: t2.medium
  sshKeyName: default

the machine deployment

apiVersion: bootstrap.cluster.x-k8s.io/v1alpha2
kind: KubeadmConfigTemplate
metadata:
  name: test1-md-0
  namespace: default
spec:
  template:
    spec:
      joinConfiguration:
        nodeRegistration:
          kubeletExtraArgs:
            cloud-provider: aws
            max-pods: 19
            node-ip: '{{ ds.meta_data.hostname }}'
          name: '{{ ds.meta_data.hostname }}'

security group info

Drawing on the security group policy of EKS, nodes in the cluster directly communicate with each other by default.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 30, 2019
@k8s-ci-robot
Copy link
Contributor

Hi @Sn0rt. Thanks for your PR.

I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 30, 2019
@Sn0rt Sn0rt changed the title WIP: CAPA support amazon vpc cni k8s WIP: CAPA support amazon vpc cni k8s 0.4.x Sep 30, 2019
Copy link
Member

@randomvariable randomvariable Oct 7, 2019

Choose a reason for hiding this comment

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

Would like to follow K8s convention for CamelCase on field values and use AmazonVPC?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would like to follow K8s convention for CamelCase on field values and use AmazonVPC?

copy that.

Comment on lines +299 to +320
Copy link
Contributor

@aaroniscode aaroniscode Oct 7, 2019

Choose a reason for hiding this comment

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

Would it make sense to make this optional? Amazon best practices is to grant Least Privilege. A cluster not running aws-vpc-cni doesn't need these privileges.

The question is how to make this optional? Would this be best exposed in clusterawsadm as a flag?

Maybe --cni amazon-vpc-cni. Not sure there are other CNI's that would require different policy.

or --add-vpc-cni-policy

Thoughts on if this should be optional and if so, how best to expose?

Copy link
Contributor Author

@Sn0rt Sn0rt Oct 8, 2019

Choose a reason for hiding this comment

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

someone wants to create multiple clusters with different network solutions, such as calico and the amazon-vpc-cni, under one AWS account.
that use case how to separate the privilege of account ?

Copy link
Member

@randomvariable randomvariable Oct 8, 2019

Choose a reason for hiding this comment

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

Make it accept multi-value with comma-separation?

Options:

  • Calico
  • AmazonVPC

Default: [Calico]

Or leave it as one or another for now, and we can revisit this in a follow up issue. Ideally, we want to move CNI rules out of the controller loop anyway, so can figure it out properly then.

Copy link
Contributor Author

@Sn0rt Sn0rt Oct 9, 2019

Choose a reason for hiding this comment

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

agree with you, we can talk about this issue later.

@Sn0rt Sn0rt changed the title WIP: CAPA support amazon vpc cni k8s 0.4.x CAPA support amazon vpc cni k8s 0.4.x Oct 8, 2019
@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 Oct 8, 2019
@Sn0rt
Copy link
Contributor Author

Sn0rt commented Oct 9, 2019

1 cluster spec

1.1 cluster spec

the cluster yaml will tell CAPA controller I want to create a cluster and the network of cluster is AmazonVPC.

apiVersion: cluster.x-k8s.io/v1alpha2
kind: Cluster
metadata:
  annotations:
    cluster.k8s.io/network-cni: AmazonVPC
  name: newcluster
spec:
  clusterNetwork:
    pods:
      cidrBlocks:
      - 10.0.0.0/16
    services:
      cidrBlocks:
      - 192.168.0.0/16
  infrastructureRef:
    apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
    kind: AWSCluster
    name: newcluster
    namespace: default
---
apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
kind: AWSCluster
metadata:
  name: newcluster
  namespace: default
spec:
  region: us-east-2
  sshKeyName: guohao

1.2 controlplane

will create three master nodes with one AZ, and will set kubelet parmeter with maxpod and nodeip.

apiVersion: bootstrap.cluster.x-k8s.io/v1alpha2
kind: KubeadmConfig
metadata:
  name: newcluster-controlplane-0
  namespace: default
spec:
  clusterConfiguration:
    apiServer:
      extraArgs:
        cloud-provider: aws
    controllerManager:
      extraArgs:
        cloud-provider: aws
  initConfiguration:
    nodeRegistration:
      kubeletExtraArgs:
        cloud-provider: aws
        max-pods: "19"
        node-ip: '{{ ds.meta_data.local_ipv4 }}'
      name: '{{ ds.meta_data.hostname }}'
---
apiVersion: bootstrap.cluster.x-k8s.io/v1alpha2
kind: KubeadmConfig
metadata:
  name: newcluster-controlplane-1
  namespace: default
spec:
  joinConfiguration:
    controlPlane: {}
    nodeRegistration:
      kubeletExtraArgs:
        cloud-provider: aws
        max-pods: "19"
        node-ip: '{{ ds.meta_data.local_ipv4 }}'
      name: '{{ ds.meta_data.hostname }}'
---
apiVersion: bootstrap.cluster.x-k8s.io/v1alpha2
kind: KubeadmConfig
metadata:
  name: newcluster-controlplane-2
  namespace: default
spec:
  joinConfiguration:
    controlPlane: {}
    nodeRegistration:
      kubeletExtraArgs:
        cloud-provider: aws
        max-pods: "19"
        node-ip: '{{ ds.meta_data.local_ipv4 }}'
      name: '{{ ds.meta_data.hostname }}'
---
apiVersion: cluster.x-k8s.io/v1alpha2
kind: Machine
metadata:
  labels:
    cluster.x-k8s.io/cluster-name: newcluster
    cluster.x-k8s.io/control-plane: "true"
  name: newcluster-controlplane-0
spec:
  bootstrap:
    configRef:
      apiVersion: bootstrap.cluster.x-k8s.io/v1alpha2
      kind: KubeadmConfig
      name: newcluster-controlplane-0
  infrastructureRef:
    apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
    kind: AWSMachine
    name: newcluster-controlplane-0
    namespace: default
  version: v1.15.3
---
apiVersion: cluster.x-k8s.io/v1alpha2
kind: Machine
metadata:
  labels:
    cluster.x-k8s.io/cluster-name: newcluster
    cluster.x-k8s.io/control-plane: "true"
  name: newcluster-controlplane-1
spec:
  bootstrap:
    configRef:
      apiVersion: bootstrap.cluster.x-k8s.io/v1alpha2
      kind: KubeadmConfig
      name: newcluster-controlplane-1
  infrastructureRef:
    apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
    kind: AWSMachine
    name: newcluster-controlplane-1
    namespace: default
  version: v1.15.3
---
apiVersion: cluster.x-k8s.io/v1alpha2
kind: Machine
metadata:
  labels:
    cluster.x-k8s.io/cluster-name: newcluster
    cluster.x-k8s.io/control-plane: "true"
  name: newcluster-controlplane-2
spec:
  bootstrap:
    configRef:
      apiVersion: bootstrap.cluster.x-k8s.io/v1alpha2
      kind: KubeadmConfig
      name: newcluster-controlplane-2
  infrastructureRef:
    apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
    kind: AWSMachine
    name: newcluster-controlplane-2
    namespace: default
  version: v1.15.3
---
apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
kind: AWSMachine
metadata:
  name: newcluster-controlplane-0
  namespace: default
spec:
  iamInstanceProfile: control-plane.cluster-api-provider-aws.sigs.k8s.io
  instanceType: t2.medium
  sshKeyName: guohao
---
apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
kind: AWSMachine
metadata:
  name: newcluster-controlplane-1
  namespace: default
spec:
  iamInstanceProfile: control-plane.cluster-api-provider-aws.sigs.k8s.io
  instanceType: t2.medium
  sshKeyName: guohao
---
apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
kind: AWSMachine
metadata:
  name: newcluster-controlplane-2
  namespace: default
spec:
  iamInstanceProfile: control-plane.cluster-api-provider-aws.sigs.k8s.io
  instanceType: t2.medium
  sshKeyName: guohaoyaml

2 k8s info

2.1 node info

all of the node are ready.

$ kubectl --kubeconfig guohao4 get node -o wide --all-namespaces
NAME                                       STATUS   ROLES    AGE     VERSION   INTERNAL-IP   EXTERNAL-IP   OS-IMAGE             KERNEL-VERSION    CONTAINER-RUNTIME
ip-10-0-0-195.us-east-2.compute.internal   Ready    master   5m39s   v1.15.3   10.0.0.195    <none>        Ubuntu 18.04.3 LTS   4.15.0-1045-aws   containerd://1.2.8
ip-10-0-0-196.us-east-2.compute.internal   Ready    master   5m37s   v1.15.3   10.0.0.196    <none>        Ubuntu 18.04.3 LTS   4.15.0-1045-aws   containerd://1.2.8
ip-10-0-0-208.us-east-2.compute.internal   Ready    master   6m44s   v1.15.3   10.0.0.208    <none>        Ubuntu 18.04.3 LTS   4.15.0-1045-aws   containerd://1.2.8
ip-10-0-0-39.us-east-2.compute.internal    Ready    <none>   62s     v1.15.3   10.0.0.39     <none>        Ubuntu 18.04.3 LTS   4.15.0-1045-aws   containerd://1.2.8

2.2 pod info

2048 pods are running.

$ kubectl --kubeconfig guohao4 get pod -o wide --all-namespaces
NAMESPACE     NAME                                                               READY   STATUS    RESTARTS   AGE     IP           NODE                                       NOMINATED NODE   READINESS GATES
2048-game     2048-deployment-c99556b4b-sr99d                                    1/1     Running   0          2m56s   10.0.0.189   ip-10-0-0-39.us-east-2.compute.internal    <none>           <none>
2048-game     2048-deployment-c99556b4b-vmwzm                                    1/1     Running   0          2m56s   10.0.0.92    ip-10-0-0-39.us-east-2.compute.internal    <none>           <none>
kube-system   aws-node-9qm7j                                                     1/1     Running   0          5m16s   10.0.0.208   ip-10-0-0-208.us-east-2.compute.internal   <none>           <none>
kube-system   aws-node-m4sgr                                                     1/1     Running   0          73s     10.0.0.39    ip-10-0-0-39.us-east-2.compute.internal    <none>           <none>
kube-system   aws-node-mcb5r                                                     1/1     Running   0          5m16s   10.0.0.195   ip-10-0-0-195.us-east-2.compute.internal   <none>           <none>
kube-system   aws-node-n28js                                                     1/1     Running   0          5m16s   10.0.0.196   ip-10-0-0-196.us-east-2.compute.internal   <none>           <none>
kube-system   coredns-5c98db65d4-gwhjs                                           1/1     Running   0          6m37s   10.0.0.174   ip-10-0-0-196.us-east-2.compute.internal   <none>           <none>
kube-system   coredns-5c98db65d4-xf52h                                           1/1     Running   0          6m37s   10.0.0.64    ip-10-0-0-196.us-east-2.compute.internal   <none>           <none>
kube-system   etcd-ip-10-0-0-195.us-east-2.compute.internal                      1/1     Running   0          5m50s   10.0.0.195   ip-10-0-0-195.us-east-2.compute.internal   <none>           <none>
kube-system   etcd-ip-10-0-0-196.us-east-2.compute.internal                      1/1     Running   0          5m35s   10.0.0.196   ip-10-0-0-196.us-east-2.compute.internal   <none>           <none>
kube-system   etcd-ip-10-0-0-208.us-east-2.compute.internal                      1/1     Running   0          6m2s    10.0.0.208   ip-10-0-0-208.us-east-2.compute.internal   <none>           <none>
kube-system   kube-apiserver-ip-10-0-0-195.us-east-2.compute.internal            1/1     Running   0          4m57s   10.0.0.195   ip-10-0-0-195.us-east-2.compute.internal   <none>           <none>
kube-system   kube-apiserver-ip-10-0-0-196.us-east-2.compute.internal            1/1     Running   1          5m33s   10.0.0.196   ip-10-0-0-196.us-east-2.compute.internal   <none>           <none>
kube-system   kube-apiserver-ip-10-0-0-208.us-east-2.compute.internal            1/1     Running   0          5m34s   10.0.0.208   ip-10-0-0-208.us-east-2.compute.internal   <none>           <none>
kube-system   kube-controller-manager-ip-10-0-0-195.us-east-2.compute.internal   1/1     Running   0          4m32s   10.0.0.195   ip-10-0-0-195.us-east-2.compute.internal   <none>           <none>
kube-system   kube-controller-manager-ip-10-0-0-196.us-east-2.compute.internal   1/1     Running   0          4m47s   10.0.0.196   ip-10-0-0-196.us-east-2.compute.internal   <none>           <none>
kube-system   kube-controller-manager-ip-10-0-0-208.us-east-2.compute.internal   1/1     Running   1          5m57s   10.0.0.208   ip-10-0-0-208.us-east-2.compute.internal   <none>           <none>
kube-system   kube-proxy-72mpq                                                   1/1     Running   0          5m42s   10.0.0.196   ip-10-0-0-196.us-east-2.compute.internal   <none>           <none>
kube-system   kube-proxy-q8vlv                                                   1/1     Running   0          5m50s   10.0.0.195   ip-10-0-0-195.us-east-2.compute.internal   <none>           <none>
kube-system   kube-proxy-svtfz                                                   1/1     Running   0          6m37s   10.0.0.208   ip-10-0-0-208.us-east-2.compute.internal   <none>           <none>
kube-system   kube-proxy-xwglg                                                   1/1     Running   0          73s     10.0.0.39    ip-10-0-0-39.us-east-2.compute.internal    <none>           <none>
kube-system   kube-scheduler-ip-10-0-0-195.us-east-2.compute.internal            1/1     Running   0          4m33s   10.0.0.195   ip-10-0-0-195.us-east-2.compute.internal   <none>           <none>
kube-system   kube-scheduler-ip-10-0-0-196.us-east-2.compute.internal            1/1     Running   0          4m33s   10.0.0.196   ip-10-0-0-196.us-east-2.compute.internal   <none>           <none>
kube-system   kube-scheduler-ip-10-0-0-208.us-east-2.compute.internal            1/1     Running   1          5m58s   10.0.0.208   ip-10-0-0-208.us-east-2.compute.internal   <none>           <none>

@randomvariable
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 9, 2019
@randomvariable
Copy link
Member

Make sure to put the license headers on the new files.

@Sn0rt
Copy link
Contributor Author

Sn0rt commented Oct 9, 2019

/test pull-cluster-api-provider-aws-verify

@ncdc
Copy link
Contributor

ncdc commented Oct 10, 2019

Is this for #931?

@ncdc ncdc changed the title CAPA support amazon vpc cni k8s 0.4.x Add Amazon VPC CNI support Oct 10, 2019
@ncdc ncdc added this to the v0.5.0 milestone Oct 10, 2019
@ncdc
Copy link
Contributor

ncdc commented Oct 10, 2019

/hold
We have a few small PRs going in to master for a new v0.4.x release before we fully open master for breaking changes / new features for v0.5.0. We'll unhold once that's done. Thanks for your patience!

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 10, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Sn0rt
To complete the pull request process, please assign detiber
You can assign the PR to them by writing /assign @detiber in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cluster.k8s.io/network-cni: ${NETWORK}
cluster.x-k8s.io/network-cni: ${NETWORK}

Copy link
Member

Choose a reason for hiding this comment

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

We need to adopt the alpha x-k8s.io annotations for now, but otherwise good to go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to adopt the alpha x-k8s.io annotations for now, but otherwise good to go.

thx very much.

Copy link
Member

Choose a reason for hiding this comment

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

We need to adopt the alpha x-k8s.io annotations for now, but otherwise good to go.

@randomvariable
Copy link
Member

Apologies @Sn0rt for leaving this so long.
As much as we would like to enable the choice of the VPC CNI, I don't think this is the right place for it. I would like to revisit this after we have clusterctl v2 in place which is happening as part of v1alpha3. I have created #1484 as an umbrella issue and would welcome your comments.

/close

@k8s-ci-robot
Copy link
Contributor

@randomvariable: Closed this PR.

In response to this:

Apologies @Sn0rt for leaving this so long.
As much as we would like to enable the choice of the VPC CNI, I don't think this is the right place for it. I would like to revisit this after we have clusterctl v2 in place which is happening as part of v1alpha3. I have created #1484 as an umbrella issue and would welcome your comments.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 15, 2020
@k8s-ci-robot
Copy link
Contributor

@Sn0rt: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants