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

control-service: Make fluentdconfig configurable #74

Merged

Conversation

gabrielgeorgiev1
Copy link
Contributor

@gabrielgeorgiev1 gabrielgeorgiev1 commented Aug 11, 2021

This change allows the fluentdconfig to be altered
through the Values.yml file. The purpose of this is
that making changes to a deployed version of
control-service no long requires pushing changes to
the main control-service repository.

Testing done:
With the following config in Values.yml:

fluentd:
  enabled: true
  filter: |-
    @type parser
    key_name log
    reserve_data true
    remove_key_name_field true
    <parse>
      @type json
    </parse>
  match: |-
    @type elastic

We get the following config:

apiVersion: logs.vdp.vmware.com/v1beta1
kind: FluentdConfig
metadata:
  name: pipelines-control-service-parser
spec:
  fluentconf: |
    <filter $labels(app_kubernetes_io/name=pipelines-control-service)>
      @type parser
      key_name log
      reserve_data true
      remove_key_name_field true
      <parse>
        @type json
      </parse>
    </filter>
    <match $labels(app_kubernetes_io/name=pipelines-control-service)>
      @type elastic
    </match>

If we extend the config the with a fluentd.extra value:

  extra: |-
    <source>
      @type http
      port 8090
    </source>
    <system>
      some system config
    </system>

We get the following fluentdconfig:

apiVersion: logs.vdp.vmware.com/v1beta1
kind: FluentdConfig
metadata:
  name: pipelines-control-service-parser
spec:
  fluentconf: |
    <source>
      @type http
      port 8090
    </source>
    <system>
      some system config
    </system>
    <filter $labels(app_kubernetes_io/name=pipelines-control-service)>
      @type parser
      key_name log
      reserve_data true
      remove_key_name_field true
      <parse>
        @type json
      </parse>
    </filter>
    <match $labels(app_kubernetes_io/name=pipelines-control-service)>
      @type elastic
    </match>

Signed-off-by: gageorgiev [email protected]

@gabrielgeorgiev1 gabrielgeorgiev1 force-pushed the person/gageorgiev/make-fluentdconfig-configurable branch 2 times, most recently from 1d9767a to 742e576 Compare August 11, 2021 08:51
@gabrielgeorgiev1 gabrielgeorgiev1 force-pushed the person/gageorgiev/make-fluentdconfig-configurable branch 4 times, most recently from 4e0e21e to 080b655 Compare August 11, 2021 15:03
@antoniivanov
Copy link
Collaborator

Please avoid said in testing done "local", "manual", those do not really tell anything to the reader.

Reminder from the git template
In case there is any manual tests needed describe the exact steps -
what were the commands, what was the output, what was expected, details of your testing environment.

Copy link
Collaborator

@antoniivanov antoniivanov left a comment

Choose a reason for hiding this comment

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

Everything looks good to me. I want to review how it was tested only. How we are sure the produced config is valid and working.

This change allows the fluentdconfig to be altered
through the Values.yml file. The purpose of this is
that making changes to a deployed version of
control-service no long requires pushing changes to
the main control-service repository. The fluentd
block in values.yml includes three values relevant to
the config - filter, match and extra. Filter and match
are inserted into the corresponding filter and match
blocks in the fluentdconfig, whereas extra is any
additional configuration you might include using the
remaining directives.

Testing done:
With the following config in Values.yml:
```yml
fluentd:
  enabled: true
  filter: |-
    @type parser
    key_name log
    reserve_data true
    remove_key_name_field true
    <parse>
      @type json
    </parse>
  match: |-
    @type elastic
```

We get the following config:
```yml
apiVersion: logs.vdp.vmware.com/v1beta1
kind: FluentdConfig
metadata:
  name: pipelines-control-service-parser
spec:
  fluentconf: |
    <filter $labels(app_kubernetes_io/name=pipelines-control-service)>
      @type parser
      key_name log
      reserve_data true
      remove_key_name_field true
      <parse>
        @type json
      </parse>
    </filter>
    <match $labels(app_kubernetes_io/name=pipelines-control-service)>
      @type elastic
    </match>
```

If we extend the config the with a fluentd.extra value:
```yml
  extra: |-
    <source>
      @type http
      port 8090
    </source>
    <system>
      some system config
    </system>
```

We get the following fluentdconfig:
```yml
apiVersion: logs.vdp.vmware.com/v1beta1
kind: FluentdConfig
metadata:
  name: pipelines-control-service-parser
spec:
  fluentconf: |
    <source>
      @type http
      port 8090
    </source>
    <system>
      some system config
    </system>
    <filter $labels(app_kubernetes_io/name=pipelines-control-service)>
      @type parser
      key_name log
      reserve_data true
      remove_key_name_field true
      <parse>
        @type json
      </parse>
    </filter>
    <match $labels(app_kubernetes_io/name=pipelines-control-service)>
      @type elastic
    </match>
```

Signed-off-by: gageorgiev <[email protected]>
@gabrielgeorgiev1 gabrielgeorgiev1 force-pushed the person/gageorgiev/make-fluentdconfig-configurable branch from 080b655 to 899e46b Compare August 12, 2021 08:25
Copy link
Collaborator

@antoniivanov antoniivanov left a comment

Choose a reason for hiding this comment

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

Thanks. Looks good to me.

@gabrielgeorgiev1 gabrielgeorgiev1 merged commit bb687f9 into main Aug 12, 2021
@gabrielgeorgiev1 gabrielgeorgiev1 deleted the person/gageorgiev/make-fluentdconfig-configurable branch August 12, 2021 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants