Allow make push to replace and relaunch Appliance components#7164
Allow make push to replace and relaunch Appliance components#7164gigawhitlocks wants to merge 9 commits intovmware:masterfrom gigawhitlocks:hackathon
make push to replace and relaunch Appliance components#7164Conversation
Makefile
Outdated
| test: install-govmomi portlayerapi $(TEST_JOBS) | ||
|
|
||
| push: | ||
| ./infra/scripts/replace-running-components.sh |
There was a problem hiding this comment.
Use $(BASE_DIR)//infra/scripts/replace-running-components.sh - I know it's not consistent throughout the Makefile currently but it should be.
|
|
||
| set -e | ||
|
|
||
| for x in $(echo GOVC_USERNAME GOVC_PASSWORD VCH_NAME GOVC_URL GOPATH GOVC_INSECURE); do |
There was a problem hiding this comment.
Use govc env to dump out the necessary fields and check them. This doesn't care how the user/password are supplied so is tolerant to more configurations.
| echo "GOVC_PASSWORD: password on ESX/vCenter target" | ||
| echo "VCH_NAME: name of VCH; matches --name argument for vic-machine" | ||
| echo "GOVC_URL: IP or FQDN of your vCenter/ESX target" | ||
| echo "GOPATH: your GOPATH, obviously" |
There was a problem hiding this comment.
Don't use GOPATH - it's not actually needed in what we're doing.
You can use the following to determine the directory of the script and then go relative from there:
BASE_DIR=$(dirname $(readlink -f "$BASH_SOURCE"))
Line 19 in b82d95f
| done | ||
|
|
||
| function get-thumbprint () { | ||
| openssl s_client -connect $GOVC_URL:443 </dev/null 2>/dev/null \ |
There was a problem hiding this comment.
check out govc about.cert --help, particularly govc about.cert --thumbprint
It removes the additional dependency on having openssl available (I know it's very common but I'd prefer to assume govc.
| | cut -d= -f2 | ||
| } | ||
|
|
||
| $GOPATH/src/github.com/vmware/vic/bin/vic-machine-linux debug --target=$GOVC_URL --name=$VCH_NAME --user=$GOVC_USERNAME --password=$GOVC_PASSWORD --thumbprint=$(get-thumbprint) |
There was a problem hiding this comment.
I recommend either explicitly specifying the compute-resource or using the --id parameter instead. This has the potential for name conflicts if in VC
| on-vch mv /tmp/$1 /sbin/$1 | ||
| on-vch chmod 755 /sbin/$1 | ||
| pid=$(on-vch ps -e --format='pid,args' | grep $1 | grep -v grep | awk '{print $1}') | ||
| on-vch kill -9 $pid |
There was a problem hiding this comment.
It's not in yet, but I expect to merge #6943 soon. That moves to providing for a clean shutdown via SIGTERM. Just a note that we should update this in whichever merges second.
| replace-component vic-init | ||
| else | ||
| replace-component $1 | ||
| replace-component vic-init |
There was a problem hiding this comment.
This should not be necessary - I tried it on the flight for other reasons. It takes a couple of seconds due to the throttling but it happily relaunched the docker-personality for me.
We should dig into what's actually occurring and not mask it with this.
| replace-component vic-init | ||
| fi | ||
|
|
||
| on-vch vic-init & |
There was a problem hiding this comment.
Likewise the systemd unit should be configured for restart if it's not currently which would negate the need for an explicit launch.
|
Thanks for the feedback @hickeng I'll clean this up hackathon code: all the shortcuts LOL |
|
OK @hickeng that should address all of your feedback except the systemd unit (which I'll take care of in the morning when I'm warming up before I get back on the tar issue again) and also I did a bit of cleanup/organization/polish, took some liberties with allowing the user to provide the VCH name instead of the ID and as long as it comes back unique it will figure out the ID and continue, but if the ID is provided it'll just use that instead, and adjusted the error handling to be a bit better. |
| VIC_ID=$($VIC_DIR/bin/vic-machine-linux ls --target=$target --user=$username --password=$password --thumbprint=$(get-thumbprint) | grep $VIC_NAME | awk '{print $1}') | ||
| fi | ||
|
|
||
| echo "Enabling SSH access on your VCH" |
| exit 1 | ||
| fi | ||
|
|
||
| username=$(govc env | grep GOVC_USERNAME | cut -d= -f2) |
There was a problem hiding this comment.
This will be problem for those of us who define GOVC_URL="user:pass@targeturl". We could try to grab user/pass from GOVC_URL if GOVC_USERNAME and GOVC_PASSWORD are not defined, but I see you do echo usage info explaining this.
There was a problem hiding this comment.
govc env should break those out already if you only assign GOVC_URL so this should be safe from this concern. By using govc env it should always have all of the required fields for GOVC populated for consumption, which should avoid a "something isn't set right" style of error.
There was a problem hiding this comment.
oh, interesting. I didn't know about this option.
|
ahahaha i messed up my repo and need to close and re-open this, sorry |
5 participants
Hackathon task