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

Revert "revert all 1.0.0 changes" #509

Merged
merged 4 commits into from Mar 24, 2020
Merged

Revert "revert all 1.0.0 changes" #509

merged 4 commits into from Mar 24, 2020

Conversation

perk-sumo
Copy link
Contributor

@perk-sumo perk-sumo commented Mar 20, 2020

This reverts commit f25e71f.

Description

All the changes added to master since the revert:
https://github.com/SumoLogic/sumologic-kubernetes-collection/compare/3cdc6acd716cfa8a003dcb78948991d5dbe58d1a..revert-the-1.0-revert

This ^^ should show all the changes added in the meantime.
Please double check nothing is missing and nothing got reverted.

Testing performed
  • ci/build.sh
  • Redeploy fluentd and fluentd-events pods
  • Confirm events, logs, and metrics are coming in

Copy link
Contributor

@sumo-drosiek sumo-drosiek left a comment

Choose a reason for hiding this comment

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

Looks ok, could we strip all spaces before endline?

@perk-sumo
Copy link
Contributor Author

Looks ok, could we strip all spaces before endline?

I thought about it and I think maybe let's not do it?
I didn't want to introduce any new changes to not make this commit even more complicated.

Copy link
Contributor

@vsinghal13 vsinghal13 left a comment

Choose a reason for hiding this comment

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

LGTM except for some minor things

Comment on lines +257 to +258
kubelet:
outputConf: |-
Copy link
Contributor

Choose a reason for hiding this comment

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

@samjsong do you want to add the enabled flags for the log types in this PR itself?

Copy link
Contributor

Choose a reason for hiding this comment

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

let's include these in a separate PR if we choose to do this 👍


## Systemd log configuration
systemd:
outputConf: |-
Copy link
Contributor

Choose a reason for hiding this comment

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

same enabled flag

deploy/helm/sumologic/values.yaml Outdated Show resolved Hide resolved
deploy/helm/sumologic/values.yaml Outdated Show resolved Hide resolved
deploy/helm/sumologic/values.yaml Outdated Show resolved Hide resolved
@perk-sumo perk-sumo force-pushed the revert-the-1.0-revert branch 2 times, most recently from 2bf5388 to ea257ba Compare March 23, 2020 08:48
@sumo-drosiek
Copy link
Contributor

@perk-sumo It would be easier to review if you not use force-push, please
It's hard to catch what you changed, and you can squash and force-push after all :)

Copy link
Contributor

@sumo-drosiek sumo-drosiek left a comment

Choose a reason for hiding this comment

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

LGTM

@perk-sumo
Copy link
Contributor Author

@perk-sumo It would be easier to review if you not use force-push, please
It's hard to catch what you changed, and you can squash and force-push after all :)

If you use the https://github.com/SumoLogic/sumologic-kubernetes-collection/pull/509/files and check the files you've already seen as Viewed then GH shows only the ones that changed.
It might help you track the changes and I wont be blocked with rebasing :)

@sumo-drosiek
Copy link
Contributor

@perk-sumo It would be easier to review if you not use force-push, please
It's hard to catch what you changed, and you can squash and force-push after all :)

If you use the https://github.com/SumoLogic/sumologic-kubernetes-collection/pull/509/files and check the files you've already seen as Viewed then GH shows only the ones that changed.
It might help you track the changes and I wont be blocked with rebasing :)

Rebases. It explains everything 👍

Copy link
Contributor

@samjsong samjsong left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Perk!

@perk-sumo perk-sumo merged commit 2e68994 into master Mar 24, 2020
@perk-sumo perk-sumo deleted the revert-the-1.0-revert branch March 24, 2020 09:27
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