Skip to content
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

Migrate tink-server to cobra and viper #431

Merged
merged 2 commits into from
Feb 17, 2021
Merged

Conversation

gianarb
Copy link
Contributor

@gianarb gianarb commented Feb 3, 2021

This PR refactors how tink-server starts.
It uses Cobra and Viper same as tink-cli and tink-worker.

Right now it tries to keep compatibility with the old way of doing
things.

TODO:

  • run this version in sandbox without any change. it should work like before
  • Unit test (project aborted, I don't know how to mock env var!)

Signed-off-by: Gianluca Arbezzano gianarb92@gmail.com

@gianarb
Copy link
Contributor Author

gianarb commented Feb 3, 2021

@displague you did a similar job for the other binaries. Can you have a look and tell me if it makes sense? I know you don't like to keep "useless retro-compatibility" but the code will get removed soon trust me!! 😇

Thanks

@codecov
Copy link

codecov bot commented Feb 3, 2021

Codecov Report

Merging #431 (06ce919) into master (a7e947e) will increase coverage by 0.08%.
The diff coverage is 23.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #431      +/-   ##
==========================================
+ Coverage   34.73%   34.82%   +0.08%     
==========================================
  Files          47       47              
  Lines        2893     2886       -7     
==========================================
  Hits         1005     1005              
+ Misses       1796     1789       -7     
  Partials       92       92              
Impacted Files Coverage Δ
grpc-server/events.go 0.00% <0.00%> (ø)
grpc-server/grpc_server.go 0.00% <0.00%> (ø)
grpc-server/hardware.go 1.55% <0.00%> (ø)
http-server/http_server.go 0.00% <0.00%> (ø)
grpc-server/workflow.go 28.80% <36.84%> (ø)
grpc-server/template.go 36.53% <47.05%> (ø)
grpc-server/tinkerbell.go 92.12% <66.66%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7e947e...550c63a. Read the comment docs.

@gianarb
Copy link
Contributor Author

gianarb commented Feb 3, 2021

@detiber I know you have this PR #266 in the fight but let's hold (it is pretty stale already) it until we figure out #432

@gianarb gianarb force-pushed the chore/tink-server-startup branch 3 times, most recently from 71a8d4a to e9c3b74 Compare February 3, 2021 14:08
@gianarb gianarb force-pushed the chore/tink-server-startup branch 2 times, most recently from e70bf26 to 87f1637 Compare February 9, 2021 15:00
@gianarb gianarb added the ready-to-merge Signal to Mergify to merge the PR. label Feb 9, 2021
mmlb
mmlb previously approved these changes Feb 9, 2021
@gianarb
Copy link
Contributor Author

gianarb commented Feb 9, 2021

$ go run cmd/tink-server/main.go --help
Usage:
  tink-server [flags]

Flags:
      --cert-dir string
      --facility string            This is temporary. It will be removed (default "deprecated")
      --grpc-authority string      The address used to expose the gRPC server (default ":42113")
  -h, --help                       help for tink-server
      --http-authority string      The address used to expose the HTTP server (default ":42114")
      --only-migration             When enabled the server applies the migration to postgres database and it exits
      --postgres-database string   The Postgres database name (default "tinkerbell")
      --postgres-password string   The Postgres database password (default "tinkerbell")
      --postgres-sslmode string    Enable or disable SSL mode in postgres (default "disable")
      --postgres-user string       The Postgres database username (default "tinkerbell")
      --tls-cert string

mmlb
mmlb previously approved these changes Feb 9, 2021
Gianluca Arbezzano added 2 commits February 11, 2021 21:39
This PR refactors how tink-server starts.
It uses Cobra and Viper same as tink-cli and tink-worker.

Right now it tries to keep compatibility with the old way of doing
things.

Signed-off-by: Gianluca Arbezzano <gianarb92@gmail.com>
Signed-off-by: Gianluca Arbezzano <gianarb92@gmail.com>
@displague displague changed the title Migrate tink-server to cobra and viber Migrate tink-server to cobra and viper Feb 12, 2021
@gianarb gianarb requested a review from mmlb February 12, 2021 11:45
@gianarb gianarb removed the request for review from displague February 17, 2021 14:46
@mergify mergify bot merged commit a41538d into master Feb 17, 2021
@mergify mergify bot deleted the chore/tink-server-startup branch February 17, 2021 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Signal to Mergify to merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants