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

Rename provision_task to janus_cli, add create-datastore-key command. #314

Merged
merged 8 commits into from Jul 18, 2022

Conversation

branlwyd
Copy link
Member

@branlwyd branlwyd commented Jul 15, 2022

This will be used to provision a datastore key for new environments.

A few notes:

  • The largest part of this PR is testing. I introduce a new kubernetes::EphemeralCluster type that turns up a cluster on creation & turns it back down once the value is dropped. This is implemented by shelling out to kind; I'm not too happy about shelling out, but I can't find an equivalent Rust library, and I suppose it's somewhat more acceptable in test code.
  • Due to issues with rustls (rustls cannot reach a cluster through ip kube-rs/kube#153), testing against local clusters only works with openssl. I have set up the feature flags such that release builds use rustls & dev builds use openssl by default. This makes things work, but means that our release builds will be using a different TLS library than our dev builds. Thankfully, this doesn't affect "in-cluster" usage, so I think we'll be fine once some version of this tool ends up migrated to run in the cluster.
  • Arguably, the refactor should have gone further: create-datastore-key doesn't need a datastore to exist at all, yet it must receive datastore config information, including (bogus) datastore keys. If we want to clean this up, I would recommend dropping the janus_main abstraction, and moving the different things that janus_main does into helper functions that each binary can call as-needed. I'm going to leave this for a later PR, however.

The testing is done via a new library used to spin up an ephemeral
Kubernetes cluster using `kind`. It interacts with `kind` by shelling
out; I'm not happy about this, but I can't find an equivalent library
for Rust.
Unfortunately, kube doesn't work with rustls when talking to local
clusters: kube-rs/kube#153

Thankfully, openssl is only a dependency if the test-utils feature is
enabled, so our release builds will not depend on openssl.
Unfortunately, that means that our release builds will be interacting
with Kubernetes using a different TLS library than our tests use.
Dockerfile Outdated Show resolved Hide resolved
It's no longer necessary since the crate features are set up to
(rightfully) not include openssl in release builds.
@branlwyd branlwyd changed the title Provision task datastore key Rename provision_task to janus_cli, add create-datastore-key command. Jul 15, 2022
@branlwyd branlwyd marked this pull request as ready for review July 15, 2022 19:57
@branlwyd branlwyd requested a review from a team as a code owner July 15, 2022 19:57
@branlwyd
Copy link
Member Author

(I suppose an obvious side-effect of this PR is that we'll now need to have kind installed on our workstations to run the tests. I think that's OK, since we're already planning on using kind for some local/integration testing.)

Copy link
Contributor

@tgeoghegan tgeoghegan left a comment

Choose a reason for hiding this comment

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

Looks fine, though I have a couple suggestions that can be punted to future enhancements. Last comment is that we should document the dependency on having kind in $PATH in a README.

janus_server/src/bin/janus_cli.rs Show resolved Hide resolved
janus_server/src/bin/janus_cli.rs Show resolved Hide resolved
Also, update a stale reference to "PPM" as the protocol name to "DAP".
@branlwyd
Copy link
Member Author

Updated the README to note the new kind dependency for tests.

@branlwyd branlwyd merged commit 7beee2a into main Jul 18, 2022
@branlwyd branlwyd deleted the provision-task-datastore-key branch July 18, 2022 16:04
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.

None yet

3 participants