-
Notifications
You must be signed in to change notification settings - Fork 799
fix(contrib/vagrant): fix vagrant n-node Makefile #759
Conversation
LGTM. |
|
||
run: install restart | ||
run: install start | ||
|
||
clean: uninstall | ||
$(call ssh_all,'cd share && for c in $(COMPONENTS); do docker rm -f deis-$$c; done') |
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 cd share
here is fluff and is not required. Same goes for full-clean
.
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.
How is it fluff? It's pulling the images before we try starting services, so people aren't sitting around waiting and wondering why deis isn't running.
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 command itself isn't fluff, but cd share
is not required for make clean
to run. It just removes the container from docker with no reliance on what directory it's running under.
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.
Ah, gotcha. Yep.
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.
FWIW those were just copied from the main Makefile, so they should be fixed there as well.
The fleetctl commands weren't valid, and we should disable host checking, as Vagrant nodes will change frequently.
@bacongobbler made those changes. |
fix(contrib/vagrant): fix vagrant n-node Makefile
The fleetctl commands weren't valid, and we should disable host
checking, as Vagrant nodes will change frequently.