Skip to content

Commit

Permalink
Merge #41083
Browse files Browse the repository at this point in the history
41083: roachtest: properly fail when uploading binaries fails r=nvanbenschoten a=nvanbenschoten

Closes #41016.
Closes #40864.
Closes #40578.

In all of the referenced issues, we were seeing uploading cockroach
binaries fail (which should be idempotent). We could see this in the
log:
```
05:11:12 test.go:182: test status: uploading binary
05:11:12 cluster.go:315: > /home/agent/work/.go/src/github.com/cockroachdb/cockroach/bin/roachprod put teamcity-1569301790-07-n4cpu4 /home/agent/work/.go/src/github.com/cockroachdb/cockroach/cockroach.linux-2.6.32-gnu-amd64 ./cockroach
teamcity-1569301790-07-n4cpu4: putting (dist) /home/agent/work/.go/src/github.com/cockroachdb/cockroach/cockroach.linux-2.6.32-gnu-amd64 ./cockroach
................
   1: done
   2: ~ scp -r -C -o StrictHostKeyChecking=no -i /root/.ssh/id_rsa -i /root/.ssh/google_compute_engine /home/agent/work/.go/src/github.com/cockroachdb/cockroach/cockroach.linux-2.6.32-gnu-amd64 root@35.222.255.152:./cockroach
Warning: Permanently added '35.222.255.152' (ECDSA) to the list of known hosts.
packet_write_wait: Connection to 35.222.255.152 port 22: Broken pipe
lost connection
: exit status 1
   3: done
   4: done
I190924 05:11:29.022222 1 cluster_synced.go:1088  put /home/agent/work/.go/src/github.com/cockroachdb/cockroach/cockroach.linux-2.6.32-gnu-amd64 failed
```

The test would then ignore the failure and proceed to get tripped up
when starting cockroach:
```
05:11:34 cluster.go:315: > /home/agent/work/.go/src/github.com/cockroachdb/cockroach/bin/roachprod start --racks=1 --args=--locality-advertise-addr=rack=0@35.222.255.152 teamcity-1569301790-07-n4cpu4:2
teamcity-1569301790-07-n4cpu4: starting
0: exit status 255
~ ./cockroach version

github.com/cockroachdb/cockroach/pkg/cmd/roachprod/install.getCockroachVersion
	/home/agent/work/.go/src/github.com/cockroachdb/cockroach/pkg/cmd/roachprod/install/cockroach.go:88
github.com/cockroachdb/cockroach/pkg/cmd/roachprod/install.Cockroach.Start.func1
	/home/agent/work/.go/src/github.com/cockroachdb/cockroach/pkg/cmd/roachprod/install/cockroach.go:149
github.com/cockroachdb/cockroach/pkg/cmd/roachprod/install.(*SyncedCluster).Parallel.func1.1
	/home/agent/work/.go/src/github.com/cockroachdb/cockroach/pkg/cmd/roachprod/install/cluster_synced.go:1535
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:1337:
```

The problem was that we were ignoring the Put error accidentally, so the
tests got very confused. This commit fixes this by properly handling the
Put error. This doesn't actually fix the referenced issues entirely, but
it gets us a step closer to doing so, so I'm going to use it as an
opportunity to close them.

Release justification: Testing only.

Release note: None

Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
  • Loading branch information
craig[bot] and nvanbenschoten committed Sep 25, 2019
2 parents 1311b51 + 1ecaa0f commit fe735c9
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -1534,7 +1534,7 @@ func (c *cluster) PutE(ctx context.Context, l *logger, src, dest string, opts ..

err := execCmd(ctx, c.l, roachprod, "put", c.makeNodes(opts...), src, dest)
if err != nil {
return errors.Wrap(ctx.Err(), "cluster.Put")
return errors.Wrap(err, "cluster.Put")
}
return nil
}
Expand Down

0 comments on commit fe735c9

Please sign in to comment.