Add support for an embedded NATS server

[bruth]

@brandond Just an FYI that I will be a pushing a few more commits, refactoring the code, simplifying it, and adding tests where appropriate.

One question I did have was whether the CI that runs is the canonical/sufficient test suite that changes need to be tested against? Also one heads up is that I saw there was indeed a failure (which I am fixing) in the CI, but it still shows the CI passed: https://drone-pr.k3s.io/k3s-io/kine/147/1/3

[bruth]

Sorry, one more question. There are various log statements being emitted within the NATS driver. Is there any policy/best practice recommended for logging given it will be embedded within k3s?

[brandond]

Just an FYI that I will be a pushing a few more commits, refactoring the code, simplifying it, and adding tests where appropriate.

Are you still working on this?

whether the CI that runs is the canonical/sufficient test suite that changes need to be tested against?

The CI is sufficient to confirm that it works with K3s, but our QA will still want to do a full set of tests on it when we update the Kine that is embedded in K3s itself.

There are various log statements being emitted within the NATS driver. Is there any policy/best practice recommended for logging given it will be embedded within k3s?

Don't be overly chatty to the point where it obscures the K3s logs. Also keep in mind that K3s will never set the log level to TRACE, so if you ever want to see TRACE-level logs you'll need to run it standalone.

It increases the size by ~26% (7,768,648 bytes).

For the build flag, I would like to see the NATS server stuff stubbed out so that it just returns an error if someone attempts to use the embedded server when it's not built in. All of the related code should use the build flag to ensure that we're not pulling in any extra modules when it's not enabled. I'm not sure we'll want to enable the embedded server in K3s, as 7MB is a non-trivial addition to the binary size.

[bruth]

Thanks for the response @brandond.

Are you still working on this?

Absolutely. I paused after the hackathon due to other priorities as well as wanting answers to these questions so I didn't go down a wrong path for the contribution.

The CI is sufficient to confirm that it works with K3s, ...

Ok. I will assume if the CI passes, that will be sufficient to then move onto the QA review and we can address more issues there.

Don't be overly chatty...

Understood, thanks.

For the build flag, I would like to see the NATS server stuff stubbed out so that it just returns an error...

Will address, thanks.

[bruth]

Just checking that I have picked up on this again and will have updates by tomorrow (Friday).

[bruth]

I will rebase for the Go deps.

[brandond]

I did a quick test of K3s with the nats server built in an it's actually not too terrible; it adds just over 1MiB to the shipping binary size.

// without nats
k3s binary dist/artifacts/k3s size 68452352 is less than max acceptable size of 73400320 bytes (70 MiB)
// with nats
k3s binary dist/artifacts/k3s size 69722112 is less than max acceptable size of 73400320 bytes (70 MiB)

I did notice that even when running with dontListen, the embedded server still seems to be listening? thats what the log says at least. I am also confused by the fact that it reports that it's connecting to a server at 0.0.0.0 which isn't possible.

brandond@dev01:~/go/src/github.com/k3s-io/k3s$ k3s server --token=token '--datastore-endpoint=nats://?embedServer&dontListen'
INFO[0000] Starting k3s v1.27.1+k3s-3988142f (3988142f)
INFO[0000] using an embedded NATS server
INFO[0000] started embedded NATS server
INFO[0000] using bucket: kine
INFO[0000] connecting to nats://0.0.0.0:4222
INFO[0000] Kine available at unix://kine.sock

[bruth]

^Hrm.. will check out why it is still listening or reporting as such.

[bruth]

Confirmed it was a misplaced log line, but does not actually listen.

[bruth]

@brandond If you are considering NATS can be built-in by default without requiring a build tag, then that may change the configuration design. Let me know either way and will update the code/docs to acknowledge that.

[brandond]

can be built-in by default without requiring a build tag

I'm not sure what you mean by this, but I am assuming you're talking about building K3s with or without the build tag to enable the embedded server?

I haven't run it by the rest of the team and our PM on the K3s side just yet. I think for Kine itself we can document it as available since we'll ship it that way in the release artifacts. Since the binary size impact for k3s is not terrible I think it should be doable over there as well.

[bruth]

I updated the example doc and decided to use the noEmbed query param to disable use of an embedded server when the server in fact embedded. This will make the embedded build default to use an embedded server.. and a non-embedded build expect to connect to an external server.

[brandond]

Ah, got it - so without the embedded server, the host:port are taken to be the address of an external server, and with it, they are the address to bind to?

[bruth]

Yes, correct.

[bruth]

Feel free to squash merge and keep the issue title. Thank you!

[bruth]

I appreciate the review and support!