Turnoff misleading error/warning messages #1554

shuklanirdesh82 · 2017-07-07T06:31:28Z

This PR cleans up following misleading error/warning message from CI test run.

Avoid spitting warning: overriding commands for target clean'`
Not a git repository (or any of the parent directories): .git when the VERSION_TAG is set no need to invoke remote call to fetch GIT_SHA or GIT_TAG .. in case of vm on the CI where repo is not checked out; it prints such fatal message that creates some distraction while debugging the failure.

e.g. https://ci.vmware.run/vmware/docker-volume-vsphere/949

:::::::::::::::::::::::::::
:::::::::::::::::::::::::::
Makefile:152: warning: overriding commands for target `clean'
../plugin/Makefile:87: warning: ignoring old commands for target `clean'
:::::::::::::::::::::::::::
:::::::::::::::::::::::::::
Makefile:152: warning: overriding commands for target `clean'
../plugin/Makefile:87: warning: ignoring old commands for target `clean'
Removing auth DB file at 192.168.31.62:'/etc/vmware/vmdkops/auth-db'
Makefile:152: warning: overriding commands for target `clean'
../plugin/Makefile:87: warning: ignoring old commands for target `clean'
:::::::::::::::::::::::::::
:::::::::::::::::::::::::::
fatal: Not a git repository (or any of the parent directories): .git
fatal: Not a git repository (or any of the parent directories): .git

Testing done: yes

make clean build-all deploy-all (works fine as expected)

CI output https://ci.vmware.run/vmware/docker-volume-vsphere/968

Using the following config:
DOCKER_HUB_REPO cnastorage EXTRA_TAG -CI VERSION_TAG latest
PLUGIN_NAME cnastorage/docker-volume-vsphere:latest-CI
=== Cleaning work files, images and plugin(s)...
rm -rf /tmp/plugin
rm -f docker-volume-vsphere
docker plugin rm cnastorage/docker-volume-vsphere:latest-CI -f
Error response from daemon: plugin "cnastorage/docker-volume-vsphere:latest-CI" not found
Makefile:54: recipe for target 'clean' failed
docker rm -vf tempContainer
make: [clean] Error 1 (ignored)
Error response from daemon: No such container: tempContainer
Makefile:54: recipe for target 'clean' failed
docker rmi cnastorage/docker-volume-vsphere:rootfs
make: [clean] Error 1 (ignored)
Error response from daemon: No such image: cnastorage/docker-volume-vsphere:rootfs
Makefile:54: recipe for target 'clean' failed
== building Docker image, unpacking to ./rootfs and creating plugin...
cp ../build/docker-volume-vsphere .
make: [clean] Error 1 (ignored)
docker build -q -t cnastorage/docker-volume-vsphere:rootfs .
sha256:b9e9e607f5f4bdef1b428d514b04defd8d66edf254201cc53f409362abe3598d
docker create --name tempContainer cnastorage/docker-volume-vsphere:rootfs
d364b786e6e80c616ce54b08fb6ce96bc24cb9f907bfba182aef9a5b2ef1445c
mkdir -p /tmp/plugin/rootfs
docker export  tempContainer  | tar -x -C /tmp/plugin/rootfs
cp config.json docker-volume-vsphere /tmp/plugin
-- Creating  plugin cnastorage/docker-volume-vsphere:latest-CI ...
docker plugin create cnastorage/docker-volume-vsphere:latest-CI /tmp/plugin
cnastorage/docker-volume-vsphere:latest-CI

/CC @msterin

msterin

Overall looks good. See a couple of minor points inside. Thanks for the fix !

msterin · 2017-07-07T07:19:18Z

Constants.mk

+# This Makefile's purpose is to hold common variables to reuse across
+# Makefiles for vDVS.
+
+export


What does 'export' here do

exporting all the variables resided in this file.

msterin · 2017-07-07T07:19:43Z

Constants.mk

@@ -0,0 +1,78 @@
+# Copyright 2016 VMware, Inc. All Rights Reserved.
+#


This is not really constants. More.like vars

Correct! My bad, I will rename it.

How about Commonvars.mk or EnvVars.mk?

commonvars sounds good. At any rate, this is minor

I will push the change shortly to avoid future confusion with naming.

msterin · 2017-07-07T07:20:39Z

esx_service/Makefile

-# developer builds use last tagged release and sha1 of the most recent commit.
-# Format: <last tagged release>.<last commit hash>
-PKG_VERSION ?= $(shell \
-	       git describe --tags `git rev-list --tags --max-count=1` \


This is not used anymore , right ?

it is being referred from Constants.mk.

you mean set in ? Yes, I see it now.

msterin · 2017-07-07T15:17:27Z

Constants.mk

+# developer builds use last tagged release and sha1 of the most recent commit.
+# Format: <last tagged release>.<last commit hash>
+PKG_VERSION ?= $(shell \
+	       git describe --tags `git rev-list --tags --max-count=1` \


it uses git so I think it may fail exactly the same GIT_SHA calculation used to fail during managed plugin build unless it is pre-defened from upstairs. Please check the logs.

msterin · 2017-07-07T15:19:10Z

esx_service/Makefile

-# developer builds use last tagged release and sha1 of the most recent commit.
-# Format: <last tagged release>.<last commit hash>
-PKG_VERSION ?= $(shell \
-	       git describe --tags `git rev-list --tags --max-count=1` \


you mean set in ? Yes, I see it now.

msterin

Actually,

I take the "approval" back. Per CI, (1) the managed plugin make is broken, and (2) CI itself ignores serious error in plugin build:

(1)

rm -f 
docker plugin rm : -f
Error response from daemon: plugin ":" not found
make: [clean] Error 1 (ignored)
Makefile:51: recipe for target 'clean' failed
docker rm -vf tempContainer
Error response from daemon: No such container: tempContainer
make: [clean] Error 1 (ignored)
Error response from daemon: invalid reference format
make: [clean] Error 1 (ignored)
cp: omitting directory '/'
make: *** [plugin] Error 1
Makefile:51: recipe for target 'clean' failed
docker rmi :rootfs
Makefile:51: recipe for target 'clean' failed
== building Docker image, unpacking to ./rootfs and creating plugin...
cp / .
Makefile:59: recipe for target 'plugin' failed
plugin_sanity_test: [INFO] Running plugin_sanity_test on the clean test setup...
plugin_sanity_test: [INFO] Installed plugin name is:
plugin_sanity_test: [ERROR] error has occurred while fetching created plugin name..
         please make sure plugin is built correctly or not
Pushing : to dockerhub.io...
invalid reference format

despite the above, the CI did not stop and did not report error

shuklanirdesh82 · 2017-07-07T15:33:17Z

Thanks @msterin!

#1554 (review)

For (1) I have a local fix (missed a commit) and waiting for the file rename to push new commit. Sure I will Commonvars.mk as name

for (2) I need to investigate why make deploy-vm-plugin ignores failure.

from https://github.com/vmware/docker-volume-vsphere/blob/master/misc/drone-scripts/deploy-and-test-wrapper.sh

TARGET+=" deploy-vm-plugin"


if make -s $TARGET $PARAMETER;
  | then
  | echo "=> Build Complete" `date`
  | #stop_build $VM1 $BUILD_NUMBER
  | else
  | log "Build + Test not successful"
  | INCLUDE_HOSTD="true"
  | dump_logs
  | stop_build $VM1 $BUILD_NUMBER
  | exit 1
  | fi

vmwclabot added the cla-not-required label Jul 7, 2017

shuklanirdesh82 requested a review from msterin July 7, 2017 06:43

shuklanirdesh82 self-assigned this Jul 7, 2017

shuklanirdesh82 added the component/ci-infrastructure label Jul 7, 2017

msterin approved these changes Jul 7, 2017

View reviewed changes

shuklanirdesh82 mentioned this pull request Jul 7, 2017

Turn off unnecessary logs from cleanup routines #1557

Merged

shuklanirdesh82 force-pushed the misleadingErrCleanup.shuklanirdesh82 branch from dc2f849 to 49f2190 Compare July 7, 2017 08:54

msterin approved these changes Jul 7, 2017

View reviewed changes

msterin suggested changes Jul 7, 2017

View reviewed changes

shuklanirdesh82 force-pushed the misleadingErrCleanup.shuklanirdesh82 branch from 49f2190 to 6d39c67 Compare July 7, 2017 18:25

Turnoff misleading error/warning messages (addressing Mark's comment)

4269c19

shuklanirdesh82 force-pushed the misleadingErrCleanup.shuklanirdesh82 branch from 6d39c67 to 4269c19 Compare July 7, 2017 19:50

msterin approved these changes Jul 7, 2017

View reviewed changes

shuklanirdesh82 merged commit 556bf78 into vmware-archive:master Jul 8, 2017

shuklanirdesh82 deleted the misleadingErrCleanup.shuklanirdesh82 branch July 8, 2017 00:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Turnoff misleading error/warning messages #1554

Turnoff misleading error/warning messages #1554

shuklanirdesh82 commented Jul 7, 2017 •

edited

Loading

msterin left a comment

msterin Jul 7, 2017

shuklanirdesh82 Jul 7, 2017

msterin Jul 7, 2017

shuklanirdesh82 Jul 7, 2017

msterin Jul 7, 2017

shuklanirdesh82 Jul 7, 2017

msterin Jul 7, 2017

shuklanirdesh82 Jul 7, 2017

msterin Jul 7, 2017

shuklanirdesh82 Jul 7, 2017

msterin Jul 7, 2017

msterin Jul 7, 2017

msterin left a comment •

edited

Loading

shuklanirdesh82 commented Jul 7, 2017 •

edited

Loading

		@@ -0,0 +1,78 @@
		# Copyright 2016 VMware, Inc. All Rights Reserved.
		#

Turnoff misleading error/warning messages #1554

Turnoff misleading error/warning messages #1554

Conversation

shuklanirdesh82 commented Jul 7, 2017 • edited Loading

msterin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msterin left a comment • edited Loading

Choose a reason for hiding this comment

shuklanirdesh82 commented Jul 7, 2017 • edited Loading

shuklanirdesh82 commented Jul 7, 2017 •

edited

Loading

msterin left a comment •

edited

Loading

shuklanirdesh82 commented Jul 7, 2017 •

edited

Loading