Skip to content
This repository has been archived by the owner on Feb 15, 2023. It is now read-only.

[NETOBSERV-58] Merge loki-exporter into kube-enricher #10

Merged
merged 11 commits into from
Oct 21, 2021

Conversation

jpinsonneau
Copy link
Contributor

updated go version 1.15 & dependencies versions
added vendor
configured goflow-kube.yaml
call loki.Process from reader Start

imagePullPolicy: IfNotPresent
name: goflow
resources:
limits:
Copy link
Member

@jotak jotak Oct 20, 2021

Choose a reason for hiding this comment

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

just curious why there is this limit, does that work better for you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I don't put limits, I get warning in vs code. Did you installed Go & YAML extensions ?

One or more containers do not have resource limits - this could starve other processes
containers: List of containers belonging to the pod. Containers cannot currently be added or removed. There must be at least one container in a Pod. Cannot be updated.

A single application container that you want to run within a pod.

args (string[])

Arguments to the entrypoint. The docker image's CMD is used if this is not provided. Variable references $(VAR_NAME) are expanded using the container's environment. If a variable cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, regardless of whether the variable exists or not. Cannot be updated. More info: https://kubernetes.io/docs/tasks/inject-data-application/define-command-argument-container/#running-a-command-in-a-shell

Copy link
Member

Choose a reason for hiding this comment

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

thanks, good to know. No I haven't installed any extension for yaml.

jotak
jotak previously approved these changes Oct 20, 2021
Copy link
Member

@jotak jotak left a comment

Choose a reason for hiding this comment

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

LGTM thanks @jpinsonneau
(just not forget about the follow-up task for removing the json encode/decode)

@jpinsonneau
Copy link
Contributor Author

are we good for merge ?

Copy link
Member

@jotak jotak left a comment

Choose a reason for hiding this comment

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

LGTM thank you @jpinsonneau !
I think we can still simplify further by removing the scanner and the json encode but we can do that in a follow-up

@jotak jotak merged commit 89c8fb9 into netobserv:main Oct 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants