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

Add app-change describe command to allow the user to view diffs for deployed changes #338

Open
3 of 5 tasks
100mik opened this issue Oct 6, 2021 · 1 comment
Open
3 of 5 tasks
Assignees
Labels
carvel accepted This issue should be considered for future work and that the triage process has been completed enhancement This issue is a feature request

Comments

@100mik
Copy link
Contributor

100mik commented Oct 6, 2021

Describe the problem/challenge you have
As of today, kapp app-change list -a app_name lists the duration of changes deployed for an application and displays a summary of the operations performed. This feature could be extended to provide the user more insight by making the text diff for each change available.

Describe the solution you'd like
The config maps used to track app-changes can be used to store text diffs as well. And these can be exposed to the user through an app-change describe command.

The end result might be similar to this:

❯ kapp app-change list -a test                                                                                                                                                                                             ─╯
Target cluster 'https://127.0.0.1:57406' (nodes: minikube)

App changes

Name               Started At            Finished At           Successful  Description  
test-change-vx7z5  2021-10-06T09:12:36Z  2021-10-06T09:12:36Z  true        update: Op: 1 create, 0 delete, 1 update, 0 noop / Wait to: 2 reconcile, 0 delete, 0 noop  
test-change-hcwrt  2021-10-06T09:11:56Z  2021-10-06T09:11:56Z  true        update: Op: 2 create, 0 delete, 0 update, 0 noop / Wait to: 2 reconcile, 0 delete, 0 noop  

2 app changes

Succeeded

❯ kapp app-change describe -a test --app-change test-change-vx7z5

@@ create configmap/config-1-ver-2 (v1) namespace: default @@
  ...
  1,  1   data:
  2     -   foo1: bar
      2 +   foo: bar
  3,  3   kind: ConfigMap
  4,  4   metadata:
@@ update configmap/config-2 (v1) namespace: default @@
  ...
  8,  8         kind: ConfigMap
  9     -       name: config-1-ver-1
      9 +       name: config-1-ver-2
 10, 10     creationTimestamp: "2021-10-06T09:11:56Z"
 11, 11     labels:

Acceptance Criteria

  • Text diffs for app changes should be serialised and stored in app-change config maps in addition to any other information that might add value.
  • Expose command app-change describe app_change_name which presents the information for that particular change in an intuitive view.
  • Make a flag --app-change-diff available, which can let the user to specify that they do not want to store the diff for a particular change.

Checks needed

  • Text diffs should be truncated if greater than 0.5 MB in size keeping in mind etcd limitations.
  • Consider special cases that might cause noisy diffs. (Like resources with update-strategy: skip)

Anything else you would like to add:

This will provide much needed observability into changes being applied by kapp in scenarios where a kapp deploy is automatically triggered periodically or due to an action.


Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

@100mik 100mik added enhancement This issue is a feature request carvel triage This issue has not yet been reviewed for validity labels Oct 6, 2021
@100mik
Copy link
Contributor Author

100mik commented Oct 6, 2021

We should be allowing users to easily exclude text-diff for certain resources. This could be done by:

  • An annotation like `kapp.k14s.io/ignore-app-change-diff.
  • Allowing the user to specify conditions in Config.

Does it make sense to have something like this in addition to --app-change-diff flag or should these be replacing it?

@100mik 100mik self-assigned this Oct 7, 2021
@renuy renuy added carvel accepted This issue should be considered for future work and that the triage process has been completed and removed carvel triage This issue has not yet been reviewed for validity labels Oct 29, 2021
@renuy renuy linked a pull request Feb 7, 2022 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
carvel accepted This issue should be considered for future work and that the triage process has been completed enhancement This issue is a feature request
Projects
Status: To Triage
Development

Successfully merging a pull request may close this issue.

2 participants