skatteetaten / terraform-nomad-trino Goto Github PK

Terraform module to set up presto on nomad

License: Apache License 2.0

Python 1.07% Makefile 9.67% HCL 76.91% Shell 0.05% Java 12.30%

terraform-nomad-trino's Issues

Additional information in README.md

What is the issue?

I miss information about how to create local proxy to presto instance to make http://localhost:8080 avaliable (presto-gui and running queries from intelliJ).

Suggestion(s)/solution(s) [Optional]

I suggest to include information about how to create local proxy to presto instance, which is:make proxy-presto. The commands already exists in Makefile.

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

A slash too many in a template path

Current behaviour

We have a slash at the start of a path for a template in presto_standalone.hcl and presto.hcl
https://github.com/fredrikhgrelland/terraform-nomad-presto/blob/11ef41ef3ca2472d7f32226a2c7425f350554d71/conf/nomad/presto_standalone.hcl#L153
https://github.com/fredrikhgrelland/terraform-nomad-presto/blob/11ef41ef3ca2472d7f32226a2c7425f350554d71/conf/nomad/presto.hcl#L204

Expected behaviour

Shouldn't be there, and will, according to Fredrik, cause an error in Foundation 2.

How to reproduce?

Suggestion(s)/solution(s) [Optional]

Remove the first slash

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Current behaviour

Throws warnings when running targets from makefile

~/projects/terraform-nomad-presto(master) » make clean                                                                                                      m88614@SKE-DC6KF-MD6T
Makefile:77: warning: overriding commands for target `status'
Makefile:60: warning: ignoring old commands for target `status'

Expected behaviour

No warnings

How to reproduce?

Run make clean or make up

Suggestion(s)/solution(s) [Optional]

Will look at why it happens tomorrow. Making this now so that I don't forget

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Add canary-switch

Canaries will not work with limited resources on the vagrant box. Create a switch to turn it off and use it in example.

Add random secret (in vault) for presto cluster communication

A random secret is required for cluster communication in presto.
This secret should be added to vault and used by presto by referencing vault in a template stanza.

Presto reports warning if hive is down/not present

Current behaviour

Expected behavior

Presto should fail.

How to reproduce?

make test in mode=standalone
Go to nomad, stop manually hive job
Check healthchecks in consul

Suggestion(s)/solution(s) [Optional]

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Readd healthchecks in presto_standalone example

Current behaviour

No healthchecks

Expected behaviour

Commented helathchecks should be implemented: https://github.com/fredrikhgrelland/terraform-nomad-presto/blob/master/conf/nomad/presto_standalone.hcl#L50-L67

How to reproduce?

make up-standalone

Suggestion(s)/solution(s) [Optional]

Maybe add a service proxie to do the checks for us.

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Remove dependency on consul to use box.

We can use docker in the makefile to remove any dependencies other than docker.

Input section in readme does not contain info about local_docker_image

What is the issue?

Input section in readme does not contain info about local_docker_image

Suggestion(s)/solution(s) [Optional]

Update readme

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Consul-connect enabled presto cluster.

In order to form a functioning cluster of presto nodes in a consul-connect service mesh, we need presto to resolve inside of the cluster. We can not rely in service discovery inside of the cluster, as presto will announce and resolve its workers with the discovery-server built into airlift.io.
In order to connect-enable presto we need the entire uri to match "inside and outside" of the containers connected by consul connect. In order for this to work in nomad and resolve hive metastore and minio by normal sidecards, we will use a combination of consul connect native designation, a certificates-handler sidecar and update /etc/hosts by noop templating of the service catalog.

There will be a draft PR shortly for all of this. We keep the option of standlone container without all the trickery, as well as a fully fledged cluster job.

Implement resource sizing

Based on recommendations here: https://prestosql.io/blog/2020/08/27/training-performance.html
We should implement sane defaults (t-shirt sizing maybe) and calculated memory allocation.

Remove use of locals/find new way to define examples

Feature description

The locals block in our examples is a little messy, and there is quite a bit of excess code.

Why is it needed?

Better readability

Suggestion(s)/solution(s) [Optional]

