Closed SalDaniele closed 3 years ago
amorenoz
commented
3 years ago After:
./ovs-offline collect-k8s ovn-worker NAME STATUS ROLES AGE VERSION ovn-worker Ready
26m v1.20.0 tar: Removing leading /' from member names tar: Removing leading/' from member names tar: Removing leading/' from member names tar: Removing leading/' from member names
How about we remove these ones as well?
SalDaniele
commented
3 years ago After:
./ovs-offline collect-k8s ovn-worker NAME STATUS ROLES AGE VERSION ovn-worker Ready 26m v1.20.0 tar: Removing leading
/' from member names tar: Removing leading/' from member names tar: Removing leading/' from member names tar: Removing leading/' from member namesHow about we remove these ones as well?
Yes, the new commit will take care of that too. However, one issue that came up is that kubectl cp (the command that produces the tar error) fails silently if the file does not exist [1]. Therefore sending error output to /dev/null would hide the following from the user should an expected file not exist:
tar: Removing leading `/' from member names
tar: /tmp/restore_test.sh: Cannot stat: No such file or directory
tar: Exiting with failure status due to previous errorsAs far as I can tell there is no way to remove just the single "tar leading '/' " error within the context of kubectl cp. Therefore, I sent the output to a log file to atleast have it available to the user. WDYT?
amorenoz
commented
3 years ago Ugh... I'd look for a way to do it without creating a log file just for that. options that come to mind are, grep that specific worning out (grep -v), stat the files before copying, use cat instead of cp
SalDaniele
commented
3 years ago Ugh... I'd look for a way to do it without creating a log file just for that. options that come to mind are, grep that specific worning out (grep -v), stat the files before copying, use cat instead of cp
Right. How about this way? It gets a little gross to 'grep -v' stderr with set -e, so if you think it'd be cleaner I can stat the file before each cp (although thats not great either just to remove a little warning message).
I did try using cat as well, although I ran into some issues cat-int the full $save_dir and with ovsdb-tool cluster-to-standalone giving an "unexpected file format" error when the dbs were cat rather than cp-ed.
amorenoz
commented
3 years ago If the plan is to disregard the stderr of "kubectl cp", I would move the check after the execution of "kubectl cp". That we we would detect any kubectl error that leads to the file not being copied
kubectl cp /tmp/bar $WORKDIR/foo
[ -f $WORKDIR/foo ] || error "no foo no fun!"If the plan is to disregard the stderr of "kubectl cp", I would move the check after the execution of "kubectl cp". That we we would detect any kubectl error that leads to the file not being copied
kubectl cp /tmp/bar $WORKDIR/foo [ -f $WORKDIR/foo ] || error "no foo no fun!"
So remove the grep -v on stderr? Currently stderr is printed with the exception of that one "tar remove leading /" error.
I will add the check either way, that makes sense since kubectl cp fails silently if the file doesnt exist.
amorenoz
commented
3 years ago If the plan is to disregard the stderr of "kubectl cp", I would move the check after the execution of "kubectl cp". That we we would detect any kubectl error that leads to the file not being copied
kubectl cp /tmp/bar $WORKDIR/foo [ -f $WORKDIR/foo ] || error "no foo no fun!"So remove the grep -v on stderr? Currently stderr is printed with the exception of that one "tar remove leading /" error.
I will add the check either way, that makes sense since kubectl cp fails silently if the file doesnt exist.
I meant, keep the "grep -v" but double check files have been copied and fail gracefully if they have not
SalDaniele
commented
3 years ago Understood. Checks have been added.
amorenoz
commented
3 years ago LGTM
Specify the containers used by default in kubectl exec commands to remove redundant warnings.
Previous output:
After:
Signed-off-by: Salvatore Daniele sdaniele@redhat.com