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

[newrelic-logging] crashes on install if cluster name is not set #899

Open
maxlemieux opened this issue Jul 29, 2022 · 4 comments
Open

[newrelic-logging] crashes on install if cluster name is not set #899

maxlemieux opened this issue Jul 29, 2022 · 4 comments

Comments

@maxlemieux
Copy link
Contributor

Bug description

When installing the newrelic-logging chart as a standalone chart, the pods crash if the cluster name is not set.

Version of Helm and Kubernetes

Helm 3, Kubernetes 1.24.1

Which chart?

newrelic-logging 1.11.2

What happened?

Container goes into crashLoopBackOff with these logs:

~ k logs newrelic-logging-dnpr6 
Fluent Bit v1.9.4
* Copyright (C) 2015-2022 The Fluent Bit Authors
* Fluent Bit is a CNCF sub-project under the umbrella of Fluentd
* https://fluentbit.io

[2022/07/29 20:17:55] [ warn] [env] variable ${CLUSTER_NAME} is used but not set
[2022/07/29 20:17:55] [ info] [fluent bit] version=1.9.4, commit=08de43e474, pid=1
[2022/07/29 20:17:55] [ info] [storage] version=1.2.0, type=memory-only, sync=normal, checksum=disabled, max_chunks_up=128
[2022/07/29 20:17:55] [ info] [cmetrics] version=0.3.1
[2022/07/29 20:17:55] [ info] [filter:kubernetes:kubernetes.0] https=1 host=kubernetes.default.svc.cluster.local port=443
[2022/07/29 20:17:55] [ info] [filter:kubernetes:kubernetes.0]  token updated
[2022/07/29 20:17:55] [ info] [filter:kubernetes:kubernetes.0] local POD info OK
[2022/07/29 20:17:55] [ info] [filter:kubernetes:kubernetes.0] testing connectivity with API server...
[2022/07/29 20:17:55] [ warn] [filter:kubernetes:kubernetes.0] could not get meta for POD minikube
[2022/07/29 20:17:55] [error] [config map] property 'Record' expects 2 values (only 1 were found)
[2022/07/29 20:17:55] [error] [lib] backend failed
[2022/07/29 20:17:55] [error] [plugins/filter_record_modifier/filter_modifier.c:81 errno=25] Inappropriate ioctl for device
[2022/07/29 20:17:55] [error] [filter:record_modifier:record_modifier.1] configuration error
[2022/07/29 20:17:55] [error] Failed initialize filter record_modifier.1
[2022/07/29 20:17:55] [ info] [input] pausing tail.0

This error points to this area of config: https://github.com/newrelic/helm-charts/blob/master/charts/newrelic-logging/values.yaml#L102

What you expected to happen?

Expected container to start successfully.

How to reproduce it?

Following the current directions in the README will reproduce the issue:

helm install newrelic-logging newrelic/newrelic-logging --set licenseKey=MY_INGEST_LICENSE_KEY

Anything else we need to know?

Workaround: set cluster name, as:

helm install newrelic-logging newrelic/newrelic-logging --set licenseKey=MY_INGEST_LICENSE_KEY --set cluster=my-cluster

This results in a successful installation.

This issue may have been overlooked since the chart bundle typically has the cluster name set during guided installation.

@kang-makes
Copy link
Contributor

Hi @maxlemieux,

The expected behavior we have in other charts is to fail in case cluster is not defined: https://github.com/newrelic/helm-charts/blob/master/library/common-library/templates/_cluster.tpl#L13

You can use the common library to have the same behavior as we have with the rest of the charts (common-library docs) like we have in nri-prometheus

@maxlemieux
Copy link
Contributor Author

Thanks @kang-makes for the clarification. In that case, would it be reasonable for me to file a pull request to update the docs to include the global.cluster setting in the install command which is documented for the standalone newrelic-logging chart? That way, users who are arriving at the standalone chart docs will have a better experience until the common library is implemented.

@kang-makes
Copy link
Contributor

In that case, would it be reasonable for me to file a pull request to update the docs to include the global.cluster setting in the install command which is documented for the standalone newrelic-logging chart?

I am trying to find the documentation you are talking about but I am not able to find it.

The thing here is that this is still a bug. In case global.cluster or cluster is not defined the chart should not even install and fail before the deployment happens.

On the other hand, any extra documentation is always welcome. But I cannot talk for the logging team, only for the common-library and expected behaviors compared to other charts we are maintaining.

@maxlemieux
Copy link
Contributor Author

The docs with the suggested install command for Helm are here. Adding one of the cluster name settings to this command could be a workaround for now:

https://github.com/newrelic/helm-charts/tree/master/charts/newrelic-logging#install-using-the-helm-chart-recommended

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants