Update calico to use the correct CIDR for pods

[ottoyiu]

This PR is in relations to the conversations had in #1171.

For reference, here's the default values for the individual components.

  kubeAPIServer:
    serviceClusterIPRange: 100.64.0.0/13
  kubeControllerManager:
    # I do not think we need this with CNI
    clusterCIDR: 100.96.0.0/11
  kubeDNS:
    serverIP: 100.64.0.10
  kubeProxy:
    clusterCIDR: 100.96.0.0/11
  kubelet:
    clusterDNS: 100.64.0.10
    networkPluginName: cni
    nonMasqueradeCIDR: 100.64.0.0/10
  # these values are the base subnets that everything is setup from
  # I do not believe that these should overlap
  nonMasqueradeCIDR: 100.64.0.0/10
  serviceClusterIPRange: 100.64.0.0/13

Currently, we are using .NonMasqueradeCIDR in the wrong fashion. We
should be using .KubeControllerManager.ClusterCIDR instead to prevent IP
collision with Service IPs.

Note: The .NonMasqueradeCIDR is the cluster's base subnet, and should not be directly used by the components because of overlap.

Scenarios Tested

New Cluster Creation for k8s 1.6 - PASS

Deployed a new Kubernetes 1.6.4 cluster using this branch with the changes. Pods are now being allocated an IP within the 100.96.0.0/11 range (as defined by .KubeControllerManager.ClusterCIDR vs .NonMasqueradeCIDR

Tested:

• Pod to Service IP (100.64.0.10/kube-dns and example nginx service)
• Pod to Pod
• Pod to External (google.com)

Observation:

• Nodes are assigned a Pod CIDR within the .KubeControllerManager.ClusterCIDR range as expected
• Pods are being assigned IPs in the proper range
• Service IP range remains unchanged

Cluster Upgrade for k8s 1.6 (cluster first created using kops-1.6.2)

Deployed a new Kubernetes 1.6.4 cluster using kops-1.6.2, then ran kops built from this branch:

• kops update cluster --yes to change the CIDR.
  Observation: All pods are still functional and running with the existing Pod IPs. Pod to Service IP, Pod to Pod, Pod to External (google.com) tested.

Upgrade strategy attempts

1. kops rolling-update --force --yes to do rolling restart - FAIL
   Observation: Existing Pods continue to be functional with its old Pod IPs (which may or may not lie within the newly defined IP range) while nodes are being cycled. A period of several minutes where DNS is not responsive in Pods, and shortly recovers. Not sure if it has anything to do with the CIDR change. Seems more to do with the "rolling" update. New Nodes are still assigned old CIDR range. Service IP range remains unchanged.

2. Deleting running calico/node pods from calico daemonset - FAIL
   kubectl get pods --namespace kube-system | grep calico-node | awk '{print $1}' | xargs kubectl delete pod --namespace kube-system

Observation: Existing Pods continue to be functional with its old Pod IPs. New Pods being rescheduled on existing Nodes are still getting a Pod IP in the old IP range. New Nodes are still assigned old CIDR range.

Failure Investigation

Calico only uses the CALICO_IPV4POOL_CIDR to create a default IPv4 pool if a pool doesn't exist already:
https://github.com/projectcalico/calicoctl/blob/v1.3.0/calico_node/startup/startup.go#L463

Because of this, we need to run two jobs that execute calicoctl manually to migrate on the new CIDR - one to create a new IPv4 pool that we want, and one to delete the existing IP pool that we no longer want. This is to be executed after executing one of the listed upgrade strategies:

# This ConfigMap is used to configure a self-hosted Calico installation.
kind: ConfigMap
apiVersion: v1
metadata:
  name: calico-config-ippool
  namespace: kube-system
data:
  # The default IP Pool to be created for the cluster.
  # Pod IP addresses will be assigned from this pool.
  ippool.yaml: |
      apiVersion: v1
      kind: ipPool
      metadata:
        cidr: 100.96.0.0/11
      spec:
        ipip:
          enabled: true
          mode: cross-subnet
        nat-outgoing: true
---
## This manifest deploys a Job which adds a new ippool to calico
apiVersion: batch/v1
kind: Job
metadata:
  name: configure-calico-ippool
  namespace: kube-system
  labels:
    k8s-app: calico
    role.kubernetes.io/networking: "1"
spec:
  template:
    metadata:
      name: configure-calico-ippool
      annotations:
        scheduler.alpha.kubernetes.io/critical-pod: ''
    spec:
      hostNetwork: true
      serviceAccountName: calico
      tolerations:
      - key: node-role.kubernetes.io/master
        effect: NoSchedule
      - key: CriticalAddonsOnly
        operator: Exists
      restartPolicy: OnFailure
      containers:
        # Writes basic configuration to datastore.
        - name: configure-calico
          image: calico/ctl:v1.2.1
          args:
          - apply
          - -f
          - /etc/config/calico/ippool.yaml
          volumeMounts:
            - name: config-volume
              mountPath: /etc/config
          env:
            # The location of the etcd cluster.
            - name: ETCD_ENDPOINTS
              valueFrom:
                configMapKeyRef:
                  name: calico-config
                  key: etcd_endpoints
      volumes:
       - name: config-volume
         configMap:
           name: calico-config-ippool
           items:
            - key: ippool.yaml
---
## This manifest deploys a Job which deletes the old ippool from calico
apiVersion: batch/v1
kind: Job
metadata:
  name: configure-calico-ippool
  namespace: kube-system
  labels:
    k8s-app: calico
    role.kubernetes.io/networking: "1"
spec:
  template:
    metadata:
      name: configure-calico-ippool
      annotations:
        scheduler.alpha.kubernetes.io/critical-pod: ''
    spec:
      hostNetwork: true
      serviceAccountName: calico
      tolerations:
      - key: node-role.kubernetes.io/master
        effect: NoSchedule
      - key: CriticalAddonsOnly
        operator: Exists
      restartPolicy: OnFailure
      containers:
        # Writes basic configuration to datastore.
        - name: configure-calico
          image: calico/ctl:v1.2.1
          args:
          - delete
          - ipPool
          - 100.64.0.0/10
          env:
            # The location of the etcd cluster.
            - name: ETCD_ENDPOINTS
              valueFrom:
                configMapKeyRef:
                  name: calico-config

By doing that, new Pods will get new IPs in the right range, and existing Pods with existing IPs continue to function.

Operations on k8s 1.5 using the same tests as above

• New Cluster Creation for k8s 1.5 - PASS
• Cluster Upgrade for k8s 1.5 - SAME ISSUE AS DESCRIBED ABOVE WITH K8S 1.6

---

This change is 

[k8s-ci-robot]

Hi @ottoyiu. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot 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.

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. I understand the commands that are listed here.

[chrislovecnm]

Comment for you

[chrislovecnm]

I think you will need 2.1.2-kops.1 not 2.1.1-kops.1

[chrislovecnm]

@k8s-bot ok to test

[justinsb]

Do we know what happens when we change this value?

[ottoyiu]

@justinsb: you mean when a user modifies the .KubeControllerManager.ClusterCIDR value, or you mean, what happens when we switch from .NonMasqueradeCIDR to .KubeControllerManager.ClusterCIDR?

[ottoyiu]

@justinb @chrislovecnm I have tested these changes. The upgrade/migration path for an existing cluster to the new non-overlapping CIDR requires a manual step (as shown in main PR notes) which to some can be seen as sub-optimal. However, it could also be seen as ideal, as this change will have zero effect on existing running clusters unless the manual step is ran. I would like to hear what you think is the proper approach in this case. I'm leaning towards just documenting the migration step for existing clusters if the cluster operator(s) deem the migration necessary.

[chrislovecnm]

@ottoyiu I think documenting and printing a warning on screen may be helpful for the user. Any other ides?

[chrislovecnm]

@ottoyiu can we get release notes like #2875

[chrislovecnm]

👍

[ottoyiu]

@chrislovecnm just came by from vacation. What's the proper way to contribute to a release note? Should I submit a PR on justinb's release notes branch?

[blakebarnett]

@ottoyiu See #3018 and #3019, need to probably turn off cross-subnet mode as the default or any users doing this migration will break their clusters.

[ottoyiu]

@blakebarnett will fix #3018 ASAP. Oversight on my part; my apologies. #3019 seems to be a much trickier problem to solve, since its tied with the calico addon version. Kops already thinks the calico has been upgraded... will document it in the ticket instead.