sostheim / krak8s

API Service for Kraken and Kubernetes Commands
Apache License 2.0
1 stars 5 forks source link

Update to latest configuration file format for Kraken config #16

Open sostheim opened 6 years ago

sostheim commented 6 years ago

implement required changes to kraken config.yaml

sostheim commented 6 years ago

Change taints The method by which taints were added to nodepools was a hack to work around a deficiency in kraken-lib at the time. That problem has since been fixed. Change nodepool definitions with taints per the following:

Original:

        - name: forayNodes
          count: 5
          kubeConfig: *colonybeadKube
          containerConfig: *defaultDocker
          osConfig: *defaultCoreOs
          nodeConfig: 
            << : *defaultAwsClusterNode
            taints:
              - key: customer
                value: foray
                effect: NoSchedule
          keyPair: *colonybeadKeyPair

New:

       - name: forayNodes
          count: 5
          kubeConfig: *colonybeadKube
          containerConfig: *defaultDocker
          osConfig: *defaultCoreOs
          nodeConfig: *defaultAwsClusterNode
          taints:
            - key: customer
              value: foray
              effect: NoSchedule
          keyPair: *colonybeadKeyPair
sostheim commented 6 years ago

/CC @joejulian @davidewatson

davidewatson commented 6 years ago

/lgtm

davidewatson commented 6 years ago

Hold on. My comment above was based on the schema change @joejulian made in kraken-lib/1026. However, a quick glance at this code:

---
- set_fact:
    taints: "{{ new_taints }}"
    taints_exist: "{{ new_taints | length > 0 | bool }}"
  vars:
    nodePoolTaintQuery: "{{ node | json_query('schedulingConfig.taints[*]') }}"
    nodePoolTaints: "{{ (nodePoolTaintQuery == '') | ternary([],nodePoolTaintQuery) }}"
    nodeTypeTaintQuery: "{{ node | json_query('nodeConfig.taints[*]') }}"
    nodeTypeTaints: "{{ (nodeTypeTaintQuery == '') | ternary([],nodeTypeTaintQuery) }}"
    new_taints: "{{ nodePoolTaints + nodeTypeTaints }}"

makes me wonder where the nodePool level taints are picked up in ansible.

I haven't looked far, so maybe everything is fine. We obviously need to test this though.

sostheim commented 6 years ago

@davidewatson - holding.

davidewatson commented 6 years ago

Yeah, this won't work. Using the config file here:

        - name: forayNodes
          count: 5
          kubeConfig: *colonybeadKube
          containerConfig: *defaultDocker
          osConfig: *defaultCoreOs
          nodeConfig: *defaultAwsClusterNode
          keyPair: *colonybeadKeyPair
          taints:
            - key: customer
              value: foray
              effect: NoSchedule

I end up with no taints:

Name:           ip-10-0-104-151.ec2.internal
Role:           
Labels:         beta.kubernetes.io/arch=amd64
            beta.kubernetes.io/instance-type=m4.2xlarge
            beta.kubernetes.io/os=linux
            failure-domain.beta.kubernetes.io/region=us-east-1
            failure-domain.beta.kubernetes.io/zone=us-east-1b
            kubernetes.io/hostname=ip-10-0-104-151.ec2.internal
            nodepool=forayNodes
Annotations:        node.alpha.kubernetes.io/ttl=0
            volumes.kubernetes.io/controller-managed-attach-detach=true
Taints:         <none>
CreationTimestamp:  Thu, 21 Dec 2017 07:52:57 -0800

@joejulian: ^^

joejulian commented 6 years ago

Yep, my failure to read.

Looks like it should be

        - name: forayNodes
          count: 5
          kubeConfig: *colonybeadKube
          containerConfig: *defaultDocker
          osConfig: *defaultCoreOs
          nodeConfig: *defaultAwsClusterNode
          keyPair: *colonybeadKeyPair
          schedulingConfig:
            taints:
              - key: customer
                value: foray
                effect: NoSchedule

This passes the schema check but I don't know if it produces the taints.

sostheim commented 6 years ago

17 merged.