Remove the locals block and write the variables directly.

Definition of done

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

make-command in option #3 in verifying setup gives an error

Current behaviour

I'm following the setup here: https://github.com/fredrikhgrelland/terraform-nomad-presto#option-3-local-presto-cli

Eivinds-MacBook-Pro:terraform-nomad-presto eivindberg$ sudo make presto-cli
Password:
Makefile:78: warning: overriding commands for target `status'
Makefile:61: warning: ignoring old commands for target `status'
make: *** No rule to make target `y', needed by `presto-cli'.  Stop.

Removing the :y-flag in the Makefile gives the Presto-cli, however I have no connection to Presto.

Eivinds-MacBook-Pro:terraform-nomad-presto eivindberg$ sudo make presto-cli
Makefile:78: warning: overriding commands for target `status'
Makefile:61: warning: ignoring old commands for target `status'
CID=$(docker run --rm -d --network host consul:1.8 connect proxy -token master -service presto-local -upstream presto:8080)
docker run --rm -it --network host prestosql/presto:341 presto --server localhost:8080 --http-proxy localhost:8080 --catalog hive --schema default --user presto --debug
presto:default> show catalogs;
Error running command: java.net.ConnectException: Failed to connect to localhost/0:0:0:0:0:0:0:1:8080
java.io.UncheckedIOException: java.net.ConnectException: Failed to connect to localhost/0:0:0:0:0:0:0:1:8080

Having no connection could be related to issue #46 Update documentation, distributed mode deployment.

The make presto-cli not working is something else.

Expected behaviour

To enter the CLI and being able to run a query successfully from the cli by running:

make presto-cli and then the query (like SHOW CATALOGS;).

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Update changelog

@fredrikhgrelland

Missing PR's changelog

Turn `ON` all optional features in example

What is the issue?

Lacking optional features in tests:

vault-provided credentials

Suggestion(s)/solution(s) [Optional]

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Increase memory allocation for consul sidecar proxy

Feature description

Increase memory allocation for presto sidecar proxy

Why is it needed?

Proxy crashes due to beeing out of memory

Suggestion(s)/solution(s) [Optional]

Increase memory allocation

Definition of done

Side car proxy services are running with more memory

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Bump modules versions

minio
postgres
hive

`make presto-cli` does not work

Current behaviour

show catalogs; throw the error

Expected behaviour

presto:default> show catalogs;
 Catalog 
---------
 hive    
 jmx     
 memory  
 system  
 tpcds   
 tpch    
(6 rows)

Query 20201026_132630_00069_hhwdp, FINISHED, 1 node
http://localhost:8080/ui/query.html?20201026_132630_00069_hhwdp
Splits: 19 total, 19 done (100.00%)
CPU Time: 0.0s total,     0 rows/s,     0B/s, 5% active
Per Node: 0.0 parallelism,     0 rows/s,     0B/s
Parallelism: 0.0
Peak Memory: 0B
0.47 [0 rows, 0B] [0 rows/s, 0B/s]

How to reproduce?

make up
make presto-cli
show catalogs;

Log

 ~/src/github.com/zhenik/terraform-nomad-presto │ master *1 !1  make presto-cli                                                                                                                                     ✔ │ 11s │ 14:27:29 
Makefile:77: warning: overriding recipe for target 'status'
Makefile:60: warning: ignoring old recipe for target 'status'
CID=$(docker run --rm -d --network host consul:1.8 connect proxy -token master -service presto-local -upstream presto:8080)
docker run --rm -it --network host prestosql/presto:341 presto --server localhost:8080 --http-proxy localhost:8080 --catalog hive --schema default --user presto --debug
docker rm -f $CID
presto:default> show catalogs;
Error running command: java.net.SocketException: Connection reset
java.io.UncheckedIOException: java.net.SocketException: Connection reset
        at io.prestosql.client.JsonResponse.execute(JsonResponse.java:154)
        at io.prestosql.client.StatementClientV1.<init>(StatementClientV1.java:134)
        at io.prestosql.client.StatementClientFactory.newStatementClient(StatementClientFactory.java:24)
        at io.prestosql.cli.QueryRunner.startInternalQuery(QueryRunner.java:146)
        at io.prestosql.cli.QueryRunner.startQuery(QueryRunner.java:132)
        at io.prestosql.cli.Console.process(Console.java:347)
        at io.prestosql.cli.Console.runConsole(Console.java:273)
        at io.prestosql.cli.Console.run(Console.java:172)
        at io.prestosql.cli.Console.call(Console.java:101)
        at io.prestosql.cli.Console.call(Console.java:74)
        at picocli.CommandLine.executeUserObject(CommandLine.java:1933)
        at picocli.CommandLine.access$1100(CommandLine.java:145)
        at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2332)
        at picocli.CommandLine$RunLast.handle(CommandLine.java:2326)
        at picocli.CommandLine$RunLast.handle(CommandLine.java:2291)
        at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2159)
        at picocli.CommandLine.execute(CommandLine.java:2058)
        at io.prestosql.cli.Presto.main(Presto.java:32)
Caused by: java.net.SocketException: Connection reset
        at java.base/java.net.SocketInputStream.read(SocketInputStream.java:186)
        at java.base/java.net.SocketInputStream.read(SocketInputStream.java:140)
        at okio.Okio$2.read(Okio.java:139)
        at okio.AsyncTimeout$2.read(AsyncTimeout.java:237)
        at okio.RealBufferedSource.indexOf(RealBufferedSource.java:345)
        at okio.RealBufferedSource.readUtf8LineStrict(RealBufferedSource.java:217)
        at okio.RealBufferedSource.readUtf8LineStrict(RealBufferedSource.java:211)
        at okhttp3.internal.http1.Http1Codec.readResponseHeaders(Http1Codec.java:187)
        at okhttp3.internal.http.CallServerInterceptor.intercept(CallServerInterceptor.java:88)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
        at io.prestosql.client.OkHttpUtil.lambda$interceptRequest$3(OkHttpUtil.java:106)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.java:45)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
        at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.java:93)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
        at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.java:93)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.java:125)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
        at okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:200)
        at okhttp3.RealCall.execute(RealCall.java:77)
        at io.prestosql.client.JsonResponse.execute(JsonResponse.java:131)
        ... 17 more
presto:default>

Suggestion(s)/solution(s) [Optional]

Fix proxy and cli commands

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Update documentation, distributed (cluster) mode deployment

What is the issue?

Ref: https://github.com/fredrikhgrelland/terraform-nomad-presto#option-2-presto-and-nomad

Verifying setup -> Option 2 does not work when deploying presto in distributed mode.
It fails on the step when the user needs to execute a command

show catalogs;

After command presto

Suggestion(s)/solution(s) [Optional]

Check the configuration of coordinator.

It might be the wrong port 8080, due to internal proxy communication
http disabled, only https

node.id=3b1c9ca0-2096-9018-22c1-017d58008f1c
node.environment=presto
node.internal-address=presto


coordinator=true
node-scheduler.include-coordinator=false
discovery-server.enabled=true
discovery.uri=https://127.0.0.1:25056

dynamic.http-client.https.hostname-verification=false
failure-detector.http-client.https.hostname-verification=false
memoryManager.http-client.https.hostname-verification=false
scheduler.http-client.https.hostname-verification=false
workerInfo.http-client.https.hostname-verification=false

discovery.http-client.https.hostname-verification=false
node-manager.http-client.https.hostname-verification=false
exchange.http-client.https.hostname-verification=false


http-server.http.enabled=false
http-server.authentication.type=CERTIFICATE
# Work behind proxy
http-server.authentication.allow-insecure-over-http=true
http-server.process-forwarded=true
http-server.https.enabled=true
http-server.https.port=25056
http-server.https.keystore.path=/local/presto.pem
http-server.https.truststore.path=/local/roots.pem

# This is the same jks, but it will not do the consul connect authorization in intra cluster communication
internal-communication.https.required=true

internal-communication.shared-secret= "asdasdsadafdsa"

internal-communication.https.keystore.path=/local/presto.pem
internal-communication.https.truststore.path=/local/roots.pem

query.client.timeout=5m
query.min-expire-age=30m
query.max-memory=76MB

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Add CPU to resources

Feature description

Add CPU as a variable you can set in the module

Why is it needed?

More flexibility and control for the user

Suggestion(s)/solution(s) [Optional]

add to variables.tf

Definition of done

User can set CPU from input variable to module

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Make consul connect plugin source configurable

To support airgapped environments the artefact source uri for the plugin must be configurable

Add data examples and create tables

csv
json
avro
protobuf

Add support for vault-provided credentials (and example)

What is the issue?

Add support for fetching credentials for dependent modules from the Vault and render them directly to nomad job.

Suggestion(s)/solution(s) [Optional]

Follow hive pr Skatteetaten/terraform-nomad-hive#53

Optional

Creds with vault

related Skatteetaten/terraform-nomad-minio#66
related Skatteetaten/terraform-nomad-postgres#45
related Skatteetaten/terraform-nomad-hive#44

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Add tests

Upgrade to 0.7.0

What is the issue?

Upgrade and test v0.7.0 of the hashistack box.
https://app.vagrantup.com/fredrikhgrelland/boxes/hashistack/versions/0.7.0

Suggestion(s)/solution(s) [Optional]

Change Vagrantfile.default versioning to fetch newest box between 0.7 and 0.8.

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Automate release process

Automate the release process and add appropriate labels

patch
minor
major

Fix linter version

Blocker #9

Update module versions

What is the issue?

Need to update modules to latest versions

mino (waiting for release 0.2.0)
postgres (waiting for release 0.2.0)
hive (waiting for release 0.2.0)

Suggestion(s)/solution(s) [Optional]

Change in main.tf. https://github.com/fredrikhgrelland/terraform-nomad-presto/blob/master/example/main.tf#L57

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Remove certificate_handler as it is not needed.

After digging in the source code of presto I realized that presto can handle concatinated pem-formated files as well as jks. Moving the template stanzas into the server task to simplify.

`make clean` does not delete state directories from presto-cluster folder

Current behaviour

Make clean sletter ikke det som ligger under /examples/presto-cluster. Måtte slette dem manuelt ved å kjøre rm -rf .terrafom/ terraform.tfstate

Expected behaviour

.terraform/ og terraform.tfstate under /examples/presto-cluster slettes når man kjører make clean

How to reproduce?

Suggestion(s)/solution(s) [Optional]

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Update documentation header

Proper rendering of credentials

Feature description

As a user of the module we want proper render of credentials, without using vault provider

Why is it needed?

To make it easier to use the module.

Suggestion(s)/solution(s) [Optional]

Done in Presto module: https://github.com/fredrikhgrelland/terraform-nomad-presto/blob/master/conf/nomad/presto.hcl#L327

Definition of done

Rendering credentials in .hcl file

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Use latest template

Fix - ACL problems consul_acl_default_policy=deny

Skatteetaten/vagrant-hashistack#344

Add intentions documentation

What is the issue?

No documentation about intentions.

Suggestion(s)/solution(s) [Optional]

Add intetions documentation in README.md

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

`make up-standalone` does not work

Current behavior

Fail at resolving variables

TASK [Terraform presto standalone] *********************************************
fatal: [default]: FAILED! => {
    "changed": false
}

MSG:

Failed to validate Terraform configuration files:

Error: Unsupported argument

  on main.tf line 37, in module "presto":
  37:   shared_secret_provider = local.presto.shared_secret_provider

An argument named "shared_secret_provider" is not expected here.


