Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create an APB for the automation broker deployment #1

Merged
merged 17 commits into from Apr 11, 2018

Conversation

djzager
Copy link
Contributor

@djzager djzager commented Apr 2, 2018

No description provided.

resources: ["networkpolicies"]
verbs: ["create", "delete"]
- apiGroups: ["automationbroker.io"]
# attributeRestrictions: null
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These aren't actually supported in the python-client. What happens if we don't specifically set them to null?

Choose a reason for hiding this comment

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

I don't think anything happens

- Only create the broker auth secret if basic auth is enabled
- Flatten the variables in defaults to make it easy for a user to modify
  them
- Use jinja templating more heavily in configmap
- Push crd logic into it's own tasks file
- Push variables the user shouldn't touch into vars/
@djzager djzager changed the title [wip] Create an APB for the automation broker deployment Create an APB for the automation broker deployment Apr 4, 2018
name: {{ broker_name }}-auth-secret
namespace: {{ broker_namespace }}
data:
username: {{ broker_basic_auth_username | b64encode }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figured @fabianvf would appreciate this.

@djzager
Copy link
Contributor Author

djzager commented Apr 6, 2018

Example run, in OpenShift:

docker run --rm --net=host -v $HOME/.kube:/opt/apb/.kube:z -u $UID \
docker.io/djzager/automation-broker-apb \
provision \
-e 'cluster=openshift' \
-e 'create_broker_namespace=true' \
-e 'broker_helm_enabled=true'

Copy link

@dymurray dymurray left a comment

Choose a reason for hiding this comment

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

Ran into an error testing with the readme instructions.

TASK [automation-broker-apb : Set broker deployment object state=present] ******
fatal: [localhost]: FAILED! => {"changed": false, "msg": "Error parsing resource definition. Encountered spec_selector_app, which does not map to a parameter expected by the OpenShift Python module."}
        to retry, use: --limit @/opt/apb/actions/provision.retry

PLAY RECAP *********************************************************************

$ kubectl create namespace automation-broker
$ kubectl create serviceaccount automation-broker-apb --namespace automation-broker
$ kubectl create clusterrolebinding automation-broker-apb --clusterrole=cluster-admin --serviceaccount=automation-broker:automation-broker-apb
$ kubectl run automation-broker-apb \
Copy link

Choose a reason for hiding this comment

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

These commands worked with oc 👍

@dymurray
Copy link

dymurray commented Apr 6, 2018

Ah... my error was probably because I didn't set the cluster var. Retrying.

@dymurray
Copy link

dymurray commented Apr 6, 2018

changed: [localhost] => (item=broker.configmap.yaml)
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: TypeError: __init__() got an unexpected keyword argument 'apiGroups'
failed: [localhost] (item=broker-auth.clusterrole.yaml) => {"changed": false, "item": "broker-auth.clusterrole.yaml", "module_stderr": "Traceback (most recent call last):\n  File \"/tmp/ansible_0SKNA_/ansible_module_k8s_raw.py\", line 163, in <module>\n    main()\n  File \"/tmp/ansible_0SKNA_/ansible_module_k8s_raw.py\", line 159, in main\n    KubernetesRawModule().execute_module()\n  File \"/tmp/ansible_0SKNA_/ansible_modlib.zip/ansible/module_utils/k8s/raw.py\", line 142, in execute_module\n  File \"/tmp/ansible_0SKNA_/ansible_modlib.zip/ansible/module_utils/k8s/helper.py\", line 136, in object_from_params\n  File \"/tmp/ansible_0SKNA_/ansible_modlib.zip/ansible/module_utils/k8s/helper.py\", line 293, in __set_obj_attribute\n  File \"/tmp/ansible_0SKNA_/ansible_modlib.zip/ansible/module_utils/k8s/helper.py\", line 483, in __compare_obj_list\nTypeError: __init__() got an unexpected keyword argument 'apiGroups'\n", "module_stdout": "", "msg": "MODULE FAILURE", "rc": 1}

@djzager
Copy link
Contributor Author

djzager commented Apr 6, 2018

There are a few issues with k8s that I need to work through. Specifically, creating the tls stuff.

@djzager
Copy link
Contributor Author

djzager commented Apr 9, 2018

@dymurray I think this is cleaned up. Let me know if you have any more issues. @rthallisey if you could have a look at this from the k8s perspective that would be really helpful.

-out /tmp/{{ broker_name }}-cert/cert.pem
-days 365
-subj "/CN={{ broker_name }}.{{ broker_name }}.svc"
when: cluster == 'kubernetes' and apb_action == 'provision'

Choose a reason for hiding this comment

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

👍

file:
path: /tmp/{{ broker_name }}-cert
state: directory
when: cluster == 'kubernetes' and apb_action == 'provision'

Choose a reason for hiding this comment

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

I think it's cleaner to encapsulate tasks that share a condition in a block or import task. Also prevents you from missing a conditional update somewhere if something changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I can definitely improve this file and the OpenShift file to encapsulate better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe I fixed it with the latest commit @fabianvf let me know what you think.

resource_name=broker_name + '-client'
) | json_query('secrets') | first | json_query('name')
}}"
when: cluster == 'kubernetes'

Choose a reason for hiding this comment

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

not to nitpick, but for large blocks like these I like to put the when at the top (like after name), because the whitespace can be kind of hard to parse.

# privatekey_path: /tmp/{{ broker_name }}-cert/key.pem
# subject: "/CN={{ broker_name }}.{{ broker_name }}.svc"
# provider: selfsigned
- name: 'Create OpenSSL Cert for Broker'

Choose a reason for hiding this comment

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

just FYI I don't think the task name is displayed in the output for blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I get for trusting the example 😎 http://docs.ansible.com/ansible/latest/user_guide/playbooks_blocks.html#id1

@@ -0,0 +1,155 @@
---

Choose a reason for hiding this comment

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

Since this isn't a template it might make more sense to put it in files/

Copy link

@fabianvf fabianvf left a comment

Choose a reason for hiding this comment

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

Added some stylistic comments, but none of them are blockers or even very important.

@djzager djzager merged commit b2f9d42 into automationbroker:master Apr 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants