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

Support custom release and namespace #431

Merged
merged 7 commits into from Feb 20, 2020

Conversation

vsinghal13
Copy link
Contributor

@vsinghal13 vsinghal13 commented Feb 19, 2020

Description

This PR makes use of a new configmap to store the release name and using the k8s downward API for accessing the namespace where the pods are deployed. These two params are then exposed as env variables to fluent-bit and prometheus, to make the configuration dynamic.

Testing performed
  • ci/build.sh
  • Redeploy fluentd and fluentd-events pods
  • Confirm events, logs, and metrics are coming in
  • Verified helm installation with a custom release name and different namespace

- name: CHART
valueFrom:
configMapKeyRef:
name: sumologic-configmap
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we tested the non-helm case? What happens if the configmap has not been created yet at the time of prometheus install? Will this be able to pick up the values from the configmap later when it is created? Same for fluent-bit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the configmap creation is part of the fluentd-sumologic.yaml.tmpl, it will be created as the first step as per the Non-Helm Installation docs, so it will be available by default. https://github.com/SumoLogic/sumologic-kubernetes-collection/blob/v0.14.0/deploy/docs/Non_Helm_Installation.md#deploy-fluentd

Copy link
Contributor

@frankreno frankreno left a comment

Choose a reason for hiding this comment

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

LGTM, one question, have you tested the upgrade path?

@vsinghal13
Copy link
Contributor Author

LGTM, one question, have you tested the upgrade path?

@frankreno Yes, verified the upgrade path as well. It is working fine.

@vsinghal13 vsinghal13 merged commit e04a396 into master Feb 20, 2020
@vsinghal13 vsinghal13 deleted the vsinghal-support-custom-release-and-namespace branch February 20, 2020 23:50
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

4 participants