Use alpine images to make tests take less time #629

denismakogon · 2017-12-24T20:56:48Z

use PG alpine
use Minio alpine
no official alpine distro for MySQL, uhhh :(
install swagger tool instead of docker image
use retry func to confirm that datastore is okay before running tests

denismakogon · 2017-12-24T21:46:06Z

Reverting minio image:

docker images | grep minio
minio/minio                   latest              4110ee55fc11        4 weeks ago         27.3MB
webcenter/alpine-minio        latest              49c8f4547226        7 weeks ago         82.3MB

denismakogon · 2017-12-24T21:49:29Z

Not sure what we can do with mysql image, which is 408Mb, mysql docker community has done zero to build on alpine by this time.

PG alpine size made me happy, really:

docker images | grep postgres
postgres                      latest              ec61d13c8566        12 days ago         287MB
postgres                      9.3-alpine          0350272b4017        2 weeks ago         35.5MB

So, the time difference is not that huge at this moment, but a good thing to have along with #624.

rdallman · 2017-12-26T16:36:48Z

helpers.sh

+    docker rm -fv func-minio-test 2>/dev/null || true
+}
+
+function install_swagger_tool {


https://github.com/swagger-api/validator-badge seems to outline a way to send a file to a service running a swagger validator. the badge would be a nice addition, but sending our file to a validator service seems much cleaner than us downloading a binary every build, if anything because in theory the service will ensure we keep up to date with the latest swagger spec and here we have to manually keep the binary version up to date.

Well, the badge is a good thing, but it's not related to this PR, but what if I want to modify swagger how can know if that is okay?

I can add the badge, but we do know that github cache works really slow, we've seen that couple times with swagger that's why we use huge docker image with swagger tool only to validate our swagger doc through the mounted file:

docker run -v /Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn:/go/src/github.com/fnproject/fn --rm quay.io/goswagger/swagger validate /go/src/github.com/fnproject/fn/docs/swagger.yml

The point of downloading binary is to reduce the time of swagger doc validation.
FYI, the swagger tool image is 493MB, swagger tool binary ~2MB.

Not worried about the badge so much here, it would be nice to have separate from this. We can send the file from on disk inside of CI to the swagger server listed from that link for validation (I think), which requires no binary download and then we don't have to worry about versioning of the binary. If we change swagger we had the same issue with the image previously, I don't think this is much of a concern vs us lagging behind swagger updates by manually downloading the binary.

Unfortunately, recommended method (posting swagger doc as the payload) doesn't work:

curl -v -X POST -d @docs/swagger.yml -H 'Content-Type:application/json' http://online.swagger.io/validator/debug Note: Unnecessary use of -X or --request, POST is already inferred. * Trying 23.22.16.221... * TCP_NODELAY set * Connected to online.swagger.io (23.22.16.221) port 80 (#0) > POST /validator/debug HTTP/1.1 > Host: online.swagger.io > User-Agent: curl/7.54.0 > Accept: */* > Content-Type:application/json > Content-Length: 19268 > Expect: 100-continue > < HTTP/1.1 100 Continue * We are completely uploaded and fine < HTTP/1.1 500 Internal Server Error < Date: Wed, 27 Dec 2017 16:19:42 GMT < Access-Control-Allow-Origin: * < Access-Control-Allow-Methods: GET, POST, DELETE, PUT < Access-Control-Allow-Headers: Content-Type < Content-Type: text/html; charset=ISO-8859-1 < Cache-Control: must-revalidate,no-cache,no-store < Content-Length: 323 < Connection: close < Server: Jetty(9.2.9.v20150224) < <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"/> <title>Error 500 Internal Server Error</title> </head> <body><h2>HTTP ERROR 500</h2> <p>Problem accessing /validator/debug. Reason: <pre> Internal Server Error</pre></p><hr><i><small>Powered by Jetty://</small></i><hr/> </body> </html> * Closing connection 0

so, leaving as is.

Ok, next idea: can we build a smaller swagger image with that binary to use instead?

rdallman · 2017-12-26T16:39:37Z

api/datastore/sql/sql_test.go

+func retryDBConnection(t *testing.T, attempts int, sleep time.Duration, dbURL *url.URL) (err error) {
+	for i := 0; i < attempts; i++ {
+		t.Logf("[%v] - attempting to reach out datastore %v", i, dbURL.String())
+		_, err = newDS(dbURL)


this generates a lot of new db instances, which isn't really cheap afaik and also doesn't really ensure that the datastore instance we're going to actually use is valid and adds extra code in the db initialization caller path to call retryDBConnection. is there any reason not to do just do this in our db initialization ping inside of the sql package?

fixed, see #630, I'll rebase this PR to that branch.

rdallman · 2017-12-26T16:42:56Z

Not sure what we can do with mysql image, which is 408Mb, mysql docker community has done zero to build on alpine by this time.

i might piddle around with this, it shouldn't be too bad to get all the deps into alpine but sometimes it is.

rdallman

I'm OK with the pg image change but the rest of the changes I'm less sure of, esp. if we're not getting a lot of benefit.

rdallman · 2017-12-26T17:16:16Z

i might piddle around with this, it shouldn't be too bad to get all the deps into alpine but sometimes it is.

found docker-library/mysql#179 -- we can try to poke the right people internally to get this supported, it appears to work now but need to have release track to follow.

denismakogon · 2017-12-26T21:52:02Z

@rdallman i'll address all comments except swagger tool change. Regarding the MySQL alpine image, we might ask @mattlord for help.

denismakogon · 2018-01-02T18:26:46Z

@rdallman I pushed the image to my own docker repo: denismakogon/swagger-tool:0.0.1. Dockerfile you can find in-tree.
It's not 2Mb size, but 30Mb. Still better than 0.5Gb.

treeder · 2018-01-02T18:27:38Z

How about add it to fnproject/dockers ? Or at least we need the Dockerfile and build scripts public.

denismakogon · 2018-01-02T18:28:36Z

Sure, can submit there.

denismakogon · 2018-01-02T18:32:10Z

@treeder fnproject/dockers#7

skinowski · 2018-01-02T18:59:12Z

Kind of unrelated, but we should eventually be able to get rid of sleeper, hello and error images as fn-test-utils can perform same functionality. I tried removing these with one PR, but I quickly realized it's more work than I anticipated.

rdallman · 2018-01-02T19:31:17Z

.gitignore

@@ -33,3 +33,4 @@ fnlb/fnlb
 .idea/
 *iml
 target/
+swagger


can remove this change now

rdallman

needs rebase, but LGTM

- implements retry func specifically for SQL datastore ping - fmt fixes - using sqlx.Db.PingContext instead of sqlx.Db.Ping - propogate context to SQL datastore

* use PG alpine * use Minio alpine * no official alpine distro for MySQL, uhhh :( * install swagger tool instead of docker image * use retry func to confirm that datastore is okay before running tests

somehow it's a problem to put binary to ${GOPATH}/bin

denismakogon · 2018-01-02T20:36:25Z

Rebased.

denismakogon · 2018-01-02T20:54:23Z

Nice, we're at 07:46, which is good. Is there anything else that we can speed up?

rdallman · 2018-01-02T21:01:58Z

we could play with caching the static library files between runs. 99.9% of the time they don't change, and go install takes about a minute, and should handle building new versions only when it needs to.
seems like reducing tests down to one image could save maybe 30s or so, per Use alpine images to make tests take less time #629 (comment)
we're building the release docker image on branches even though we don't use it for anything, we only need to do this on master. that's 2 minutes.

seems like we could get under 5 minutes pretty easily.

rdallman · 2018-01-02T21:03:45Z

we're also installing new docker version every time, that's 1 minute. maybe we could cache that, too

at least, we could have it in a .deb pack we could just dpkg -i it, that we could cache

skinowski · 2018-01-02T21:04:06Z

+1 on "go install", that should shave extensions/middleware building time as well.

denismakogon requested review from rdallman and skinowski December 24, 2017 20:57

denismakogon force-pushed the small-images branch from 30b97e7 to 4ce63ab Compare December 24, 2017 22:02

rdallman reviewed Dec 26, 2017

View reviewed changes

rdallman mentioned this pull request Dec 26, 2017

Speed up API tests #624

Merged

denismakogon force-pushed the small-images branch from 4ce63ab to 3c15ab8 Compare December 26, 2017 21:53

rdallman mentioned this pull request Jan 2, 2018

fn: test and release scripts now run some jobs in background #620

Closed

denismakogon force-pushed the small-images branch from f433783 to 80a11d1 Compare January 2, 2018 18:27

denismakogon force-pushed the small-images branch from 80a11d1 to ea887fe Compare January 2, 2018 18:45

rdallman reviewed Jan 2, 2018

View reviewed changes

.gitignore Outdated

@@ -33,3 +33,4 @@ fnlb/fnlb

.idea/

*iml

target/

swagger

Copy link

Contributor

rdallman Jan 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove this change now

rdallman approved these changes Jan 2, 2018

View reviewed changes

denismakogon added 7 commits January 2, 2018 22:35

Use retry func while trying to ping SQL datastore

e4cab71

- implements retry func specifically for SQL datastore ping - fmt fixes - using sqlx.Db.PingContext instead of sqlx.Db.Ping - propogate context to SQL datastore

Use alpine images to make tests take less time

7cfec32

* use PG alpine * use Minio alpine * no official alpine distro for MySQL, uhhh :( * install swagger tool instead of docker image * use retry func to confirm that datastore is okay before running tests

Store swagger tool at Fn during CI time

2a6a3d4

somehow it's a problem to put binary to ${GOPATH}/bin

Adjust swagger tool reference path

cc4eec1

Revert minio image

e82f794

Use amd64/alpine-based swagger tool image for API spec validation

47b1d6a

Cleanup

95021d0

denismakogon force-pushed the small-images branch from ea887fe to 95021d0 Compare January 2, 2018 20:36

rdallman merged commit 4bb0744 into master Jan 2, 2018

rdallman deleted the small-images branch January 2, 2018 20:56

rdallman mentioned this pull request Jan 2, 2018

investigate speeding up CI builds #617

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use alpine images to make tests take less time #629

Use alpine images to make tests take less time #629

denismakogon commented Dec 24, 2017

denismakogon commented Dec 24, 2017

denismakogon commented Dec 24, 2017

rdallman Dec 26, 2017

denismakogon Dec 26, 2017

rdallman Dec 27, 2017

denismakogon Dec 27, 2017 •

edited

Loading

rdallman Dec 28, 2017

rdallman Dec 26, 2017

denismakogon Dec 26, 2017

rdallman commented Dec 26, 2017

rdallman left a comment

rdallman commented Dec 26, 2017

denismakogon commented Dec 26, 2017

denismakogon commented Jan 2, 2018

treeder commented Jan 2, 2018 •

edited

Loading

denismakogon commented Jan 2, 2018

denismakogon commented Jan 2, 2018

skinowski commented Jan 2, 2018

rdallman Jan 2, 2018

rdallman left a comment

denismakogon commented Jan 2, 2018

denismakogon commented Jan 2, 2018

rdallman commented Jan 2, 2018

rdallman commented Jan 2, 2018 •

edited

Loading

skinowski commented Jan 2, 2018

@@ @@ -33,3 +33,4 @@ fnlb/fnlb @@
               .idea/
               *iml
               target/
+              swagger

Use alpine images to make tests take less time #629

Use alpine images to make tests take less time #629

Conversation

denismakogon commented Dec 24, 2017

denismakogon commented Dec 24, 2017

denismakogon commented Dec 24, 2017

rdallman Dec 26, 2017

Choose a reason for hiding this comment

denismakogon Dec 26, 2017

Choose a reason for hiding this comment

rdallman Dec 27, 2017

Choose a reason for hiding this comment

denismakogon Dec 27, 2017 • edited Loading

Choose a reason for hiding this comment

rdallman Dec 28, 2017

Choose a reason for hiding this comment

rdallman Dec 26, 2017

Choose a reason for hiding this comment

denismakogon Dec 26, 2017

Choose a reason for hiding this comment

rdallman commented Dec 26, 2017

rdallman left a comment

Choose a reason for hiding this comment

rdallman commented Dec 26, 2017

denismakogon commented Dec 26, 2017

denismakogon commented Jan 2, 2018

treeder commented Jan 2, 2018 • edited Loading

denismakogon commented Jan 2, 2018

denismakogon commented Jan 2, 2018

skinowski commented Jan 2, 2018

rdallman Jan 2, 2018

Choose a reason for hiding this comment

rdallman left a comment

Choose a reason for hiding this comment

denismakogon commented Jan 2, 2018

denismakogon commented Jan 2, 2018

rdallman commented Jan 2, 2018

rdallman commented Jan 2, 2018 • edited Loading

skinowski commented Jan 2, 2018

denismakogon Dec 27, 2017 •

edited

Loading

treeder commented Jan 2, 2018 •

edited

Loading

rdallman commented Jan 2, 2018 •

edited

Loading