Update logic handling failures for kapp-controller custom resources

[100mik]

What this PR does / why we need it:

It ensures that kapp shows the usefulErrorMessage rather than a message pointing towards it when there is a failure in a kapp-controller custom resource

Does this PR introduce a user-facing change?

Improve failure messages for kapp-controller resources

Review Checklist:

• Follows the developer guidelines
• Relevant tests are added or updated
• Relevant docs in this repo added or updated
• Relevant carvel.dev docs added or updated in a separate PR and there's
  a link to that PR
• Code is at least as readable and maintainable as it was before this
  change

Additional documentation e.g., Proposal, usage docs, etc.:

[praveenrewar]

Added a few comments for Apps which can be extrapolated for Pkgi and Pkgr

[100mik]

Old output:

Target cluster 'https://127.0.0.1:54877' (nodes: minikube)

Changes

Namespace  Name      Kind  Age  Op      Op st.  Wait to    Rs  Ri  
default    dev-test  App   -    create  -       reconcile  -   -  

Op:      1 create, 0 delete, 0 update, 0 noop, 0 exists
Wait to: 1 reconcile, 0 delete, 0 noop

Continue? [yN]: y

4:28:15PM: ---- applying 1 changes [0/1 done] ----
4:28:15PM: create app/dev-test (kappctrl.k14s.io/v1alpha1) namespace: default
4:28:15PM: ---- waiting on 1 changes [0/1 done] ----
4:28:15PM: ongoing: reconcile app/dev-test (kappctrl.k14s.io/v1alpha1) namespace: default
4:28:15PM:  ^ Waiting for generation 1 to be observed
4:28:18PM: fail: reconcile app/dev-test (kappctrl.k14s.io/v1alpha1) namespace: default
4:28:18PM:  ^ Reconcile failed:  (message: Deploying: Error (see .status.usefulErrorMessage for details))

kapp: Error: waiting on reconcile app/dev-test (kappctrl.k14s.io/v1alpha1) namespace: default:
  Finished unsuccessfully (Reconcile failed:  (message: Deploying: Error (see .status.usefulErrorMessage for details)))

vs
New output:

Target cluster 'https://127.0.0.1:54877' (nodes: minikube)

Changes

Namespace  Name      Kind  Age  Op      Op st.  Wait to    Rs  Ri  
default    dev-test  App   -    create  -       reconcile  -   -  

Op:      1 create, 0 delete, 0 update, 0 noop, 0 exists
Wait to: 1 reconcile, 0 delete, 0 noop

Continue? [yN]: y

4:34:13PM: ---- applying 1 changes [0/1 done] ----
4:34:13PM: create app/dev-test (kappctrl.k14s.io/v1alpha1) namespace: default
4:34:13PM: ---- waiting on 1 changes [0/1 done] ----
4:34:13PM: ongoing: reconcile app/dev-test (kappctrl.k14s.io/v1alpha1) namespace: default
4:34:13PM:  ^ Waiting for generation 1 to be observed
4:34:16PM: fail: reconcile app/dev-test (kappctrl.k14s.io/v1alpha1) namespace: default
4:34:16PM:  ^ Reconcile failed:  (message: kapp: Error: Expected to find kind '/v1/ConfigMaps', but did not:
- Kubernetes API server did not have matching apiVersion + kind
- No matching CRD was found in given configuration)

kapp: Error: waiting on reconcile app/dev-test (kappctrl.k14s.io/v1alpha1) namespace: default:
  Finished unsuccessfully (Reconcile failed:  (message: kapp: Error: Expected to find kind '/v1/ConfigMaps', but did not:
- Kubernetes API server did not have matching apiVersion + kind
- No matching CRD was found in given configuration))

[100mik]

@praveenrewar this would also affect the information fields in diff summary, how do you feel about that?
Example:

Target cluster 'https://127.0.0.1:54877' (nodes: minikube)

Changes

Namespace  Name      Kind  Age  Op  Op st.  Wait to    Rs    Ri  
default    dev-test  App   55s  -   -       reconcile  fail  Reconcile failed:  (message: kapp:  
                                                             Error: Expected to find kind  
                                                             '/v1/ConfigMaps', but did not:  
                                                             - Kubernetes API server did not  
                                                             have matching apiVersion + kind  
                                                             - No matching CRD was found in  
                                                             given configuration)  

Op:      0 create, 0 delete, 0 update, 1 noop, 0 exists
Wait to: 1 reconcile, 0 delete, 0 noop

We could start truncating here, but we do not do that for other resources (to the best of my knowledge)

[praveenrewar]

4:34:16PM:  ^ Reconcile failed:  (message: kapp: Error: Expected to find kind '/v1/ConfigMaps', but did not:
- Kubernetes API server did not have matching apiVersion + kind
- No matching CRD was found in given configuration)

kapp: Error: waiting on reconcile app/dev-test (kappctrl.k14s.io/v1alpha1) namespace: default:
  Finished unsuccessfully (Reconcile failed:  (message: kapp: Error: Expected to find kind '/v1/ConfigMaps', but did not:
- Kubernetes API server did not have matching apiVersion + kind
- No matching CRD was found in given configuration))

Hmm, that seems like a lot of error messages, should we try to avoid printing the error in one of the places? Probably not a good idea, but just thinking out loud.

[praveenrewar]

Namespace  Name      Kind  Age  Op  Op st.  Wait to    Rs    Ri  
default    dev-test  App   55s  -   -       reconcile  fail  Reconcile failed:  (message: kapp:  
                                                             Error: Expected to find kind  
                                                             '/v1/ConfigMaps', but did not:  
                                                             - Kubernetes API server did not  
                                                             have matching apiVersion + kind  
                                                             - No matching CRD was found in  
                                                             given configuration)

Ouch!! Yeah, truncating does seems like a good option, but how would users view the complete error message then 🤔

[100mik]

I do think it is helpful to show as much information as we can in the summary. And in this case, i do not think it looks too bad.
I am thinking about other scenarios. Like a failed fetch for instance where it might be too long.

It does feel weird to surface lesser information since we are trying to do the opposite here. I am trying to figure out whether there are cases where the summary would feel like it's illegible.

[praveenrewar]

LGTM!
Based on the feedback from users, we can look into updating/truncating the error messages later on.

[100mik]

That makes sense to me, merging this away!

[100mik]

kctrl should surface information for failed PackageInstallations that are a part of a meta package better for versions of kapp-controller that are using kapp with these changes 🚀