Skip to content
This repository has been archived by the owner on Jan 25, 2019. It is now read-only.

pkg/helm: adding manifest diff logging #61

Merged
merged 1 commit into from
Oct 30, 2018
Merged

pkg/helm: adding manifest diff logging #61

merged 1 commit into from
Oct 30, 2018

Conversation

joelanford
Copy link
Member

Description of change
Adding diff output to operator logs to show resource changes for release installations, updates, and uninstallations.

Motivation for change
See #52. One of the suggestions is to help users troubleshoot and better understand the changes that are made to resources in their cluster as a result of changes to the CRs watched by the operator.

text := diff.Text
switch diff.Type {
case diffmatchpatch.DiffInsert:
_, _ = buff.WriteString("\x1b[32m")
Copy link
Collaborator

Choose a reason for hiding this comment

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

@AlexNPavel I recall we had some issues in seeing the colorized output while doing a diff for our unit tests. Do you remember if that was specific to what type of shell you use.

@joelanford Are you able to see the colorized output properly when you get the logs? Can you share a sample output?

Choose a reason for hiding this comment

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

Most shells should support colors. Some shells may not, and this print out [32m in the output. This shouldn't really happen though, as almost all shells support color (or at least ignore the entire block defining the color, which is fine here since we are doing line based diffs)

Copy link
Collaborator

@hasbro17 hasbro17 left a comment

Choose a reason for hiding this comment

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

LGTM

@joelanford joelanford merged commit c8ae742 into operator-framework:master Oct 30, 2018
@joelanford joelanford deleted the diff branch October 30, 2018 13:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants