Implement Kubernetes service deployer#246
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪 |
|
Thank you for taking a look, Christos! The problem with https://github.com/elastic/beats/blob/master/deploy/kubernetes/elastic-agent-kubernetes.yaml is that it deploys two agents for node and cluster. I'm not sure if we need two separate instances to test the integration (actually it's about endpoints/API not the actual deployment). Is this the way to go? Always in-pair? I'd like to stick to the principle that we should test integrations not the deployment model. It's more like an end-to-end testing, which we're not doing here. I'm happy to adjust the PR to better fit our needs, but I'd like to understand if this model won't meet our requirements - won' be possible to test some Kubernetes resources. |
|
Well the model of deploying 2 instances was inherited from Metricbeat (https://github.com/elastic/beats/blob/v7.9.0/deploy/kubernetes/metricbeat-kubernetes.yaml). However, now I'm thinking of it again and it should be ok to test all datastreams with only one single Agent, both manually but also with system tests so you can just by-pass my previous comment and go ahead with your design. Thanks! |
| } | ||
| currentContext := string(bytes.TrimSpace(output)) | ||
|
|
||
| if currentContext != "kind-kind" { |
There was a problem hiding this comment.
| if currentContext != "kind-kind" { | |
| if currentContext != kindContext { |
| } | ||
| } | ||
|
|
||
| logger.Debugf("attach service container %s (ID: %s) to stack network %s", kindControlPlaneContainerName, controlPlaneContainerID, stackNetwork) |
There was a problem hiding this comment.
| logger.Debugf("attach service container %s (ID: %s) to stack network %s", kindControlPlaneContainerName, controlPlaneContainerID, stackNetwork) | |
| logger.Debugf("attach kind control plane container %s (ID: %s) to stack network %s", kindControlPlaneContainerName, controlPlaneContainerID, stackNetwork) |
There was a problem hiding this comment.
I adjusted the comment to the similar form.
internal/testrunner/runners/system/servicedeployer/kubernetes.go
Outdated
Show resolved
Hide resolved
| cmd := exec.Command("kubectl", args...) | ||
| errOutput := new(bytes.Buffer) | ||
| cmd.Stderr = errOutput | ||
| if err := cmd.Run(); err != nil { | ||
| return errors.Wrapf(err, "kubectl apply failed (stderr=%q)", errOutput.String()) | ||
| } | ||
| return nil |
There was a problem hiding this comment.
We are doing something similar in verifyKindContext. Does it make sense to extract this part into its own function for running generic kubectl commands?
There was a problem hiding this comment.
Maybe these two functions are not good candidates to get unified, but this might a good point to extract it to the kind package.
There was a problem hiding this comment.
++ to kind package. That would be even better!
There was a problem hiding this comment.
Fixed. Introduced kind, kubectl and added some changes to docker package.
|
jenkins run the tests please |
ycombinator
left a comment
There was a problem hiding this comment.
LGTM.
This was not an easy service deployer to build, IMO. Very good job at iterating on it and then ultimately arriving at this PR.
|
Thank you for reviewing. Let's hold on with merging this PR immediately as we need the changes in Agent to go through first: elastic/beats#24001 . Not sure in which branch it will land finally, in the worst case I will bump the stack version to 7.12.0-SNAPSHOT. |
|
/test |
This reverts commit 764dfd2.
|
I assume that the next revision of Elastic Agent will contain the bugfix (default policy selection) and I'll be able to merge this PR. |
|
/test |
|
The CI job has finally passed. I'll merge this one and work on enabling it in integrations (install kind). |
Issue: #239
How to use it:
TODOs:
kind-kindkindcontainer to the Elastic stack networkkindandkubectl- [ ] Update system docsI'll extract this to a separate PR, so we can push the implementation and write docs based on the pushed state.