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

feat(new source): Inital kubernetes source implementation #893

Merged
merged 34 commits into from
Dec 13, 2019
Merged

feat(new source): Inital kubernetes source implementation #893

merged 34 commits into from
Dec 13, 2019

Conversation

ktff
Copy link
Contributor

@ktff ktff commented Sep 18, 2019

Implements 1. Decentralized configuration part of #260.

Implementation differs from showed example in:

  • Instead of passing vector configuration as argument, vector configuration is mounted as vector.toml file, and it's path passed as argument.

  • include_namespace option was added as it proved to be necessary in tests. (Edit: removed because it's implementation relied on unspecified behavior)

Tests are currently behind kubernetes-integration-tests feature flag as they require:

  • Access to Kubernetes cluster.

  • Tested vector image present on Kubernetes hosts. Currently they are pulling vector image from my repository.

Before merger, find a way to:

  • spin up local Kubernetes cluster
  • build/find tested vector image and copy it to every Kubernetes host
  • enable tests
  • test and accommodate, where possible, other non Docker container runtimes. Only distributed.yaml can be possibly changed by this. This is required mostly because *.log files ,which are being read, are symlinks to other files. Which means that they also have to be mounted to be accessible.

Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
@LucioFranco
Copy link
Contributor

@ktff Looking great! I will give this a proper review on Monday. For testing, I feel like setting up kube in our current test-stable job might be too much. So maybe we want to see if we can setup a specific test stage to spin up minikube and just run the kube tests.

Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
@ktff
Copy link
Contributor Author

ktff commented Sep 23, 2019

For testing, I feel like setting up kube in our current test-stable job might be too much. So maybe we want to see if we can setup a specific test stage to spin up minikube and just run the kube tests.

@LucioFranco I agree.

The stage should have installed kubectl and minikube. If possible, minikube that is running directly on host so that we can avoid step 2. That would also eliminate the need for virtualization. There are guides for that in Kubernetes documentation:

Then during testing stage:

  1. build/find tested vector image
  2. copy it to every Kubernetes host. (this could be avoided)
  3. minikube start
  4. run tests
  5. minikube delete

@LucioFranco
Copy link
Contributor

@ktff looks like there are already a few issues and people have solved adding minikube to circleci. I would like to see this added before we merge here as well as some docs. I will give the main code a review today.

Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
Copy link
Contributor

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

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

This is looking really good!! Left some comments inline.

Cargo.toml Outdated Show resolved Hide resolved
config/kubernetes/distributed.yaml Outdated Show resolved Hide resolved
config/kubernetes/distributed.yaml Outdated Show resolved Hide resolved
src/sources/kubernetes.rs Outdated Show resolved Hide resolved
src/sources/kubernetes.rs Outdated Show resolved Hide resolved
src/sources/kubernetes.rs Outdated Show resolved Hide resolved
src/sources/kubernetes.rs Outdated Show resolved Hide resolved
src/sources/kubernetes.rs Outdated Show resolved Hide resolved
src/sources/kubernetes.rs Outdated Show resolved Hide resolved
tests/kubernetes.rs Outdated Show resolved Hide resolved
ktff and others added 14 commits October 2, 2019 11:57
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>

Co-Authored-By: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>

Co-Authored-By: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>

Co-Authored-By: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>

Co-Authored-By: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>

Co-Authored-By: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>

Co-Authored-By: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>

Co-Authored-By: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
Necessary to use errors that #811 brought.

Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
@LucioFranco
Copy link
Contributor

@binarylogic I think the docs are ready to go. I would like to write more thorough guides for kubernetes but I don't think the docs need much more since right now this implementation is simple.

All that is left is your approval and the new svg.

@binarylogic
Copy link
Contributor

@LucioFranco we need to improve the docs quite a bit more. For example, there is no Kubernetes platform installation page. This is an important enough source that it warrants being thoughtful about the documentation and setup experience. I would like to review and work on the docs before we merge.

- run:
name: setup kubectl
command: |
curl -Lo kubectl https://storage.googleapis.com/kubernetes-release/release/${K8S_VERSION}/bin/linux/amd64/kubectl && chmod +x kubectl && sudo mv kubectl /usr/local/bin/
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you enlighten me how a developer would run these tests locally? Are there docs for this? Even better, is there a way to embed this into the current workflow?

Copy link
Contributor

Choose a reason for hiding this comment

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

All of the kubernetes tests are behind a kubernetes-integration-tests feature flag and not enabled by default.

minikube start
cargo test --features kubernetes-integration-tests

It will use your kube config that exists in ~/.kube/config. Minikube will place one for you or you can run this command to fetch the config from an existing cluster. It just needs some kube cluster.

curl http://<some kube cluster>/config > ~/.kube/config

The idea here is that if you have access to a kube cluster via kubectl then you can run the tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this documented?

Copy link
Contributor

Choose a reason for hiding this comment

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

Happy to add this to the contributing guide, will do that tomorrow. We can also provide a separate test command to test with minikube.

@binarylogic binarylogic self-requested a review October 17, 2019 22:28
@LucioFranco
Copy link
Contributor

@binarylogic I'm not sure what setup there is? Most of the setup stuff I can think about would involve being a guide since they would need to talk about deployment strategies. So I was not sure what direction we wanted to take these docs versus deployment strategies.

For example, there is no Kubernetes platform installation page

I'm not sure what you mean by this, do we have any examples of a platform installation page? I tried to follow how the other sources documented things.

Another thing to note is that this source only works if you run it as a DaemonSet and not as a sidecar which we want to support as another deployment strategy. So, in theory, this may not be the only source that users use in a kubernetes deployment. So because of that, I'm not sure this should contain our high-level kubernetes guides/setup but be a reference point for using this source with a per-node deployment style.

{% endcode-tabs-item %}
{% endcode-tabs %}

## Options
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this for now.

Due to the nature of this component, it offers a
[**best effort** delivery guarantee][docs.guarantees#best-effort-delivery].

### Deployment
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this belongs here, this is also hard to follow. Shouldn't we provide commands that user can copy/paste to quickly get setup?

@binarylogic binarylogic changed the title feat(new source): First part of inital kubernetes source implementation feat(new source): Inital kubernetes source implementation Oct 19, 2019
Signed-off-by: Kruno Tomola Fabro <krunotf@gmail.com>
Signed-off-by: Ben Johnson <bjohnson@binarylogic.com>
Signed-off-by: Ben Johnson <bjohnson@binarylogic.com>
@binarylogic
Copy link
Contributor

Just providing an update. I have merged in all of the upstream changes from master. This includes the new website/documentation. I am working on the installation docs as well as the use case page for Kubernetes so that we can announce it properly.

@binarylogic
Copy link
Contributor

Nice work on this! I'm going to merge so that we can unblock work. Documentation is being worked on.

@binarylogic binarylogic merged commit df7f478 into vectordotdev:master Dec 13, 2019
@ktff ktff deleted the kubernetes branch December 14, 2019 14:09
@ktff ktff restored the kubernetes branch December 15, 2019 16:31
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.

4 participants