Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

If CM's purge_node() fails then raise an error. #310

Merged
merged 1 commit into from
Nov 7, 2013

Conversation

tombh
Copy link
Contributor

@tombh tombh commented Nov 7, 2013

Just provides some helpful logging.

BTW how does everyone else add their Deis Controller client to the Chef admin group? I thought you could do it on the command line with knife client edit deis-controller and then change the admin key to 'true'. But I'm wondering if that key even refers to the admin group? Do you actually have to go into the Chef Server web UI and edit the admin group there?

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling f0fd45f on tombh:purge-node-feedback into b062fe6 on opdemand:master.

@gabrtv
Copy link
Member

gabrtv commented Nov 7, 2013

@tombh this PR fixes #243. Nice one!

Re: deis-controller, we were thinking about removing the admin perms requirement altogether. I've been talking with the Opscode guys about a more granular way to add permissions. The controller doesn't actually need full admin, it just needs the ability to purge node/client records that it created. We could likely use their undocumented RBAC endpoints to do that as part of the bootstrap process.

In the meantime, if there's an easy CLI one-liner for adding admin perms to deis-controller, we should include that in the setup docs so people can skip the web UI.

gabrtv pushed a commit that referenced this pull request Nov 7, 2013
If CM's purge_node() fails then raise an error.
@gabrtv gabrtv merged commit fd0faba into deis:master Nov 7, 2013
@tombh
Copy link
Contributor Author

tombh commented Nov 7, 2013

Oh cool! Two birds with one stone :)

@gabrtv That more granular approach to perms sounds interesting. At the moment I just don't think there is a a CLI one-liner, though I'm still looking. Either way I'll certainly include the process in the README for Vagrant.

@tombh tombh deleted the purge-node-feedback branch November 7, 2013 17:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants