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

Added elasticsearch.suppressTypeName to values.yaml to support settin… #862

Merged
merged 2 commits into from
Jan 4, 2023

Conversation

codesometech
Copy link

…g Suppress_Type_Name for Opensearch 2.0 and above

Issue

#861

Description of changes

Added suppressTypeName to aws-for-fluent-bit's values.yaml and changed configmap template to include Suppress_Type_Name if a value is present.

Checklist

  • Added/modified documentation as required (such as the README.md for modified charts)
  • Incremented the chart version in Chart.yaml for the modified chart(s)
  • Manually tested. Describe what testing was done in the testing section below
  • [ X] Make sure the title of the PR is a good description that can go into the release notes

Testing

Manually tested including and excluding the value and deployed the chart on an EKS cluster

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…g Suppress_Type_Name for Opensearch 2.0 and above
Copy link
Contributor

@PettitWesley PettitWesley left a comment

Choose a reason for hiding this comment

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

This looks good to me from a Fluent Bit POV but I have not actually pulled it and tested it myself.

Copy link
Contributor

@PettitWesley PettitWesley left a comment

Choose a reason for hiding this comment

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

@codesometech Do you know how I can check that the template renders properly without actually deploying it. Currently the following is failing for me:

147dda285fd4:aws-for-fluent-bit wppttt$ helm template --dry-run .
Error: template: templates/psp.yaml:4:11: executing "templates/psp.yaml" at <include "aws-for-fluent-bit.fullname" .>: error calling include: template: no template "aws-for-fluent-bit.fullname" associated with template "gotpl"

Use --debug flag to render out invalid YAML
147dda285fd4:aws-for-fluent-bit wppttt$ helm template --dry-run --debug .
install.go:192: [debug] Original chart version: ""
install.go:209: [debug] CHART PATH: /Users/wppttt/logging-projects/eks-charts/stable/aws-for-fluent-bit


Error: template: templates/psp.yaml:4:11: executing "templates/psp.yaml" at <include "aws-for-fluent-bit.fullname" .>: error calling include: template: no template "aws-for-fluent-bit.fullname" associated with template "gotpl"
helm.go:84: [debug] template: templates/psp.yaml:4:11: executing "templates/psp.yaml" at <include "aws-for-fluent-bit.fullname" .>: error calling include: template: no template "aws-for-fluent-bit.fullname" associated with template "gotpl"
147dda285fd4:aws-for-fluent-bit wppttt$ helm template aws-for-fluent-bit
Error: non-absolute URLs should be in form of repo_name/path_to_chart, got: aws-for-fluent-bit
147dda285fd4:aws-for-fluent-bit wppttt$ helm template aws-for-fluent-bit .
Error: template: templates/psp.yaml:4:11: executing "templates/psp.yaml" at <include "aws-for-fluent-bit.fullname" .>: error calling include: template: no template "aws-for-fluent-bit.fullname" associated with template "gotpl"

Use --debug flag to render out invalid YAML
147dda285fd4:aws-for-fluent-bit wppttt$ helm template aws-for-fluent-bit --generate-name .
Error: cannot set --generate-name and also specify a name
147dda285fd4:aws-for-fluent-bit wppttt$ helm template --generate-name .
Error: template: templates/psp.yaml:4:11: executing "templates/psp.yaml" at <include "aws-for-fluent-bit.fullname" .>: error calling include: template: no template "aws-for-fluent-bit.fullname" associated with template "gotpl"

Use --debug flag to render out invalid YAML

@PettitWesley PettitWesley self-requested a review December 19, 2022 19:18
@PettitWesley PettitWesley dismissed their stale review December 19, 2022 19:19

don't want to block since I don't know what i'm doing just revoking my previous approval

@codesometech
Copy link
Author

Can you try helm template?

helm template aws-for-fluent-bit stable/aws-for-fluent-bit/ --values values.yaml > fb.template.yaml

@codesometech
Copy link
Author

codesometech commented Dec 19, 2022

helm template --dry-run .

helm template --dry-run . --set elasticsearch.enabled=true --set elasticsearch.suppressTypeName="On"

That command works fine for me. I am able to generate the template successfully

@PettitWesley
Copy link
Contributor

@codesometech Thanks! That command works for me and I see that the supress_type_name option is set.

@@ -94,6 +94,7 @@ helm delete aws-for-fluent-bit --namespace kube-system
| `elasticsearch.port` | TCP Port of the target service. | 443 |
| `elasticsearch.retryLimit` | Integer value to set the maximum number of retries allowed. N must be >= 1 | 6 |
| `elasticsearch.replaceDots` | Enable or disable Replace_Dots | On |
| `elasticsearch.suppressTypeName` | OpenSearch 2.0 and above needs to have type option being removed by setting Suppress_Type_Name On | |
Copy link
Contributor

Choose a reason for hiding this comment

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

So there's no default value for this, so if not specificied, its just not in the template?

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked this myself with the helm dry run command

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool

@PettitWesley
Copy link
Contributor

Thanks!

@PettitWesley
Copy link
Contributor

I'm gonna merge it... I checked dry run template command shown above + run the command without new option, both run without error. So I think this is safe 🤞🏻

@PettitWesley PettitWesley merged commit 5befc89 into aws:master Jan 4, 2023
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.

None yet

2 participants