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

[stable/chaoskube] Feature/structured log flag #10986

Merged
merged 3 commits into from
Apr 30, 2019

Conversation

syedimam0012
Copy link
Contributor

@syedimam0012 syedimam0012 commented Jan 30, 2019

What this PR does / why we need it:

This feature adds a new flag logFormat that can be set to either "text" (i.e. default) or "json" without affecting the existing behaviour.

Special notes for your reviewer:

PR in the source has to be merged before this one, which will return a new appVersion that requires to be updated here. Chart versionalso needs to be bumped.
@linki

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • DCO signed
  • Chart Version bumped
  • Variables are documented in the README.md

@helm-bot helm-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 30, 2019
@helm-bot helm-bot added Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 30, 2019
@helm-bot helm-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 30, 2019
@linki
Copy link
Contributor

linki commented Jan 30, 2019

@syedimam0012 Thank you!

  • Please leave out all the changed files that are not under ./stable/chaoskube.
  • Bump the chart version to: 0.15.0
  • Bump the appVersion and imageTag to: 0.13.0 (image will be pushed once we merge your PR)

@@ -40,6 +40,9 @@ excludedDaysOfYear:
# Set specific Timezone for Actions to take place
timezone: UTC

# If nothing set, defaults to "text". Switch to "json" to enable structured logging
logFormat:
Copy link
Contributor

Choose a reason for hiding this comment

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

You could use logFormat: text here and leave out the {{- if .Values.logFormat }} stuff in deployment.yaml.

I tend to like that more in this chart but ultimately you can decide 😄

Copy link
Contributor Author

@syedimam0012 syedimam0012 Feb 1, 2019

Choose a reason for hiding this comment

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

That's what I did initially before I had encountered logFormat: empty was failing. Happy to revert this and leave "text" in the helm values. ☺️

Copy link
Contributor

Choose a reason for hiding this comment

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

For upgrading it's actually better to do the if stuff. Thank you 😃

@helm-bot helm-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 1, 2019
@helm-bot helm-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 1, 2019
@linki
Copy link
Contributor

linki commented Feb 5, 2019

lgtm, thank you @syedimam0012 😀

@marc-sensenich
Copy link
Contributor

@linki would you be able to /approve this for it to be released?

@@ -1,7 +1,7 @@
apiVersion: v1
name: chaoskube
version: 0.14.0
appVersion: 0.12.1
version: 0.15.0
Copy link
Contributor

Choose a reason for hiding this comment

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

With #11780 this version number should change to 1.1.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @marc-sensenich for the update.
@linki would you want me to update this to 1.1.0 ?

@maorfr
Copy link
Member

maorfr commented Apr 22, 2019

/assign
/ok-to-test

@syedimam0012, is this good to go?
if so, would you mind rebasing?

@helm-bot helm-bot removed the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Apr 30, 2019
@helm-bot helm-bot added the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Apr 30, 2019
@syedimam0012
Copy link
Contributor Author

/assign
/ok-to-test

@syedimam0012, is this good to go?
if so, would you mind rebasing?

I think so @maorfr
I've just rebased again.

@maorfr
Copy link
Member

maorfr commented Apr 30, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 30, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maorfr, syedimam0012

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 30, 2019
@k8s-ci-robot k8s-ci-robot merged commit 8b9e72d into helm:master Apr 30, 2019
iNoahNothing pushed a commit to datawire/charts that referenced this pull request Apr 30, 2019
* [stable/chaoskube] add option to choose log format

Signed-off-by: Syed Imam <[email protected]>

* [stable/chaoskube] bumping Chart and image versions

Signed-off-by: Syed Imam <[email protected]>

* [stable/chaoskube] Version bump

Signed-off-by: Syed Imam <[email protected]>
dpkirchner pushed a commit to dpkirchner/charts that referenced this pull request May 9, 2019
* [stable/chaoskube] add option to choose log format

Signed-off-by: Syed Imam <[email protected]>

* [stable/chaoskube] bumping Chart and image versions

Signed-off-by: Syed Imam <[email protected]>

* [stable/chaoskube] Version bump

Signed-off-by: Syed Imam <[email protected]>
@linki
Copy link
Contributor

linki commented May 10, 2019

@syedimam0012 @marc-sensenich @maorfr Thank you!

goshlanguage pushed a commit to goshlanguage/charts that referenced this pull request May 17, 2019
* [stable/chaoskube] add option to choose log format

Signed-off-by: Syed Imam <[email protected]>

* [stable/chaoskube] bumping Chart and image versions

Signed-off-by: Syed Imam <[email protected]>

* [stable/chaoskube] Version bump

Signed-off-by: Syed Imam <[email protected]>
eyenx pushed a commit to eyenx/charts that referenced this pull request May 28, 2019
* [stable/chaoskube] add option to choose log format

Signed-off-by: Syed Imam <[email protected]>

* [stable/chaoskube] bumping Chart and image versions

Signed-off-by: Syed Imam <[email protected]>

* [stable/chaoskube] Version bump

Signed-off-by: Syed Imam <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). lgtm Indicates that a PR is ready to be merged. ok-to-test size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants