-
Notifications
You must be signed in to change notification settings - Fork 7
Add kubernetes-plugins ci initial test #121
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
Conversation
Signed-off-by: ryanohnemus <ryanohnemus@gmail.com>
Yeah I think that's fair enough, I was thinking it potentially could be done with stdout and then checking the output of the pods or another output like Loki, etc. (we could also use a PV for file access) but this is fine and works. Just need to check why it did not run the integration tests in this case. We may need to run them for the specific branch but I was hoping it would trigger on PR to confirm the test actually passes before we merge. Potentially we could consider using the debug container but I think integration tests really should be on the production deployment. It may mean some extra hoops to jump through but ensures no delta in the production container. |
Signed-off-by: ryanohnemus <ryanohnemus@gmail.com>
|
@patrick-stephens I like your idea of using stdout plugin much better than my original version. I just pushed a new update and removed the elasticsearch output dependency. This version works by creating fluentbit that will read container logs that are in the Let me know your thoughts! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor niggles but otherwise looks good to me.
Signed-off-by: ryanohnemus <ryanohnemus@gmail.com>
|
@patrick-stephens - ok i just pushed the latest version. bats-detik version was updated, it looked like some of the other libs had higher versions but i only updated the one i needed to fix that |
|
@patrick-stephens just pushed another diff, please take a look when you get a chance! |
Signed-off-by: ryanohnemus <ryanohnemus@gmail.com>
c8483c4 to
09b89e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine, I'd like to verify it runs successfully though - I'm not sure why the PR is not running it but we can potentially explicitly run the workflow for a branch or if you can confirm it is ok manually?
|
@ryanohnemus does this pass locally for you btw? |
|
|
Can you rebase and push a new commit to your original PR so it can trigger the new tests? |
|
@patrick-stephens - ok i created a new draft pr #122 over here, but i still don't see it automatically kicking off a run? |
|
Sorry I meant to your Fluent Bit PR - the namespace label support there. |
Adds an initial CI test folder with a single test to check if the kubernetes filter plugin is enhancing records with k8s pod labels.
We can add kubernetes_namespace label checks after fluent/fluent-bit#8279 has merged.
NOTE: Since the fluentbit container does not have tar available, I wasn't able to just use a file output with a
kubectl cpand instead just use another output plugin (elasticsearch) to be able to fetch and check the results easily. (Obviously this will look very similar to the elasticsearch-basic ci/bats test).