Skip to content

Worker E2E: compile and run API ad Worker binaries #1347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 28 commits into from
Jun 9, 2023

Conversation

michaljurecko
Copy link
Contributor

@michaljurecko michaljurecko commented Jun 6, 2023

Jira: https://keboola.atlassian.net/browse/PSGO-216

Intro

V Buffer Service mame 2 typy E2E testov:

  • Pre API: https://github.com/keboola/keboola-as-code/tree/main/test/buffer/api
    • Spusti sa API binarka, posielaju sa requests, kontroluju sa responses a stav etcd databazy.
  • Pre Worker (+API):
    • Tu je to zlozitejsie na otestovanie, kedze sa jedne o distribuovany system s viacerými nodes.
    • Ciel je otestovat vsetky operacie, ktoré robí worker, najmä:
      • upload slice na staging storage
      • import file do table
      • dolezita je aj synchronizacia s API nodes, aby upload slice zacal az ked uz ziadna API node do nej (v etcd) nezapisuje, tj. všetky API nodes sa prepli na novu slice.

V tomto PR sa namiesto testovania Worker a API nodes v Go kode spustia priamo binarky:

  • Na API nodes sa posielaju HTTP requests (nahodne sa vyberie jedna).
  • Zbieraju sa logs: stdout/stderr.
  • Priebezne sa kontroluje stav etcd.
  • Nakoniec sa skontroluju rows v Keboola Table, cez table preview API call.

Motivacia:

  • Uz sa stalo, ze bola chyba v dependencies, ktoru sposob testovania bez spustenia binarky neodhalil a spadlo to az na produkcii.

Test Parts

Jedna sa prakticky o jeden test, ktory ma viacero casti:

ts := newTestSuite(t, ctx, testsDir, project)
go func() {
// Start API and Worker nodes
ts.StartCluster()
// Run test-cases
ts.test000Setup()
ts.test001ImportRecords()
ts.test002SliceUpload()
ts.test003FileImport()
ts.test004EmptyFileAndSlice()
ts.test998BufferSizeOverflow()
ts.test999Cleanup()
// Shutdown all nodes
ts.ShutdownCluster()
}()
// t.Fatal cannot be called from a goroutine, it would not stop the test, therefore the fatalCh is used.
// https://github.com/golang/go/issues/15758
if err := <-ts.fatalCh; err != nil {
t.Fatal(err)
}

V teste sa vacsinou:

  • Vykona nejká API operácia:
    // Import records 5-6
    for i := 5; i <= 6; i++ {
    ts.Import(i)
    }
  • Caka kym sa nieco stane, detekuje sa to na zaklade logs:
    // Periodic condition checks have detected that the UPLOAD conditions for both slices/exports have been met.
    ts.t.Logf("waiting for upload conditions check ...")
    if ts.WaitForLogMessages(15*time.Second, `
    [worker-node-%d][bufferWorker][service][conditions]INFO closing slice "%s": count threshold met, received: 6 rows, threshold: 5 rows
    [worker-node-%d][bufferWorker][service][conditions]INFO closing slice "%s": count threshold met, received: 6 rows, threshold: 5 rows
    `) {
    ts.t.Logf("upload conditions met")
    }
  • Na záver danej casti sa skontroluje stav etcd + ze sa nezalogovala ziadna chyba/warning.
    // Check etcd state
    ts.AssertEtcdState("002-slice-upload")
    ts.AssertNoLoggedWarning()
    ts.AssertNoLoggedError()
    ts.TruncateLogs()

etcd expected state

Vramci testu sa do etcd ulozi relativne vela záznamov, no kazdá časť testu mení iba pár záznamov.

Preto sú snapshoty etcd rozdelené do viacerých súborov:

image

A dany snapshot sa posklada zo spolocnych casti, napr.:

<include _runtime>
<include _config_001>
<include _secrets>
<include _tasks_setup>
<include _files_opened_001>
<include _files_imported_001>
<include _slices_opened_001>
<include _slices_imported_001>
<include _slices_imported_002>
<include _tasks_slices_swap_001>
<include _tasks_slices_upload_001>
<include _tasks_slices_upload_002>
<include _tasks_files_swap_001>
<include _tasks_files_import_001>
<<<<<
runtime/last/record/id/%%TEST_KBC_PROJECT_ID%%/my-receiver/my-export-1
-----
10
>>>>>
<<<<<
runtime/last/record/id/%%TEST_KBC_PROJECT_ID%%/my-receiver/my-export-2
-----
10
>>>>>

.out

.out adresar obsahuje reálne výstupy počas testu, ktoré sa dajú použiť na debugovanie:

image

Test Log

Nizsie je log testu, bezi ~80s kym sa vsetko nasetupuje (Storage API File Resources, ...), uploadne a importuje do tabulky.

  • >=5 zaznamou trigruje upload (v teste je ich 6)
  • >=10 záznamou trigruje import (v teste je ich 10), teda 2 slices
Log testu go test -v -count 1 -race ./test/buffer/worker/...` (expand)
/code > go test -v -count 1 -race ./test/buffer/worker/...
=== RUN   TestBufferWorkerE2E
=== PAUSE TestBufferWorkerE2E
=== CONT  TestBufferWorkerE2E
    cluster_test.go:336: compiling "buffer-worker" binary to "/tmp/TestBufferWorkerE2E872270074/001/buffer-worker" ...
    cluster_test.go:332: compiling "buffer-api" binary to "/tmp/TestBufferWorkerE2E872270074/002/buffer-api" ...
    cluster_test.go:336: compilation "buffer-worker" done
    cluster_test.go:332: compilation "buffer-api" done
    cluster_test.go:344: -------------------------
    cluster_test.go:345: compilation successful
    cluster_test.go:346: -------------------------
    cluster_test.go:208: waiting for all nodes ...
    cluster_test.go:319: started node "worker-node-2"
    cluster_test.go:319: started node "worker-node-3"
    cluster_test.go:319: started node "worker-node-1"
    cluster_test.go:263: waiting for node "api-node-1"
    cluster_test.go:319: started node "worker-node-4"
    cluster_test.go:263: waiting for node "api-node-5"
    cluster_test.go:263: waiting for node "api-node-4"
    cluster_test.go:319: started node "worker-node-5"
    cluster_test.go:263: waiting for node "api-node-2"
    cluster_test.go:263: waiting for node "api-node-3"
    cluster_test.go:268: started node "api-node-5"
    cluster_test.go:268: started node "api-node-4"
    cluster_test.go:268: started node "api-node-1"
    cluster_test.go:268: started node "api-node-2"
    cluster_test.go:268: started node "api-node-3"
    cluster_test.go:211: -------------------------
    cluster_test.go:212: cluster started, 5 API nodes, 5 worker nodes
    cluster_test.go:213: -------------------------
    000_setup_test.go:11: -------------------------
    000_setup_test.go:12: 000 setup
    000_setup_test.go:13: -------------------------
    000_setup_test.go:15: creating receiver "my-receiver" ...
    000_setup_test.go:15: created task: create receiver "my-receiver"
    000_setup_test.go:15: waiting for task "my-receiver/receiver.create/2023-06-06T08:32:25.785Z_27nT4" ...
    000_setup_test.go:15: created receiver "my-receiver"
    000_setup_test.go:15: loading receiver "my-receiver" ...
    000_setup_test.go:15: loaded receiver "my-receiver"
    000_setup_test.go:17: creating export "my-receiver/my-export-1" ...
    000_setup_test.go:17: created task: create export "my-receiver/my-export-1"
    000_setup_test.go:17: waiting for task "my-receiver/my-export-1/export.create/2023-06-06T08:32:27.004Z_nvm4T" ...
    000_setup_test.go:17: created export "my-receiver/my-export-1"
    000_setup_test.go:17: loading export "my-receiver/my-export-1" ...
    000_setup_test.go:17: loaded export "my-receiver/my-export-1"
    000_setup_test.go:24: creating export "my-receiver/my-export-2" ...
    000_setup_test.go:24: created task: create export "my-receiver/my-export-2"
    000_setup_test.go:24: waiting for task "my-receiver/my-export-2/export.create/2023-06-06T08:32:38.779Z_drZkO" ...
    000_setup_test.go:24: created export "my-receiver/my-export-2"
    000_setup_test.go:24: loading export "my-receiver/my-export-2" ...
    000_setup_test.go:24: loaded export "my-receiver/my-export-2"
    001_import_records_test.go:9: -------------------------
    001_import_records_test.go:10: 001 import records
    001_import_records_test.go:11: -------------------------
    001_import_records_test.go:20: importing record {"key": "payload001"} (21 B) ...
    001_import_records_test.go:20: importing record {"key": "payload002"} (21 B) ...
    001_import_records_test.go:20: importing record {"key": "payload003"} (21 B) ...
    001_import_records_test.go:20: importing record {"key": "payload004"} (21 B) ...
    002_slice_upload_test.go:9: -------------------------
    002_slice_upload_test.go:10: 002 slice upload
    002_slice_upload_test.go:11: -------------------------
    002_slice_upload_test.go:20: importing record {"key": "payload005"} (21 B) ...
    002_slice_upload_test.go:20: importing record {"key": "payload006"} (21 B) ...
    002_slice_upload_test.go:24: waiting for upload conditions check ...
    002_slice_upload_test.go:29: upload conditions met
    002_slice_upload_test.go:34: waiting for slices swap: new slices should be created, old slices should be switched from writing to closing state ...
    002_slice_upload_test.go:39: slices have been swapped - switched from writing to closing state
    002_slice_upload_test.go:43: waiting for API nodes sync ...
    002_slice_upload_test.go:51: all API nodes synced to revision with new slices
    002_slice_upload_test.go:58: waiting for slices to close ...
    002_slice_upload_test.go:68: all API nodes reported the new revision, none writes to old slices anymore
    002_slice_upload_test.go:69: slices have been closed - switched from closing to closed state
    002_slice_upload_test.go:73: waiting for slices to upload ...
    002_slice_upload_test.go:78: slices have been uploaded
    002_slice_upload_test.go:82: waiting for slices upload events ...
    002_slice_upload_test.go:87: slices upload events sent
    003_file_import_test.go:12: -------------------------
    003_file_import_test.go:13: 003 file import
    003_file_import_test.go:14: -------------------------
    003_file_import_test.go:23: importing record {"key": "payload007"} (21 B) ...
    003_file_import_test.go:23: importing record {"key": "payload008"} (21 B) ...
    003_file_import_test.go:23: importing record {"key": "payload009"} (21 B) ...
    003_file_import_test.go:23: importing record {"key": "payload010"} (21 B) ...
    003_file_import_test.go:27: waiting for import conditions check ...
    003_file_import_test.go:32: import conditions met
    003_file_import_test.go:37: waiting for files swap: new files should be created, old files should be switched from writing to closing state ...
    003_file_import_test.go:42: files have been swapped - switched from writing to closing state
    003_file_import_test.go:46: waiting for slices to close ...
    003_file_import_test.go:51: slices have been closed - switched from closing to closed state
    003_file_import_test.go:55: waiting for slices to upload ...
    003_file_import_test.go:60: slices have been uploaded
    003_file_import_test.go:64: waiting for slices upload events ...
    003_file_import_test.go:69: slices upload events sent
    003_file_import_test.go:73: waiting for files to close (blocked until all slices were closed) ...
    003_file_import_test.go:74: all API nodes reported the new revision, old slices were switched from closing to closed state
    003_file_import_test.go:79: files have been closed - switched from closing to closed state
    003_file_import_test.go:83: waiting for files to import ...
    003_file_import_test.go:88: files have been imported
    003_file_import_test.go:92: waiting for files import events ...
    003_file_import_test.go:97: files import events sent
    003_file_import_test.go:122: loading preview of the table "in.c-bucket.my-export-1" ...
    003_file_import_test.go:122: loaded preview of the table "in.c-bucket.my-export-1", found 10 rows
    003_file_import_test.go:137: loading preview of the table "in.c-bucket.my-export-2" ...
    003_file_import_test.go:137: loaded preview of the table "in.c-bucket.my-export-2", found 10 rows
    004_empty_file_and_slice_test.go:13: -------------------------
    004_empty_file_and_slice_test.go:14: 004 empty file and slice
    004_empty_file_and_slice_test.go:15: -------------------------
    004_empty_file_and_slice_test.go:18: updating export "my-receiver/my-export-1" ...
    004_empty_file_and_slice_test.go:18: created task: update export "my-receiver/my-export-1"
    004_empty_file_and_slice_test.go:18: waiting for task "my-receiver/my-export-1/export.update/2023-06-06T08:33:17.766Z_38Uws" ...
    004_empty_file_and_slice_test.go:18: updated export "my-receiver/my-export-1"
    004_empty_file_and_slice_test.go:18: loading export "my-receiver/my-export-1" ...
    004_empty_file_and_slice_test.go:18: loaded export "my-receiver/my-export-1"
    004_empty_file_and_slice_test.go:28: waiting for slice to close, slice is empty, upload and import should be skipped ...
    004_empty_file_and_slice_test.go:35: slice has been closed - upload and import have been skipped
    998_overflow_test.go:13: -------------------------
    998_overflow_test.go:14: 998 buffer size overflow
    998_overflow_test.go:15: -------------------------
    998_overflow_test.go:23: importing record {"key": "-------------------------------… (100.0 KB) ...
    998_overflow_test.go:29: importing record {"key": "foo"} (14 B) ...
    998_overflow_test.go:29: imported failed: No free space in the buffer: receiver "my-receiver" has "200.1 KB" buffered for upload, limit is "100.0 KB".
    999_cleanup_test.go:5: -------------------------
    999_cleanup_test.go:6: 999 cleanup
    999_cleanup_test.go:7: -------------------------
    999_cleanup_test.go:10: deleting export "my-receiver/my-export-1" ...
    999_cleanup_test.go:10: deleted export "my-receiver/my-export-1"
    999_cleanup_test.go:13: deleting receiver "my-receiver" ...
    999_cleanup_test.go:13: deleted receiver "my-receiver"
--- PASS: TestBufferWorkerE2E (79.23s)
PASS
ok  	github.com/keboola/keboola-as-code/test/buffer/worker	79.363s

(okomentoavne po commitoch)

@michaljurecko michaljurecko force-pushed the michaljurecko-PSGO-216-e2e-test-binaries branch 2 times, most recently from 8a04405 to c6624f9 Compare June 6, 2023 13:09
@michaljurecko michaljurecko force-pushed the michaljurecko-PSGO-216-e2e-test-binaries branch from c6624f9 to ae791e3 Compare June 6, 2023 14:25
Inspired by [PhpUnit](https://phpunit.readthedocs.io/en/9.5/assertions.html#assertstringmatchesformat).
Inspired by [PhpUnit](https://docs.phpunit.de/en/10.0/assertions.html#assertstringmatchesformat).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixnuty link (404) v Markdown subore.

Comment on lines +28 to +34
# Format code, gofumpt and gci partially overlap, it is needed to run them separately
# https://github.com/golangci/golangci-lint/issues/1490
echo "Running gofumpt ..."
gofumpt -w ./cmd ./internal ./pkg ./test
echo "Running gci ..."
gci write --skip-generated -s standard -s default -s "prefix(github.com/keboola/keboola-as-code)" ./cmd ./internal ./pkg ./test

Copy link
Contributor Author

@michaljurecko michaljurecko Jun 6, 2023

Choose a reason for hiding this comment

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

Vramci golangci-lint --fix (nizsie) sa spustaju okrem ineho aj 2 nastroje gci a gofumpt, ktore sa ciastocne prekryvaju vo svojej funkcionalite - oba zoradzuju/formatuju importy v go suboroch.

gci je nastroj iba na tento ucel, gofumpt ma ine uzitocne funkcie, ale zial neda sa tam to formatovane vypnut (aby ho robil iba gci).

Vramci golangci-lint bezia vsetky linters paralelne a niekedy sa stava, ze tieto 2, ak sa pouzije aj --fix, rozbiju go subor.

Viac na uvedenom linku - komentar v kode.

Jednoduche riesenie je, spustit ich samostatne pred golangci-lint, kym to tam nejak nevyriesia.
(ked nechcem prist o to, ze kod v tomto repe ma striktne definovany format).

}
})
return telemetry.WrapDD(tracerProvider.Tracer("")), nil
if cfg.DatadogEnabled {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Chybali mi tu tieto podmienky, vysledkom boli warnings v testoch, ze sa nedari pripojit k DD agent.

Attribute("instanceId", String, "ID of the created/updated template instance.")
Attribute("instanceId", InstanceID, "ID of the created/updated template instance.")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Drobny fix, na tieto hodnoty ^^^ su definovane vlastne typy, aby sa predislo preklepom, tu chybali.
Kod nizsie je vygenerovany.

Comment on lines -130 to +131
TelemetryOptions: []middleware.OTELOption{
MiddlewareOptions: []middleware.Option{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nie su to options iba pre OTEL, ale pre vsetky middlewares. Vysvetlene nizsie.

Comment on lines +111 to +112
{"1", `{"key": "payload001"}`, `{"Accept-Encoding":"gzip, br","Content-Type":"application/json","User-Agent":"keboola-go-client"}`},
{"2", `{"key": "payload002"}`, `{"Accept-Encoding":"gzip, br","Content-Type":"application/json","User-Agent":"keboola-go-client"}`},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pouziva sa go-client/request, takze je tam tych hlaviciek viac.

Comment on lines -14 to +18
[worker-node-%s][service][conditions]INFO checked "2" opened slices
[worker-node-%s][service][conditions]INFO checked "2" opened slices
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Drobna nekompatibilita medzi Debug a Prod logs.

ts.t.Logf("-------------------------")

newTableID := ts.export1.Mapping.TableID + "-v2"
ts.export1 = ts.UpdateExport(ts.export1.ReceiverID, ts.export1.ID, &server.MappingRequestBody{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

UpdateExport metoda ma uz v sebe kontrolu err aj cakanie na dobehnutie task.

Comment on lines -41 to +32
assert.Equal(ts.t, `no free space in the buffer: receiver "my-receiver" has "200.1 KB" buffered for upload, limit is "100.0 KB"`, err.Error())
assert.Equal(ts.t, `No free space in the buffer: receiver "my-receiver" has "200.1 KB" buffered for upload, limit is "100.0 KB".`, err.Error())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Chyby sa pred API response konvertuju na vety.

Comment on lines 7 to +8
"exportId": "my-export-1",
"name": "my-export-1",
"name": "My Export 1",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

V testoch som zmenil name aby nebolo rovnake ako id, tak to menim aj v tomto snapshote/ocakavanom stave.

@michaljurecko michaljurecko requested a review from zajca June 6, 2023 16:13
github.com/keboola/go-client v1.16.1
github.com/keboola/go-client v1.16.2-0.20230606141226-ac73db20e551
Copy link
Member

Choose a reason for hiding this comment

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

TO už teď možeš změnit :) (nevím jestli to neděláš v dalším PR)

}

// returns true if the request should NOT be logged/traced.
func isRequestIgnored(req *http.Request) bool {
Copy link
Member

Choose a reason for hiding this comment

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

Tady sem si akorát všiml, že to začíná malý i tady aj jinde používáš velké písmena když tu fci používáš jinde v package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No velke pismeno zanamena ze je to public, tj. da sa pouzit MIMO package, vramci pkg to nehra rolu.
Na isRequestIgnored asi nemam zatial use-case, aby sa to pouzivalo mimo middleware pkg, ale mohlo by to byt aj public.

Ina situacia je, ze samotny typ je private, tam zvyknem davat metody Public, kedze je uz typ private, a ak je private aj jeho konstruktor funkcia newPrivate() *private, tak sa neda vytvorit mimo package.

type private struct {
}

func (p *private) PublicMethod() {
}

Je to ^^^ vyhoda v pripade refactoringu, ked zistim, ze private utilitku potrebujem vo viacerych packages, tak zmenim iba private -> Private a nemusi prepisovat metody.

^^^^ neviem ci je ten moj koment pochopitelny :) tu je k tomu nieco viac, .... kedy to je a nie je rozdiel: https://stackoverflow.com/questions/37952354/golang-public-method-to-private-struct-is-this-have-any-use-case

@michaljurecko michaljurecko merged commit 7764cf7 into main Jun 9, 2023
@michaljurecko michaljurecko deleted the michaljurecko-PSGO-216-e2e-test-binaries branch June 9, 2023 12:25
@michaljurecko
Copy link
Contributor Author

TO už teď možeš změnit :) (nevím jestli to neděláš v dalším PR)

jj, v case komentovania to nebolo mergnute, ...upravim v PR Update go-client.
#1350

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants