-
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
Create new minishift profile in case it was never created before #67
Create new minishift profile in case it was never created before #67
Conversation
740c19a
to
775e1dc
Compare
@arilivigni or @dirgim could you please take a quick look? |
@@ -58,4 +58,4 @@ | |||
|
|||
# Initialize minishift | |||
- import_tasks: init_minishift.yml | |||
when: minishift_exist.stdout != "" | |||
when: (minishift_profile_status.stdout|lower == "does not exist") or minishift_profile_status.rc != 0 |
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.
why parens around one and not the other if you are going to use them?
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.
Let me fix that.
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.
I kept those parens in as the conditional seems easier to read then (but I can also remove it if preferred).
There are two similar cases, which both need to be covered: 1) `minishift status --profile <name>` will return "Does not exist", in case someone already configured the profile with `minishift profile set <name>`(or in case it got deleted with `minishift delete`). 2) `minishift status --profile <name>` will return error saying the profile doesn't exist at all (e.g. this is the first time it is being used and queried for). `minishift start` will then create that profile. This change brings support for case 2), by checking the result code of the `minishift status` call.
775e1dc
to
f59a330
Compare
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
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.
Not really sure why this is needed. Do you have a failure to show where this happens that this is needed?
If you run That specific profile was never used, so minishift is not aware of that name. One other alternative would be to call |
@psiroky gotcha your approach is fine then, just one nit on the or in parens that I made a comment on. |
@arilivigni so I added the parens to the right side of the |
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
There are two similar cases, which both need to be covered:
minishift status --profile <name>
will return "Does not exist",in case someone already configured the profile with
minishift profile set <name>
(or in case it got deleted withminishift delete
).minishift status --profile <name>
will return error saying theprofile doesn't exist at all (e.g. this is the first time it is being
used and queried for).
minishift start
will then create thatprofile.
This change brings support for case 2), by checking the result code of
the
minishift status
call.