[skip-ci] Allow `make push` to replace and relaunch Appliance components by gigawhitlocks · Pull Request #7233 · vmware/vic

gigawhitlocks · 2018-02-01T19:04:20Z

Continuing from #7164 sorry, comments from that PR should be addressed at this time, except for a small modification to systemd in the appliance, which I'll have to modify and test. Resolves #7167

sgairo · 2018-02-01T20:00:07Z

I think its [skip ci] for future reference

hickeng · 2018-02-02T00:24:37Z

@gigawhitlocks I think this comment may have been meant for this PR: #7170 (comment)

If it's relevant, please note. If not I've not idea who I was writing that for.

vburenin

Can't wait to get it in!

hickeng · 2018-02-05T22:23:03Z

infra/scripts/replace-running-components.sh

+
+# Grab vSphere's thumbprint for calling vic-machine
+function get-thumbprint () {
+    govc about.cert | grep "SHA-1 Thumbprint" | awk '{print $NF}'


govc about.cert -k -thumbprint gives you the thumbprint undercoated so you don't need the grep/awk.

still prints the IP so I do need awk, but not grep

hickeng · 2018-02-05T22:24:25Z

infra/scripts/replace-running-components.sh

+
+# Run the command given on the VCH instead of locally
+function on-vch() {
+    sshpass -ppassword ssh -oUserKnownHostsFile=/dev/null -oStrictHostKeyChecking=no root@$VCH_IP -C $@ 2>/dev/null


could we use the vic-machine debug --authorized-key option?
It's a path I don't think we test sufficiently but would also avoid hard coding dev passwords into the scripts.

hickeng · 2018-02-05T22:27:25Z

infra/scripts/replace-running-components.sh

+
+
+if [[ ! -v VIC_ID ]] && [[ $(vch-name-is-ambiguous) ]]; then
+    echo "The provided VIC name is ambiguous; please choose the correct VCH ID from the output below and assign it to the environment variable VIC_ID, e.g., export VIC_ID=12"


Check if the script is interactive - if it is then use read -p "Enter VCH ID: " VIC_ID to get the variable from the user.

From a consistency perspective I suggest renaming that variable to be VCH_ID instead of VIC_ID

hickeng · 2018-02-05T22:33:40Z

infra/scripts/replace-running-components.sh

+    done
+fi
+
+on-vch vic-init &


This really shouldn't be necessary unless configured at a debug level that causes vic-init to exit instead of relaunching components (think that's debug>=3)

I'm not sure what will happen here, but there is a risk of multiple vic-init's running - I don't know that there's anything that prevents that.

I'd suggest using systemctl restart vic-init.service to perform a restart if necessary.
I'd also ask you to follow up on why the restart is needed.

…mment

gigawhitlocks · 2018-02-06T22:46:56Z

Apologies @hickeng I moved everything around and made a bunch of improvements, in addition to addressing your concerns

hickeng · 2018-02-06T23:21:11Z

infra/scripts/replace-running-components.sh

+    govc about.cert -k -thumbprint | awk '{print $NF}'
+}
+
+# SCPs the component in $1 to the VCH, plops it in place, and brutally kills the previous running process


This this comment is intended for replace-component

hickeng · 2018-02-06T23:23:36Z

infra/scripts/replace-running-components.sh

+    scp -oUserKnownHostsFile=/dev/null -oStrictHostKeyChecking=no $VIC_DIR/bin/$1 root@$VCH_IP:/tmp/$1 2>/dev/null
+    on-vch mv /sbin/$1 /tmp/old-$1
+    on-vch mv /tmp/$1 /sbin/$1
+    on-vch chmod 755 /sbin/$1


suggest doing this first before the mv /sbin/$1 /tmp/old-$1 - that way there's no window in which vic-init will try and fail to execute it.

hickeng · 2018-02-06T23:26:00Z

infra/scripts/replace-running-components.sh

+
+function replace-components () {
+    echo "Killing vic-init (and not taking no for an answer)..."
+    on-vch systemctl kill vic-init


still wanting an answer as to why this is necessary - vic-init should be restarting dead components.
If you still observe that not occurring with debug <= 1 please raise an issue for it with /var/log/vic/init.log attached that showcases the failure to restart.

hickeng · 2018-02-06T23:26:52Z

infra/scripts/replace-running-components.sh

+}
+
+function get-thumbprint() {
+    govc about.cert -k -thumbprint | awk '{print $NF}'


might want pipefail and or $? check for govc.

$ govc about.cert -k -thumbprint | awk '{print $NF}' govc: dial tcp 192.168.78.128:443: getsockopt: no route to host

strictly speaking govc ls is called first in the script and we'll have failed by the time we do a govc about.cert if govc can't communicate, but I think pipefail should just be on in the whole script, I was already thinking about that (enabled pipefail in the most recent commit as of time of writing)

…ents

gigawhitlocks · 2018-02-07T17:07:41Z

@mhagen-vmware apparently you have dominion over infra/scripts

mhagen-vmware

lgtm

hickeng · 2018-02-07T20:59:21Z

infra/scripts/replace-running-components.sh

+        replace-component $x
+    done
+
+    # George swears you don't need to do this but if you don't, the new binaries are not launched


George still swears you don't need to do this and wants logs from it not happening so we can track down WHY.
As written in this PR it's likely because the component processes haven't been killed after the binary is replaced.

hickeng · 2018-02-07T21:02:09Z

infra/scripts/replace-running-components.sh

+    if [[ $1 == "vic-init" ]]; then
+        on-vch systemctl restart vic-init
+    else
+        on-vch kill -9 $pid


When #6943 can you remind me that it should update this to use kill -TERM. That will cause vSphere sessions to be cleaned up properly

hickeng · 2018-02-07T21:03:41Z

infra/scripts/replace-running-components.sh

+    on-vch chmod 755 /tmp/$1
+    on-vch mv /tmp/$1 /sbin/$1
+    if [[ $1 == "vic-init" ]]; then
+        on-vch systemctl restart vic-init


We might need to look at the effect of this with #6943 - it's possible that this would cause all components to be shutdown as well which is not what we're after.

vmwclabot added the cla-not-required label Feb 1, 2018

gigawhitlocks changed the title ~~Allow make push to replace and relaunch Appliance components~~ [skip-ci] Allow make push to replace and relaunch Appliance components Feb 1, 2018

vburenin approved these changes Feb 2, 2018

View reviewed changes

hickeng reviewed Feb 5, 2018

View reviewed changes

Ian Whitlock added 12 commits February 6, 2018 16:46

Use make push to update binaries on the appliance

09a2779

This version should work though

ac39797

Add some nice messages

aefaca9

Copyright header

bdd07ab

Allow replacing single components

a0b8e3a

Restart vic-init always

03b26e3

Refactor/cleanup and address most PR concerns

32e2e31

Cleanup, refactor, and address all the comments except the systemd co…

fe37c6d

…mment

Remove debugging statements

7f4c436

vic-init auto-restart

5f2d54c

Formatting and cleanup

21add8d

Control vic-init with systemd

770419a

gigawhitlocks force-pushed the hackathon branch from 33b00ca to 770419a Compare February 6, 2018 22:46

Ian Whitlock added 2 commits February 6, 2018 16:50

Delete commented-out code

2ec8099

Actually we don't need to modify this service file anymore

b509103

hickeng reviewed Feb 6, 2018

View reviewed changes

Ian Whitlock added 4 commits February 6, 2018 17:55

Address comments, remove useless steps, argue with George in the comm…

cf1c264

…ents

Pipefail

7fa9583

Change message on success and finally get George's suggestion working

8051133

Merge branch 'master' into hackathon

b80ba27

mhagen-vmware approved these changes Feb 7, 2018

View reviewed changes

matthewavery approved these changes Feb 7, 2018

View reviewed changes

hickeng reviewed Feb 7, 2018

View reviewed changes

hickeng approved these changes Feb 7, 2018

View reviewed changes

gigawhitlocks merged commit f67dd8b into vmware:master Feb 7, 2018



		if [[ ! -v VIC_ID ]] && [[ $(vch-name-is-ambiguous) ]]; then
		echo "The provided VIC name is ambiguous; please choose the correct VCH ID from the output below and assign it to the environment variable VIC_ID, e.g., export VIC_ID=12"

Conversation

gigawhitlocks commented Feb 1, 2018

Uh oh!

sgairo commented Feb 1, 2018

Uh oh!

hickeng commented Feb 2, 2018

Uh oh!

vburenin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gigawhitlocks Feb 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gigawhitlocks commented Feb 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gigawhitlocks Feb 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gigawhitlocks commented Feb 7, 2018

Uh oh!

mhagen-vmware left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

gigawhitlocks Feb 6, 2018 •

edited

Loading

gigawhitlocks Feb 6, 2018 •

edited

Loading