Error: Unsupported argument

  on main.tf line 39, in module "presto":
  39:   shared_secret_vault = {

An argument named "shared_secret_vault" is not expected here.



PLAY RECAP *********************************************************************
default                    : ok=9    changed=1    unreachable=0    failed=1    skipped=12   rescued=0    ignored=0   

Ansible failed to complete successfully. Any error output should be
visible above. Please fix these errors and try again.
make: *** [up-standalone] Error 1

Expected behaviour

Up and running a standalone example

How to reproduce?

make clean
make up-standalone

Suggestion(s)/solution(s) [Optional]

Setup proper variables

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Remove locals block/make examples more readable

Feature description

The locals block in our examples is a little messy, and there is quite a bit of excess code.

Why is it needed?

Better readability

Suggestion(s)/solution(s) [Optional]

Remove the locals block and write the variables directly.

Definition of done

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Create premade templates for issues and PRs

Sync with template and update to use 0.6.0 box

Add `standalone` example

What is the issue?

Add standalone example for simplification

Suggestion(s)/solution(s) [Optional]

add new directory example/standalone or example/presto_standalone

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Add more flatten view example for json data

Feature description

Originally posted zhenik#1

Why is it needed?

Request from @k86021 , for further workshop

Suggestion(s)/solution(s) [Optional]

Definition of done

Automated SQL request to create VIEW

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Add auto scaling to nomad

Using the nomad autoscaler we could implement an autoscaler.
Remember, we can not kill nodes only add.

The APM could be prometheus with a jmx plugin scraping presto jmx emitter.

Adding healthcheck to presto_cluster example

Feature description

It is requested a better healthcheck for Presto in the presto_cluster example. Similar to the one in presto_standalone.hcl#L44-L52.

Why is it needed?

For better coverage and make sure presto is healthy 🧑‍⚕️

hive-availability
minio-availability

Suggestion(s)/solution(s) [Optional]

Need to do something similar as the one in presto_standalone.hcl#L44-L52.

Think we need to create a proxy service that is continuously checking the Presto service for us.

Definition of done

When we have a simliar feature as the one in presto_standalone.hcl#L44-L52.

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

enable pki backend fails, certificate error

Current behaviour

Error message:

TASK [service_bootstrap : vault - post/pki - enable PKI backend] ***************
fatal: [default]: FAILED! => {
    "changed": false
}

MSG:

Failed to initialize Terraform modules:

Error: Failed to install provider

Error while installing hashicorp/vault v2.15.0: could not query provider
registry for registry.terraform.io/hashicorp/vault: failed to retrieve
authentication checksums for provider: the request failed after 2 attempts,
please try again later: Get
"https://releases.hashicorp.com/terraform-provider-vault/2.15.0/terraform-provider-vault_2.15.0_SHA256SUMS":
x509: certificate signed by unknown authority



PLAY RECAP *********************************************************************
default                    : ok=23   changed=5    unreachable=0    failed=1    skipped=6    rescued=0    ignored=0

Ansible failed to complete successfully. Any error output should be
visible above. Please fix these errors and try again.
make: *** [up] Error 1

Expected behaviour

Succesful run

How to reproduce?

make up from root

Suggestion(s)/solution(s) [Optional]

No idea just yet

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Add test verifying that example pass all consul healthchecks

What is the issue?

Lacking HTTP request to verify that presto health checks have passing status

Suggestion(s)/solution(s) [Optional]

Add ansible playbook. Example in terraform-nomad-minio module

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Implement "Review required: At least 2 approving reviews are required by reviewers with write access."

Bump versions of dependent modules

Feature description

bump

Why is it needed?

Update, new features, stability improvements

Suggestion(s)/solution(s) [Optional]

Definition of done

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone

Move the generation of vault credentials from ansible to terraform

Feature description

I suggest we move the whole generation of secrets used in the module over to the module itself, and remove it from the ansible scripts. The ansible scripts are not part of the module, meaning anyone using our module would need to create secrets in their vault before using this. We could still keep all the funtionality we have now, of being able to use user-provided secrets, as well as setting a custom path to the vault secrets, but also bundle in a creation and usage of secrets in vault with the module itself.

Why is it needed?

User experience

Suggestion(s)/solution(s) [Optional]

Take this part, and convert it to terraform code
https://github.com/fredrikhgrelland/terraform-nomad-presto/blob/6fbb7ae2e50cd6c06fab59526ef6a103872aaaae/dev/ansible/00_generate_secrets_vault.yml#L1-L10

Using the vault provider

Definition of done

Ansible code to generate secrets is moved into the terraform code in the module itself
All existing features are kept

Checklist (after created issue)

Added label(s)
Added to project
Added to milestone