Add playbooks to provision AWS VMs for CI #9744

vrutkovs · 2018-08-24T09:33:40Z

This PR adds several simple playbooks to create AWS VMs, create inventory file from a template and deprovision VMs. See test/ci/README.md update for a suggested workflow.

These playbooks would later on be used on Prow CI similar to GCP. We can't use playbooks/aws/ for this as an average dev account can't create ELBs, launch configs etc.

vars.yml is able to define custom AMIs, so Atomic would be supported - its also able to deploy more complex sets of VMs, e.g. LB - 3 masters - 2 infra nodes - 2 compute nodes.

Cluster config in test/ci/inventory/group_vars/OSEv3 is pretty minimal and requires two env vars to be set:

RPM_REPO to be set in openshift_additional_repos
IMAGE_FORMAT for oreg_url

TODO:

Rework launch.yml to run prerequisites and deprovision automatically
Currently settings from a temporary inventory are not including test/ci/inventory/group_vars, so a simple include_playbook is insufficient.
Ensure all deps are present in openshift/origin-ansible docker image to make this work
Move extravars file to host_vars/localhost
Reworked this to use test/ci/vars.yml instead
Rework provision and deprovision to avoid templating the inventory file - remove machines based on clusterid tag
We still need it to run upgrades until we switch to dynamic enventory
Set volumes list for each host in the group
Refactor AMI search

TBD:

Require unique cluster ID and use it to deprovision instances instead of inventory?
Add a directory for custom cluster vars?
This would allow us to run tests with per-PR settings (for instance, enable ASB temporary)

vrutkovs · 2018-08-24T09:34:22Z

test/ci/create_aws_vm.yml

+    assign_public_ip: yes
+    instance_tags:
+      Name: "{{ outer_item.name_template }}-{{ item }}"
+      "kubernetes.io/cluster/{{ aws_cluster_id }}": "true"


TODO: This needs fixing, as this var doesn't get templated for some reason

@vrutkovs i suspect the reason for that is because is not a json. What i ended up doing internally for same thing was:

- name: Set ec2_tags fact for EC2 instances set_fact: ec2_tags: | { "kubernetes.io/cluster/{{ openshift_clusterid }}": "true", "Name": "{{ name }}" }

followed by

- name: Add tags to {{ name }} instance ec2_tag: region: "{{ aws_region }}" resource: "{{ instance_id }}" state: present tags: "{{ ec2_tags }}"

hth

also i believe you want to change the tag's value to one of the 2 values https://github.com/kubernetes/kubernetes/blob/master/pkg/cloudprovider/providers/aws/tags.go#L43-L51

Yeah, that did the trick, thanks!

Not sure if we care about owned vs. shared, might add this later

DanyC97 · 2018-08-24T16:31:00Z

test/ci/create_aws_vm.yml

+    region: "{{ aws_region }}"
+  when: is_atomic
+  with_items: "{{ ec2.results }}"
+


i suspect you do need to tag your volumes with same kubernetes.io/cluster/ key and value above

you want this especially if you planning to use PVs ... just saying

This is a volume for Atomic's docker storage, it doesn't know about clusters and such. In any case it seems the bot on our instance is deleting those correctly without any tags

okay, fair point

DanyC97 · 2018-08-30T10:34:34Z

test/ci/template-inventory.j2

+{{ entry }} ansible_host='{{ hostvars[entry]['ansible_host'] }}' aws_id='{{ hostvars[entry]['aws_id'] }}' {{ addon_opts }}
+{% endfor %}
+{% endif %}
+


nit: you may want to have a nice generated ini file and as such you could do with a bit of trimming the whitespace - http://jinja.pocoo.org/docs/dev/templates/#whitespace-control

Yeah, I had this initially and it turned into a nightmare :(

This inv is temporary anyway, all it needs is the hostnames and host-specific params (just one actually) so I wouldn't care about it much

michaelgugino · 2018-08-30T19:49:46Z

test/ci/launch.yml

+        instance_tags: "{{ aws_instance_tags }}"
+        volumes: "{{ item.aws_volumes | default(omit) }}"
+      register: ec2
+      with_items: "{{ vms }}"


aws_instances?

michaelgugino · 2018-08-30T19:50:59Z

test/ci/launch.yml

+        aws_region: "{{ aws_region }}"
+        aws_ip: "{{ item.instances.0.public_ip }}"
+        aws_id: "{{ item.instances.0.id }}"
+        node_group: "{{ item.instances.0.tags['ansible-node-group'] }}"


I don't think 'node_group' is the correct variable name here. openshift_node_group_name ?

Fixed. This incorrectly set for in-memory inventory, but was correctly rendered in hosts/inventory

michaelgugino · 2018-08-30T19:55:09Z

test/ci/vars.yml.sample

+    - nodes
+  aws_flavor: t2.large
+  aws_security_group: public
+  node_group: "node-config-all-in-one"


This could be better handled by group vars.

Possible inventory groups:

[OSEv3:children] etcd nodes [nodes:children] masters compute [etcd:children] masters [masters] master1.example.com [master:vars] openshift_node_group_name: node-config-master-infra [compute] node1.example.com [compute:vars] openshift_node_group_name: node-config-compute

Obviously, we can define vars sections in appropriate group_vars files and we can adjust groups for different scenarios.

That'd complicate vars.yml - a list of groups to create and assign the instance to the group? In the end I'd still have to set per-instance aws_ip and aws_id, so why not put openshift_node_group_name there too?

That'd complicate vars.yml - a list of groups to create and assign the instance to the group? In the end I'd still have to set per-instance aws_ip and aws_id, so why not put openshift_node_group_name there too?

I'm not sure what you mean. You don't need to 'create' the groups, you just make this the base template of the inventory file (since we're not using dynamic inventory).

That'd limit us in groups choice. Instead of hardcoding the list of those in the template these groups are now being created automagically. So later on we could try scale up tests, by adding hosts in new_nodes group for instance.

I'm not a fan of this implementation, I do think we need to update our group reference architecture to reflect what an actual user should do as far as group hierarchy. I'm against using hostvars where we could use groupvars because the syntax is different between the two in ini files and users never seem to understand what hostvars actually are (you'll see inventories with same host in two groups, eg masters and nodes, and competing values set on each instance of the host).

In any case, we can revisit this later.

michaelgugino · 2018-09-05T14:12:04Z

test/ci/launch.yml

+  connection: local
+  gather_facts: false
+  tasks:
+    - include_vars: "{{ item }}"


Are we saying that there should be a vars.yaml on the host that is running ansible that supplies some variables? This file is provided by CI? Let's add a comment if so.

michaelgugino · 2018-09-05T14:16:20Z

test/ci/vars.yml.sample

+    - nodes
+  aws_flavor: t2.large
+  aws_security_group: public
+  node_group: "node-config-all-in-one"


That'd complicate vars.yml - a list of groups to create and assign the instance to the group? In the end I'd still have to set per-instance aws_ip and aws_id, so why not put openshift_node_group_name there too?

I'm not sure what you mean. You don't need to 'create' the groups, you just make this the base template of the inventory file (since we're not using dynamic inventory).

michaelgugino

/lgtm

michaelgugino · 2018-09-05T15:18:32Z

test/ci/vars.yml.sample

+    - nodes
+  aws_flavor: t2.large
+  aws_security_group: public
+  node_group: "node-config-all-in-one"


I'm not a fan of this implementation, I do think we need to update our group reference architecture to reflect what an actual user should do as far as group hierarchy. I'm against using hostvars where we could use groupvars because the syntax is different between the two in ini files and users never seem to understand what hostvars actually are (you'll see inventories with same host in two groups, eg masters and nodes, and competing values set on each instance of the host).

In any case, we can revisit this later.

openshift-ci-robot · 2018-09-05T15:18:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: michaelgugino, vrutkovs

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [michaelgugino,vrutkovs]

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

vrutkovs · 2018-09-06T08:24:51Z

/cherrypick release-3.10

openshift-cherrypick-robot · 2018-09-06T08:25:54Z

@vrutkovs: new pull request created: #9936

Details

In response to this:

/cherrypick release-3.10

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.

openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 24, 2018

openshift-ci-robot requested review from mtnbikenc and sdodson August 24, 2018 09:33

vrutkovs requested a review from michaelgugino August 24, 2018 09:33

vrutkovs commented Aug 24, 2018

View reviewed changes

vrutkovs force-pushed the prow-aws-playbooks branch from 4852912 to ae4ed43 Compare August 24, 2018 09:45

vrutkovs changed the title ~~Add playbooks to provision AWS VMs for CI~~ WIP Add playbooks to provision AWS VMs for CI Aug 24, 2018

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 24, 2018

vrutkovs force-pushed the prow-aws-playbooks branch 10 times, most recently from 103727f to a3ddb54 Compare August 24, 2018 16:22

DanyC97 reviewed Aug 24, 2018

View reviewed changes

vrutkovs force-pushed the prow-aws-playbooks branch 9 times, most recently from babbd68 to c57dad8 Compare August 27, 2018 08:17

vrutkovs changed the title ~~WIP Add playbooks to provision AWS VMs for CI~~ Add playbooks to provision AWS VMs for CI Aug 27, 2018

vrutkovs force-pushed the prow-aws-playbooks branch 2 times, most recently from 6130efd to 4475ffc Compare August 30, 2018 09:19

DanyC97 reviewed Aug 30, 2018

View reviewed changes

openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 30, 2018

vrutkovs changed the title ~~WIP Add playbooks to provision AWS VMs for CI~~ Add playbooks to provision AWS VMs for CI Aug 30, 2018

openshift-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 Aug 30, 2018

vrutkovs force-pushed the prow-aws-playbooks branch from 1e51abd to 85251f9 Compare August 30, 2018 17:52

michaelgugino suggested changes Aug 30, 2018

View reviewed changes

vrutkovs force-pushed the prow-aws-playbooks branch 7 times, most recently from fa28be3 to 772af74 Compare August 31, 2018 10:29

Rework test CI

6446f05

vrutkovs force-pushed the prow-aws-playbooks branch from 772af74 to 6446f05 Compare August 31, 2018 12:33

michaelgugino suggested changes Sep 5, 2018

View reviewed changes

michaelgugino approved these changes Sep 5, 2018

View reviewed changes

openshift-ci-robot assigned michaelgugino Sep 5, 2018

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 5, 2018

openshift-merge-robot merged commit 5b57dd2 into openshift:master Sep 5, 2018

vrutkovs mentioned this pull request Sep 6, 2018

Openshift ansible: add runs on AWS CI openshift/release#1367

Merged

openshift-cherrypick-robot mentioned this pull request Sep 6, 2018

[release-3.10] Add playbooks to provision AWS VMs for CI #9936

Closed

vrutkovs mentioned this pull request Sep 21, 2018

[3.10] - add test/ci playbooks #10186

Merged

vrutkovs deleted the prow-aws-playbooks branch January 5, 2019 13:14

Add playbooks to provision AWS VMs for CI #9744

Add playbooks to provision AWS VMs for CI #9744

Uh oh!

Conversation

vrutkovs commented Aug 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanyC97 Aug 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michaelgugino left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Sep 5, 2018

Uh oh!

vrutkovs commented Sep 6, 2018

Uh oh!

openshift-cherrypick-robot commented Sep 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

vrutkovs commented Aug 24, 2018 •

edited

Loading

DanyC97 Aug 24, 2018 •

edited

Loading