-
Notifications
You must be signed in to change notification settings - Fork 29
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
Fix small bugs and add pre- and post-hooks to some roles #175
Conversation
Can one of the admins verify this patch? |
Okay, I also cleaned up some inconsistencies with the startup logic for minishift, and I added some pre- and post-hooks for some of the roles. That way, I can inject the CA cert into minishift when its done being setup but before CVP is deployed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look pretty good apart from a couple of nits - and since the PR has grown to include more structural changes (additional hooks), I'd ask for the commit and PR titles to be changed to reflect that.
I updated the PR with the requested changes, and also updated the title and description of the PR to reflect the scope of the changes. |
5fc01ed
to
cdefe88
Compare
Reworded commit message, and squashed all commits into one. |
cdefe88
to
95744f3
Compare
…e startup logic, and added some pre- and post-hooks to some of the roles.
95744f3
to
fd2ddf1
Compare
Added some more tweaks, and squashed. |
@dirgim Bump |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I just noticed this in the output:
You can see in the
Cleanup /home/wlaw/.contra-env-setup/pipeline
task, it is prepending the path with another path, making the path invalid, so it is skipped.Also as a part of this MR, the behavior around
start_minishift
was made more consistent, since how that variable was used depended on whether minishift was being provisioned locally or on a remote VM. Now it is more consistent, in that thestart_minishift
variable is respected regardless if the VM is local or remote. The default value forstart_minishift
was changed totrue
, since it is being started anyway by theos_temps
playbook.Finally, pre- and post-hooks were added to some of the playbooks to support adding in custom behavior by any consumers of this project.